Re: git: fabf705f4b5a - main - pf: fix pf divert-to loop

On 19 Oct 2023, at 17:14, 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;

You don’t need the typedef, whether you use it or not is a matter of taste.

> 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.

We have __enum_uint8(_decl) these days, one could easily add a 16-bit variant.

Jess