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
