On 2026-04-19 18:45, Atanas Vladimirov wrote:
On 2026-04-19 12:53, Alexandr Nedvedicky wrote:
Hello,
I agree this is a bug.
On Sat, Apr 18, 2026 at 08:33:44PM +0300, Atanas Vladimirov wrote:
</snip>
Index: sys/net/pf_if.c
===================================================================
--- sys/net/pf_if.c
+++ sys/net/pf_if.c
@@ -852,8 +852,17 @@ pfi_set_flags(const char *name, int flags)
} else
panic("%s pfi_kif_get() returned NULL\n",
__func__);
- } else
+ } else {
p->pfik_flags_new = p->pfik_flags | flags;
+ /*
+ * The kif already existed (created by
+ * pfi_attach_ifnet()/pfi_attach_ifgroup()). Take a
+ * flag ref so it survives a transient disappearance
+ * of the backing ifnet/ifgroup, e.g. vmd(8) destroying
+ * and recreating tap(4) across a VM reboot.
+ */
+ if (ISSET(flags, PFI_IFLAG_SKIP) &&
+ p->pfik_flagrefs == 0)
+ pfi_kif_ref(p, PFI_KIF_REF_FLAG);
+ }
} else {
RB_FOREACH(p, pfi_ifhead, &pfi_ifs)
p->pfik_flags_new = p->pfik_flags | flags;
however diff above triggered an assert() on one of my test boxes:
kernel diagnostic assertion "(p->pfik_flagrefs == 0) ||
(p->pfik_flagref
s == 1)" failed: file "/home/sashan/src.sashan/sys/net/pf_if.c",
line 901
Stopped at db_enter+0x14: popq %rbp
TID PID UID PRFLAGS PFLAGS CPU COMMAND
* 30662 18016 0 0x3 0 0K pfctl
db_enter() at db_enter+0x14
panic(ffffffff82672a84) at panic+0xd5
__assert(ffffffff826aeb78,ffffffff8270f923,385,ffffffff82628d6b)
at __assert+0x
29
pfi_clear_flags(ffff80001c8d6200,100) at pfi_clear_flags+0x206
pfioctl(14900,c028445a,ffff80001c8d6200,3,ffff80001c7f14d8) at
pfioctl+0xe0f
VOP_IOCTL(fffffd802b6e6cd0,c028445a,ffff80001c8d6200,3,fffffd8004ffda90,ffff800
01c7f14d8) at VOP_IOCTL+0x60
vn_ioctl(fffffd802909ebd0,c028445a,ffff80001c8d6200,ffff80001c7f14d8)
at vn_ioc
tl+0x86
sys_ioctl(ffff80001c7f14d8,ffff80001c8d63b0,ffff80001c8d6330) at
sys_ioctl+0x30
e
syscall(ffff80001c8d63b0) at syscall+0x5f9
Xsyscall() at Xsyscall+0x128
the /etc/pf.conf on that test box contained two 'set skip on'
statements
for lo0 interface. Diff below avoids the assert on that box.
Taking a closer look at pfi_set_flags() function I think the branch
with RB_FOREACH() should be removed. The pfctl(8) always fills name
so that code is never reached on behalf of pfctl. Also the
RB_FOREACH()
loop body suffers from the same bug we attempt to fix here.
OK ?
Thanks, Alexandr!
I'll try to test your diff and report back.
I confirm that the patch works as it should.
The tap group and tap0 are skipped and the VM gets
dhcp lease immediately on boot.
Once again thanks for your effort!
Best wishes,
Atanas
thanks and
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
index 0c6681e757e..bba7237c904 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -854,8 +854,22 @@ pfi_set_flags(const char *name, int flags)
} else
panic("%s pfi_kif_get() returned NULL\n",
__func__);
- } else
+ } else {
+ /*
+ * pf.conf may accidentally contain two set skip on ...
+ * statements. For example:
+ * set skip lo
+ * set skip lo
+ * We need to grab reference only when skip flag is
+ * set to avoid tripping assert pfi_clear_flags()
+ */
+ if (ISSET(flags, PFI_IFLAG_SKIP) &&
+ !ISSET(p->pfik_flags_new, PFI_IFLAG_SKIP) &&
+ !ISSET(p->pfik_flags, PFI_IFLAG_SKIP))
+ pfi_kif_ref(p, PFI_KIF_REF_FLAG);
+
p->pfik_flags_new = p->pfik_flags | flags;
+ }
} else {
RB_FOREACH(p, pfi_ifhead, &pfi_ifs)
p->pfik_flags_new = p->pfik_flags | flags;
