From nobody Thu Oct 19 16:14:39 2023
X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1])
	by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SBCTs0qmvz4xcC8;
	Thu, 19 Oct 2023 16:14:41 +0000 (UTC)
	(envelope-from glebius@freebsd.org)
Received: from glebi.us (glebi.us [162.251.186.162])
	by mx1.freebsd.org (Postfix) with ESMTP id 4SBCTr6zJ6z3FGl;
	Thu, 19 Oct 2023 16:14:40 +0000 (UTC)
	(envelope-from glebius@freebsd.org)
Authentication-Results: mx1.freebsd.org;
	none
Received: by glebi.us (Postfix, from userid 1000)
	id 0B5A65A94D; Thu, 19 Oct 2023 09:14:40 -0700 (PDT)
Date: Thu, 19 Oct 2023 09:14:39 -0700
From: Gleb Smirnoff <glebius@freebsd.org>
To: Kristof Provost <kp@freebsd.org>, Igor Ostapenko <pm@igoro.pro>
Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org,
	dev-commits-src-main@freebsd.org
Subject: Re: git: fabf705f4b5a - main - pf: fix pf divert-to loop
Message-ID: <ZTFV74-C4amIm0ru@FreeBSD.org>
References: <202310191237.39JCbdXp094554@gitrepo.freebsd.org>
List-Id: Commit messages for the main branch of the src repository <dev-commits-src-main.freebsd.org>
List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main
List-Help: <mailto:dev-commits-src-main+help@freebsd.org>
List-Post: <mailto:dev-commits-src-main@freebsd.org>
List-Subscribe: <mailto:dev-commits-src-main+subscribe@freebsd.org>
List-Unsubscribe: <mailto:dev-commits-src-main+unsubscribe@freebsd.org>
Sender: owner-dev-commits-src-main@freebsd.org
X-BeenThere: dev-commits-src-main@freebsd.org
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202310191237.39JCbdXp094554@gitrepo.freebsd.org>
X-Spamd-Bar: ----
X-Rspamd-Pre-Result: action=no action;
	module=replies;
	Message is reply to one we originated
X-Spamd-Result: default: False [-4.00 / 15.00];
	REPLY(-4.00)[];
	ASN(0.00)[asn:27348, ipnet:162.251.186.0/24, country:US]
X-Rspamd-Queue-Id: 4SBCTr6zJ6z3FGl

On Thu, Oct 19, 2023 at 12:37:39PM +0000, Kristof Provost wrote:
K> +++ b/sys/netinet/ip_var.h
...
K> +/* pf specific mtag for divert(4) support */
K> +enum { PF_DIVERT_MTAG_DIR_IN=1, PF_DIVERT_MTAG_DIR_OUT=2 };
K> +struct pf_divert_mtag {
K> +	uint16_t idir;	// initial pkt direction
K> +	uint16_t ndir;	// a) divert(4) port upon initial diversion
K> +			// b) new direction upon pkt re-enter
K> +};

This can be written as:

typedef enum {
    PF_DIVERT_MTAG_DIR_IN = 1,
    PF_DIVERT_MTAG_DIR_OUT = 2,
} pf_mtag_dir;
struct pf_divert_mtag {
        pf_mtag_dir     idir;         /* Initial packet direction. */
        union {
                pf_mtag_dir     ndir; /* New direction after re-enter. */
                uint16_t        port;    /* Initial divert(4) port. */
        };
};

The benefit is that in the debugger you will see PF_DIVERT_MTAG_DIR_IN instead
of 1 when looking at a structure. And compilation time failure if anybody sets
it to a wrong value. Using "port" instead of "ndir" when assigning a port
improves readability of code.

This will grow structure from 4 bytes to 8, as enum is always an int. However,
uma allocator backing m_tag_alloc() will allocate same amount of memory.

Sorry for not messaing that in the phabricator.

-- 
Gleb Smirnoff