ipfw: refactor how we store bpf tap points

Make the tap database belong to ip_fw_chain, but leave the default "ipfw0"
tap per-vnet.  This is only slightly better than keeping the database per-
vnet, as the bpf name space is per-vnet.  However, we yet have only single
ipfw chain.  Whenever multiple chains will coexist, this needs to be
addressed.

Require the chain lock to make modifications to the database.

Move tap allocation to a later ruleset build stage, when all rule numbers
are known already.  This fixes a panic introduced by 3daae1ac1d.

Fixes:	3daae1ac1d

sys/netpfil/ipfw/ip_fw2.c

@@ -3667,6 +3667,7 @@ vnet_ipfw_init(const void *unused)
 #ifdef IPFIREWALL_NAT
 	LIST_INIT(&chain->nat);
 #endif
+	RB_INIT(&chain->taps);
 
 	/* Init shared services hash table */
 	ipfw_init_srv(chain);

sys/netpfil/ipfw/ip_fw_bpf.c

@@ -1,7 +1,7 @@
 /*-
  * Copyright (c) 2016 Yandex LLC
  * Copyright (c) 2016 Andrey V. Elsukov <ae@FreeBSD.org>
- * Copyright (c) 2025 Gleb Smirnoff <glebius@FreeBSD.org>
+ * Copyright (c) 2025-2026 Gleb Smirnoff <glebius@FreeBSD.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -68,20 +68,20 @@ tap_compare(const struct ipfw_tap *a, const struct ipfw_tap *b)
 {
 	return (a->rule != b->rule ? (a->rule < b->rule ? -1 : 1) : 0);
 }
-RB_HEAD(tap_tree, ipfw_tap);
-VNET_DEFINE_STATIC(struct tap_tree, tap_tree);
-#define	V_tap_tree	VNET(tap_tree)
 RB_GENERATE_STATIC(tap_tree, ipfw_tap, entry, tap_compare);
-VNET_DEFINE_STATIC(struct ipfw_tap *, default_tap);
+VNET_DEFINE_STATIC(struct ipfw_tap, default_tap) = { .name = "ipfw0" };
 #define	V_default_tap	VNET(default_tap)
 
 void
-ipfw_tap_alloc(uint32_t rule)
+ipfw_tap_alloc(struct ip_fw_chain *ch, uint32_t rule)
 {
 	struct ipfw_tap	*tap, key = { .rule = rule };
 	int n __diagused;
 
-	tap = RB_FIND(tap_tree, &V_tap_tree, &key);
+	MPASS(rule > 0 && rule < IPFW_DEFAULT_RULE);
+	IPFW_UH_WLOCK_ASSERT(ch);
+
+	tap = RB_FIND(tap_tree, &ch->taps, &key);
 	if (tap != NULL) {
 		MPASS(tap->rule == rule);
 		tap->refs++;
@@ -90,43 +90,51 @@ ipfw_tap_alloc(uint32_t rule)
 	tap = malloc(sizeof(*tap), M_IPFW, M_WAITOK);
 	tap->rule = rule;
 	tap->refs = 1;
-	/* Note: the default rule logs to "ipfw0". */
-	if (__predict_false(rule == IPFW_DEFAULT_RULE)) {
-		V_default_tap = tap;
-		rule = 0;
-	}
 	n = snprintf(tap->name, sizeof(tap->name), "ipfw%u", rule);
 	MPASS(n > 4 && n < sizeof("ipfw4294967295"));
 	tap->bpf = bpf_attach(tap->name, DLT_EN10MB, PFLOG_HDRLEN,
 	    &bpf_ipfw_methods, NULL);
-	tap = RB_INSERT(tap_tree, &V_tap_tree, tap);
+	tap = RB_INSERT(tap_tree, &ch->taps, tap);
 	MPASS(tap == NULL);
 }
 
 void
-ipfw_tap_free(uint32_t rule)
+ipfw_tap_free(struct ip_fw_chain *ch, uint32_t rule)
 {
-
 	struct ipfw_tap	*tap, key = { .rule = rule };
 
-	tap = RB_FIND(tap_tree, &V_tap_tree, &key);
+	MPASS(rule > 0 && rule < IPFW_DEFAULT_RULE);
+	IPFW_UH_WLOCK_ASSERT(ch);
+
+	tap = RB_FIND(tap_tree, &ch->taps, &key);
 	MPASS(tap != NULL);
 	if (--tap->refs == 0) {
 		bpf_detach(tap->bpf);
-		RB_REMOVE(tap_tree, &V_tap_tree, tap);
+		RB_REMOVE(tap_tree, &ch->taps, tap);
 		free(tap, M_IPFW);
 	}
 }
 
 void
-ipfw_bpf_tap(struct ip_fw_args *args, struct ip *ip, uint32_t rulenum)
+ipfw_bpf_tap(struct ip_fw_chain *ch, struct ip_fw_args *args,
+    struct ip *ip, uint32_t rulenum)
 {
-	struct ipfw_tap *tap, key = { .rule = rulenum };
+	struct ipfw_tap *tap;
 
-	tap = RB_FIND(tap_tree, &V_tap_tree, &key);
-	MPASS(tap != NULL);
-	if (!bpf_peers_present(tap->bpf))
-		tap = V_default_tap;
+	if (rulenum == IPFW_DEFAULT_RULE) {
+		tap = &V_default_tap;
+	} else {
+		struct ipfw_tap key = { .rule = rulenum };
+
+		tap = RB_FIND(tap_tree, &ch->taps, &key);
+		MPASS(tap != NULL);
+		/*
+		 * Compatibility: if user is not using per-rule taps, fallback
+		 * to the default tap.
+		 */
+		if (!bpf_peers_present(tap->bpf))
+			tap = &V_default_tap;
+	}
 	if (args->flags & IPFW_ARGS_LENMASK) {
 		bpf_tap(tap->bpf, args->mem, IPFW_ARGS_LENGTH(args->flags));
 	} else if (args->flags & IPFW_ARGS_ETHER) {
@@ -161,7 +169,8 @@ ipfw_pflog_tap(void *data, struct mbuf *m)
 void
 ipfw_bpf_init(int first __unused)
 {
-	ipfw_tap_alloc(IPFW_DEFAULT_RULE);
+	V_default_tap.bpf = bpf_attach(V_default_tap.name, DLT_EN10MB,
+	    PFLOG_HDRLEN, &bpf_ipfw_methods, NULL);
 	V_bpf_pflog = bpf_attach("ipfwlog0", DLT_PFLOG, PFLOG_HDRLEN,
 	    &bpf_ipfw_methods, NULL);
 }
@@ -169,7 +178,6 @@ ipfw_bpf_init(int first __unused)
 void
 ipfw_bpf_uninit(int last __unused)
 {
-
-	ipfw_tap_free(IPFW_DEFAULT_RULE);
+	bpf_detach(V_default_tap.bpf);
 	bpf_detach(V_bpf_pflog);
 }

sys/netpfil/ipfw/ip_fw_log.c

@@ -722,7 +722,7 @@ ipfw_log(struct ip_fw_chain *chain, struct ip_fw *f, u_int hlen,
 	    /* O_LOG is the first action */
 	    ((cmd = ACTION_PTR(f)) && cmd->arg1 == IPFW_LOG_DEFAULT)) {
 		if (V_fw_verbose == 0) {
-			ipfw_bpf_tap(args, ip,
+			ipfw_bpf_tap(chain, args, ip,
 			    f != NULL ? f->rulenum : IPFW_DEFAULT_RULE);
 			return;
 		}
@@ -737,6 +737,6 @@ ipfw_log(struct ip_fw_chain *chain, struct ip_fw *f, u_int hlen,
 		ipfw_log_rtsock(chain, f, hlen, args, offset, tablearg, eh);
 
 	if (cmd->arg1 & IPFW_LOG_IPFW0)
-		ipfw_bpf_tap(args, ip, f->rulenum);
+		ipfw_bpf_tap(chain, args, ip, f->rulenum);
 }
 /* end of file */

sys/netpfil/ipfw/ip_fw_private.h

@@ -28,6 +28,9 @@
 #ifndef _IPFW2_PRIVATE_H
 #define _IPFW2_PRIVATE_H
 
+#include <sys/queue.h>
+#include <sys/tree.h>
+
 /*
  * Internal constants and data structures used by ipfw components
  * and not meant to be exported outside the kernel.
@@ -161,9 +164,10 @@ struct ip_fw_chain;
 
 void ipfw_bpf_init(int);
 void ipfw_bpf_uninit(int);
-void ipfw_tap_alloc(uint32_t);
-void ipfw_tap_free(uint32_t);
-void ipfw_bpf_tap(struct ip_fw_args *, struct ip *, uint32_t);
+void ipfw_tap_alloc(struct ip_fw_chain *, uint32_t);
+void ipfw_tap_free(struct ip_fw_chain *, uint32_t);
+void ipfw_bpf_tap(struct ip_fw_chain *, struct ip_fw_args *, struct ip *,
+    uint32_t);
 void ipfw_pflog_tap(void *, struct mbuf *);
 void ipfw_log(struct ip_fw_chain *chain, struct ip_fw *f, u_int hlen,
     struct ip_fw_args *args, u_short offset, uint32_t tablearg, struct ip *ip,
@@ -320,6 +324,7 @@ struct ip_fw_chain {
 	void		*ifcfg;		/* interface module data */
 	int		*idxmap_back;	/* standby skipto array of rules */
 	struct namedobj_instance	*srvmap; /* cfg name->number mappings */
+	RB_HEAD(tap_tree, ipfw_tap) taps;	/* see ip_fw_bpf.c */
 #if defined( __linux__ ) || defined( _WIN32 )
 	spinlock_t uh_lock;
 #else

sys/netpfil/ipfw/ip_fw_sockopt.c

@@ -210,8 +210,6 @@ ipfw_free_rule(struct ip_fw *rule)
 	 */
 	if (rule->refcnt > 1)
 		return;
-	if (ACTION_PTR(rule)->opcode == O_LOG)
-		ipfw_tap_free(rule->rulenum);
 	uma_zfree_pcpu(V_ipfw_cntr_zone, rule->cntr);
 	free(rule, M_IPFW);
 }
@@ -505,6 +503,8 @@ ipfw_commit_rules(struct ip_fw_chain *chain, struct rule_check_info *rci,
 			memcpy((char *)ci->urule + ci->urule_numoff, &rulenum,
 			    sizeof(rulenum));
 		}
+		if (ACTION_PTR(krule)->opcode == O_LOG)
+			ipfw_tap_alloc(chain, krule->rulenum);
 	}
 
 	/* duplicate the remaining part, we always have the default rule */
@@ -2102,6 +2102,8 @@ unref_rule_objects(struct ip_fw_chain *ch, struct ip_fw *rule)
 		else
 			no->refcnt--;
 	}
+	if (ACTION_PTR(rule)->opcode == O_LOG)
+		ipfw_tap_free(ch, rule->rulenum);
 }
 
 /*
@@ -2459,9 +2461,6 @@ import_rule_v1(struct ip_fw_chain *chain, struct rule_check_info *ci)
 
 	/* Copy opcodes */
 	memcpy(krule->cmd, urule->cmd, krule->cmd_len * sizeof(uint32_t));
-
-	if (ACTION_PTR(krule)->opcode == O_LOG)
-		ipfw_tap_alloc(krule->rulenum);
 }
 
 /*