Date: Thu, 19 Oct 2023 18:19:30 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Gleb Smirnoff <glebius@freebsd.org> Cc: Kristof Provost <kp@freebsd.org>, Igor Ostapenko <pm@igoro.pro>, 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: <CAGudoHEdKVHLnqCh1gGppDe9C9KeihLd45dXdraCMtuun=CSEg@mail.gmail.com> In-Reply-To: <ZTFV74-C4amIm0ru@FreeBSD.org> References: <202310191237.39JCbdXp094554@gitrepo.freebsd.org> <ZTFV74-C4amIm0ru@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/19/23, Gleb Smirnoff <glebius@freebsd.org> wrote:
> 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.
>
I don't have an opinion on employing an enum, but if going that route
one can shrink it by using smaller enums -- grep for __enum_uint8 for
an example.
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEdKVHLnqCh1gGppDe9C9KeihLd45dXdraCMtuun=CSEg>