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

On 19 Oct 2023, at 18:14, Gleb Smirnoff 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.
>
Something like this?

	diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c
	index ad95a1ce0d76..78ca36fc2a0f 100644
	--- a/sys/netinet/ip_divert.c
	+++ b/sys/netinet/ip_divert.c
	@@ -182,7 +182,7 @@ divert_packet(struct mbuf *m, bool incoming)
	                    (((struct ipfw_rule_ref *)(mtag+1))->info));
	        } else if ((mtag = m_tag_locate(m, MTAG_PF_DIVERT, 0, NULL)) != 
NULL) {
	                cookie = ((struct pf_divert_mtag *)(mtag+1))->idir;
	-               nport = htons(((struct pf_divert_mtag 
*)(mtag+1))->ndir);
	+               nport = htons(((struct pf_divert_mtag 
*)(mtag+1))->port);
	        } else {
	                m_freem(m);
	                return;
	diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
	index a8c687682af9..0f3facc54d4e 100644
	--- a/sys/netinet/ip_var.h
	+++ b/sys/netinet/ip_var.h
	@@ -328,11 +328,17 @@ extern int        (*ip_dn_ctl_ptr)(struct sockopt 
*);
	 extern int     (*ip_dn_io_ptr)(struct mbuf **, struct ip_fw_args *);

	 /* pf specific mtag for divert(4) support */
	-enum { PF_DIVERT_MTAG_DIR_IN=1, PF_DIVERT_MTAG_DIR_OUT=2 };
	+__enum_uint8_decl(pf_mtag_dir) {
	+       PF_DIVERT_MTAG_DIR_IN = 1,
	+       PF_DIVERT_MTAG_DIR_OUT = 2
	+};
	 struct pf_divert_mtag {
	        uint16_t idir;  // initial pkt direction
	-       uint16_t ndir;  // a) divert(4) port upon initial diversion
	-                       // b) new direction upon pkt re-enter
	+       union {
	+               __enum_uint8(pf_mtag_dir) ndir; // a) divert(4) port 
upon initial diversion
	+                               // b) new direction upon pkt re-enter
	+               uint16_t port;  /* Initial divert(4) port */
	+       };
	 };
	 #define MTAG_PF_DIVERT 1262273569

	diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
	index a6c7ee359416..1cd8412193dc 100644
	--- a/sys/netpfil/pf/pf.c
	+++ b/sys/netpfil/pf/pf.c
	@@ -8022,7 +8022,7 @@ pf_test(int dir, int pflags, struct ifnet *ifp, 
struct mbuf **m0,
	                mtag = m_tag_alloc(MTAG_PF_DIVERT, 0,
	                    sizeof(struct pf_divert_mtag), M_NOWAIT | M_ZERO);
	                if (mtag != NULL) {
	-                       ((struct pf_divert_mtag *)(mtag+1))->ndir =
	+                       ((struct pf_divert_mtag *)(mtag+1))->port =
	                            ntohs(r->divert.port);
	                        ((struct pf_divert_mtag *)(mtag+1))->idir =
	                            (dir == PF_IN) ? PF_DIVERT_MTAG_DIR_IN :

Best regards,
Kristof