Re: KASSERT() @ pf_test() is back
On Mon, Mar 07, 2016 at 10:54:26AM +0100, Mattieu Baptiste wrote: > On Mon, Mar 7, 2016 at 10:03 AM, Alexandr Nedvedicky > wrote: > > Hello Mattieu, > > > > Mark Patruck reported panic on KASSERT() in pf_test() yesterday . I've > > crafted > > patch below. Can you try it out? > > > > I think we need to apply pf_pkt_addr_changed() on broadcast packets seen by > > bridge > > as well. > > > > thanks a lot > > and sorry for inconveniences > > Hi Alexandr, > I don't know if both are necessary but, with your patch and the one > from Martin, the box survives after reboot. Thanks! > Do you want I test just with your patch? Hello Mattieu, I think it makes no point to test my patch only, Mark has done it already. do it just in case you really have time to do it. I still need to look at Martin's diff more closely. It's very likely it fixes yet another edge case, which we are not aware of. thanks for quick testing. regards sasha
Re: KASSERT() @ pf_test() is back
On Mon, Mar 7, 2016 at 10:03 AM, Alexandr Nedvedicky wrote: > Hello Mattieu, > > Mark Patruck reported panic on KASSERT() in pf_test() yesterday . I've crafted > patch below. Can you try it out? > > I think we need to apply pf_pkt_addr_changed() on broadcast packets seen by > bridge > as well. > > thanks a lot > and sorry for inconveniences Hi Alexandr, I don't know if both are necessary but, with your patch and the one from Martin, the box survives after reboot. Thanks! Do you want I test just with your patch?
Re: KASSERT() @ pf_test() is back
Hello Mattieu, Mark Patruck reported panic on KASSERT() in pf_test() yesterday . I've crafted patch below. Can you try it out? I think we need to apply pf_pkt_addr_changed() on broadcast packets seen by bridge as well. thanks a lot and sorry for inconveniences regards sasha -8<8<8<---8< Index: if_bridge.c === RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.275 diff -u -p -r1.275 if_bridge.c --- if_bridge.c 5 Dec 2015 10:07:55 - 1.275 +++ if_bridge.c 7 Mar 2016 09:00:06 - @@ -1283,6 +1283,10 @@ bridge_localbroadcast(struct bridge_soft return; } +#if NPF > 0 + pf_pkt_addr_changed(m1); +#endif /* NPF */ + bridge_ifinput(ifp, m1); }
Re: KASSERT() @ pf_test() is back
On Sun, Mar 6, 2016 at 11:33 PM, Martin Pieuchot wrote: > On 06/03/16(Sun) 22:35, Mattieu Baptiste wrote: >> On Sun, Mar 6, 2016 at 10:22 PM, Martin Pieuchot wrote: >> > On 06/03/16(Sun) 19:57, Mattieu Baptiste wrote: >> >> [...] >> >> As discussed in january, with the patches committed, -current still >> >> panics for me: >> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic10.jpg >> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic11.jpg >> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic12.jpg >> >> >> >> Here is a dmesg from the last time I built a kernel, before the >> >> patches were committed: >> > >> > Could you also paste the content of your /etc/hostname.* and "route -n >> > show" >> > for this system? >> >> Yes, sure: >> >> $ cat /etc/hostname.bridge0 >> add em0 >> add re0 >> up > > Hahaha, my favorite! Could you try the diff below and tell me if it > helps? Sorry, it panics again: https://www.brimbelle.org/mattieu/stuff/panic_statekey/panic13.jpg -- Mattieu Baptiste "/earth is 102% full ... please delete anyone you can."
Re: KASSERT() @ pf_test() is back
On 06/03/16(Sun) 22:35, Mattieu Baptiste wrote:
> On Sun, Mar 6, 2016 at 10:22 PM, Martin Pieuchot wrote:
> > On 06/03/16(Sun) 19:57, Mattieu Baptiste wrote:
> >> [...]
> >> As discussed in january, with the patches committed, -current still
> >> panics for me:
> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic10.jpg
> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic11.jpg
> >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic12.jpg
> >>
> >> Here is a dmesg from the last time I built a kernel, before the
> >> patches were committed:
> >
> > Could you also paste the content of your /etc/hostname.* and "route -n show"
> > for this system?
>
> Yes, sure:
>
> $ cat /etc/hostname.bridge0
> add em0
> add re0
> up
Hahaha, my favorite! Could you try the diff below and tell me if it
helps?
Index: if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.275
diff -u -p -r1.275 if_bridge.c
--- if_bridge.c 5 Dec 2015 10:07:55 - 1.275
+++ if_bridge.c 6 Mar 2016 22:31:28 -
@@ -1861,6 +1861,10 @@ bridge_ifenqueue(struct bridge_softc *sc
#endif /* NGIF */
len = m->m_pkthdr.len;
+#if NPF > 0
+ pf_pkt_addr_changed(m);
+#endif
+
error = if_enqueue(ifp, m);
if (error) {
sc->sc_if.if_oerrors++;
Re: KASSERT() @ pf_test() is back
On Sun, Mar 6, 2016 at 10:22 PM, Martin Pieuchot wrote: > On 06/03/16(Sun) 19:57, Mattieu Baptiste wrote: >> [...] >> As discussed in january, with the patches committed, -current still >> panics for me: >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic10.jpg >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic11.jpg >> http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic12.jpg >> >> Here is a dmesg from the last time I built a kernel, before the >> patches were committed: > > Could you also paste the content of your /etc/hostname.* and "route -n show" > for this system? Yes, sure: $ cat /etc/hostname.bridge0 add em0 add re0 up $ cat /etc/hostname.em0 dhcp rtsol inet6 2a01:e35:8aee:3fb1::24 64 up $ cat /etc/hostname.re0 wol $ route -n show Routing tables Internet: DestinationGatewayFlags Refs Use Mtu Prio Iface default192.168.42.1 UGS4 3075 - 8 em0 127/8 127.0.0.1 UGRS 00 32768 8 lo0 127.0.0.1 127.0.0.1 UHl00 32768 1 lo0 192.168.42/24 192.168.42.24 UC 3 6078 - 4 em0 192.168.42.1 00:0d:b9:33:8e:8c UHLc 2 395 - 4 em0 192.168.42.10 68:05:ca:09:38:ac UHLc 3 38 - 4 em0 192.168.42.16 68:05:ca:09:38:ac UHLc 0 400 - 4 em0 192.168.42.24 00:15:17:8a:8f:d2 UHLl 0 222 - 1 em0 192.168.42.255 192.168.42.24 UHb0 9078 - 1 em0 224/4 127.0.0.1 URS0 3434 32768 8 lo0 Internet6: DestinationGateway Flags Refs Use Mtu Prio Iface ::/104 ::1UGRS 00 32768 8 lo0 ::/96 ::1UGRS 00 32768 8 lo0 defaultfe80::20d:b9ff:fe33:8e8c%em0 UG 18 912 -56 em0 ::1::1UHl 55 32768 1 lo0 ::127.0.0.0/104::1UGRS 00 32768 8 lo0 ::224.0.0.0/100::1UGRS 00 32768 8 lo0 ::255.0.0.0/104::1UGRS 00 32768 8 lo0 :::0.0.0.0/96 ::1UGRS 00 32768 8 lo0 2002::/24 ::1UGRS 00 32768 8 lo0 2002:7f00::/24 ::1UGRS 00 32768 8 lo0 2002:e000::/20 ::1UGRS 00 32768 8 lo0 2002:ff00::/24 ::1UGRS 00 32768 8 lo0 2a01:e35:8aee:3fb1::/642a01:e35:8aee:3fb1::24 UCP 14 - 4 em0 2a01:e35:8aee:3fb1::/64 2a01:e35:8aee:3fb1:215:17ff:fe8a:8fd2 UCP00 - 4 em0 2a01:e35:8aee:3fb1::1 00:0d:b9:33:8e:8c UHLc 0 1275 - 4 em0 2a01:e35:8aee:3fb1::24 00:15:17:8a:8f:d2 UHLl 00 - 1 em0 2a01:e35:8aee:3fb1:215:17ff:fe8a:8fd2 00:15:17:8a:8f:d2 UHLl 00 - 1 em0 2a01:e35:8aee:3fb1:dc59:37be:5b19:dd1b 00:15:17:8a:8f:d2 UHLl 0 4886 - 1 em0 fe80::/10 ::1UGRS 01 32768 8 lo0 fe80::%em0/64 fe80::215:17ff:fe8a:8fd2%em0 UC 15 - 4 em0 fe80::20d:b9ff:fe33:8e8c%em0 00:0d:b9:33:8e:8c UHLc 1 3313 - 4 em0 fe80::215:17ff:fe8a:8fd2%em0 00:15:17:8a:8f:d2 UHLl 0 384 - 1 em0 fe80::1%lo0fe80::1%lo0UHl 00 32768 1 lo0 fec0::/10 ::1UGRS 00 32768 8 lo0 ff01::/16 ::1UGRS 01 32768 8 lo0 ff01::%em0/32 fe80::215:17ff:fe8a:8fd2%em0 UC 03 - 4 em0 ff01::%lo0/32 ::1UC 01 32768 4 lo0 ff02::/16 ::1UGRS 01 32768 8 lo0 ff02::%em0/32 fe80::215:17ff:fe8a:8fd2%em0 UC 03 - 4 em0 ff02::%lo0/32 ::1UC 01 32768 4 lo0 -- Mattieu Baptiste "/earth is 102% full ... please delete anyone you can."
Re: KASSERT() @ pf_test() is back
On 06/03/16(Sun) 19:57, Mattieu Baptiste wrote: > [...] > As discussed in january, with the patches committed, -current still > panics for me: > http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic10.jpg > http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic11.jpg > http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic12.jpg > > Here is a dmesg from the last time I built a kernel, before the > patches were committed: Could you also paste the content of your /etc/hostname.* and "route -n show" for this system?
Re: KASSERT() @ pf_test() is back
On Fri, Mar 4, 2016 at 2:31 PM, Alexandr Nedvedicky wrote: > Hello Stuart, > > thanks for testing it. I'll commit it today. > Hi Alexandr, As discussed in january, with the patches committed, -current still panics for me: http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic10.jpg http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic11.jpg http://www.brimbelle.org/mattieu/stuff/panic_statekey/panic12.jpg Here is a dmesg from the last time I built a kernel, before the patches were committed: OpenBSD 5.9 (GENERIC.MP) #41: Thu Feb 11 22:18:29 CET 2016 matt...@kronenbourg.brimbelle.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 8571518976 (8174MB) avail mem = 8307548160 (7922MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xf0710 (68 entries) bios0: vendor American Megatrends Inc. version "2003" date 12/14/2010 bios0: ASUSTeK Computer INC. P7P55D acpi0 at bios0: rev 2 acpi0: sleep states S0 S1 S3 S4 S5 acpi0: tables DSDT FACP APIC MCFG OEMB HPET DMAR ASPT OSFR acpi0: wakeup devices P0P4(S4) BR1E(S4) UAR1(S4) PS2K(S4) PS2M(S4) EUSB(S4) USB0(S4) USB1(S4) USB2(S4) USB3(S4) USBE(S4) USB4(S4) USB5(S4) USB6(S4) BR21(S4) BR22(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5 CPU 660 @ 3.33GHz, 3374.41 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC,SENSOR,ARAT cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 160MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE cpu1 at mainbus0: apid 4 (application processor) cpu1: Intel(R) Core(TM) i5 CPU 660 @ 3.33GHz, 3373.89 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC,SENSOR,ARAT cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 2, package 0 cpu2 at mainbus0: apid 1 (application processor) cpu2: Intel(R) Core(TM) i5 CPU 660 @ 3.33GHz, 3373.89 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC,SENSOR,ARAT cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 1, core 0, package 0 cpu3 at mainbus0: apid 5 (application processor) cpu3: Intel(R) Core(TM) i5 CPU 660 @ 3.33GHz, 3373.90 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC,SENSOR,ARAT cpu3: 256KB 64b/line 8-way L2 cache cpu3: smt 1, core 2, package 0 ioapic0 at mainbus0: apid 6 pa 0xfec0, version 20, 24 pins ioapic0: misconfigured as apic 1, remapped to apid 6 acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 7 (BR1E) acpiprt2 at acpi0: bus -1 (BR21) acpiprt3 at acpi0: bus -1 (BR22) acpiprt4 at acpi0: bus -1 (BR23) acpiprt5 at acpi0: bus 1 (P0P1) acpiprt6 at acpi0: bus -1 (P0P3) acpiprt7 at acpi0: bus -1 (P0P5) acpiprt8 at acpi0: bus -1 (P0P6) acpiprt9 at acpi0: bus 6 (BR20) acpiprt10 at acpi0: bus 5 (BR24) acpiprt11 at acpi0: bus 4 (BR25) acpiprt12 at acpi0: bus 3 (BR26) acpiprt13 at acpi0: bus 2 (BR27) acpiec0 at acpi0 acpicpu0 at acpi0: C1(@1 halt!) acpicpu1 at acpi0: C1(@1 halt!) acpicpu2 at acpi0: C1(@1 halt!) acpicpu3 at acpi0: C1(@1 halt!) aibs0 at acpi0 GGRP GITM SITM acpibtn0 at acpi0: PWRB pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel Core Host" rev 0x12 ppb0 at pci0 dev 1 function 0 "Intel Core PCIE" rev 0x12: msi pci1 at ppb0 bus 1 radeondrm0 at pci1 dev 0 function 0 "ATI Radeon HD 4670" rev 0x00 drm0 at radeondrm0 radeondrm0: msi azalia0 at pci1 dev 0 function 1 "ATI Radeon HD 4000 HD Audio" rev 0x00: msi azalia0: no supported codecs ehci0 at pci0 dev 26 function 0 "Intel 3400 USB" rev 0x06: apic 6 int 16 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 azalia1 at pci0 dev 27 function 0 "Intel 3400 HD Audio" rev 0x06: msi azalia1: codecs: VIA/0x4441 audio0 at azalia1 ppb1 at pci0 dev 28 function 0 "Intel 3400 PCIE" rev 0x06: msi pci2 at ppb1 bus 6 em0 at pci2 dev 0 function 0 "Intel 82571EB" rev 0x06: apic 6 int 16, address 00:15:17:8a:8f:d2 em1 at pci2 dev 0 function 1 "Intel 82571EB" rev 0x06: apic 6 int 17, address 00:15:17:8a:8f:d3 ppb2 at pci0 dev 28 function 4 "Intel 3400 PC
Re: KASSERT() @ pf_test() is back
Hello Stuart, thanks for testing it. I'll commit it today. regards sasha On Fri, Mar 04, 2016 at 01:08:42AM +, Stuart Henderson wrote: > On 2016/02/28 13:01, Martin Pieuchot wrote: > > On 08/02/16(Mon) 01:55, Alexandr Nedvedicky wrote: > > > Hello, > > > > > > I don't expect to see O.K. to patch below. > > > > > > The patch is the part of the change, which has been backed out last > > > weekend. > > > Too many things were wrong so I'm trying to untangle the code a bit. > > > > > > Patch below is for brave hearts, who don't mind to see KASSERT() to fire: > > > > Any progress with this diff? Now would be the good time to get it in. > > I'm running it on a pppoe router (v4/v6 plus ipsec), it hasn't exploded > yet. I'm in favour of putting it in to see what happens, it's a good time > in the cycle. >
Re: KASSERT() @ pf_test() is back
On 2016/02/28 13:01, Martin Pieuchot wrote:
> On 08/02/16(Mon) 01:55, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > I don't expect to see O.K. to patch below.
> >
> > The patch is the part of the change, which has been backed out last weekend.
> > Too many things were wrong so I'm trying to untangle the code a bit.
> >
> > Patch below is for brave hearts, who don't mind to see KASSERT() to fire:
>
> Any progress with this diff? Now would be the good time to get it in.
I'm running it on a pppoe router (v4/v6 plus ipsec), it hasn't exploded
yet. I'm in favour of putting it in to see what happens, it's a good time
in the cycle.
> > Index: net/if_etherip.c
> > ===
> > RCS file: /cvs/src/sys/net/if_etherip.c,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 if_etherip.c
> > --- net/if_etherip.c25 Jan 2016 05:12:34 - 1.5
> > +++ net/if_etherip.c7 Feb 2016 23:38:09 -
> > @@ -499,6 +499,10 @@ ip_etherip_input(struct mbuf *m, ...)
> > }
> > m->m_flags &= ~(M_BCAST|M_MCAST);
> >
> > +#if NPF > 0
> > + pf_pkt_addr_changed(m);
> > +#endif
> > +
> > ml_enqueue(&ml, m);
> > if_input(ifp, &ml);
> > }
> > @@ -641,6 +645,10 @@ ip6_etherip_input(struct mbuf **mp, int
> > }
> >
> > m->m_flags &= ~(M_BCAST|M_MCAST);
> > +
> > +#if NPF > 0
> > + pf_pkt_addr_changed(m);
> > +#endif
> >
> > ml_enqueue(&ml, m);
> > if_input(ifp, &ml);
> > Index: net/pf.c
> > ===
> > RCS file: /cvs/src/sys/net/pf.c,v
> > retrieving revision 1.965
> > diff -u -p -r1.965 pf.c
> > --- net/pf.c31 Jan 2016 00:18:07 - 1.965
> > +++ net/pf.c7 Feb 2016 23:38:16 -
> > @@ -6534,6 +6534,12 @@ done:
> > if (action == PF_PASS && qid)
> > pd.m->m_pkthdr.pf.qid = qid;
> > if (pd.dir == PF_IN && s && s->key[PF_SK_STACK]) {
> > + /*
> > +* ASSERT() below fires whenever caller forgets to call
> > +* pf_pkt_addr_changed(). This might happen when we deal with
> > +* IP tunnels.
> > +*/
> > + KASSERT(pd.m->m_pkthdr.pf.statekey == NULL);
> > pd.m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
> > }
> > if (pd.dir == PF_OUT &&
> > Index: net/pipex.c
> > ===
> > RCS file: /cvs/src/sys/net/pipex.c,v
> > retrieving revision 1.84
> > diff -u -p -r1.84 pipex.c
> > --- net/pipex.c 3 Nov 2015 21:33:56 - 1.84
> > +++ net/pipex.c 7 Feb 2016 23:38:18 -
> > @@ -1139,6 +1139,10 @@ pipex_ip_input(struct mbuf *m0, struct p
> > goto drop;
> > }
> >
> > +#if NPF > 0
> > + pf_pkt_addr_changed(m0);
> > +#endif
> > +
> > len = m0->m_pkthdr.len;
> >
> > #if NBPFILTER > 0
> > Index: netinet/ip_gre.c
> > ===
> > RCS file: /cvs/src/sys/netinet/ip_gre.c,v
> > retrieving revision 1.58
> > diff -u -p -r1.58 ip_gre.c
> > --- netinet/ip_gre.c2 Dec 2015 08:47:00 - 1.58
> > +++ netinet/ip_gre.c7 Feb 2016 23:38:20 -
> > @@ -337,6 +337,10 @@ gre_mobile_input(struct mbuf *m, ...)
> > bpf_mtap_af(sc->sc_if.if_bpf, AF_INET, m, BPF_DIRECTION_IN);
> > #endif
> >
> > +#if NPF > 0
> > + pf_pkt_addr_changed(m);
> > +#endif
> > +
> > niq_enqueue(&ipintrq, m);
> > }
> >
> >
>
Re: KASSERT() @ pf_test() is back
On 08/02/16(Mon) 01:55, Alexandr Nedvedicky wrote:
> Hello,
>
> I don't expect to see O.K. to patch below.
>
> The patch is the part of the change, which has been backed out last weekend.
> Too many things were wrong so I'm trying to untangle the code a bit.
>
> Patch below is for brave hearts, who don't mind to see KASSERT() to fire:
Any progress with this diff? Now would be the good time to get it in.
> Index: net/if_etherip.c
> ===
> RCS file: /cvs/src/sys/net/if_etherip.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 if_etherip.c
> --- net/if_etherip.c 25 Jan 2016 05:12:34 - 1.5
> +++ net/if_etherip.c 7 Feb 2016 23:38:09 -
> @@ -499,6 +499,10 @@ ip_etherip_input(struct mbuf *m, ...)
> }
> m->m_flags &= ~(M_BCAST|M_MCAST);
>
> +#if NPF > 0
> + pf_pkt_addr_changed(m);
> +#endif
> +
> ml_enqueue(&ml, m);
> if_input(ifp, &ml);
> }
> @@ -641,6 +645,10 @@ ip6_etherip_input(struct mbuf **mp, int
> }
>
> m->m_flags &= ~(M_BCAST|M_MCAST);
> +
> +#if NPF > 0
> + pf_pkt_addr_changed(m);
> +#endif
>
> ml_enqueue(&ml, m);
> if_input(ifp, &ml);
> Index: net/pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.965
> diff -u -p -r1.965 pf.c
> --- net/pf.c 31 Jan 2016 00:18:07 - 1.965
> +++ net/pf.c 7 Feb 2016 23:38:16 -
> @@ -6534,6 +6534,12 @@ done:
> if (action == PF_PASS && qid)
> pd.m->m_pkthdr.pf.qid = qid;
> if (pd.dir == PF_IN && s && s->key[PF_SK_STACK]) {
> + /*
> + * ASSERT() below fires whenever caller forgets to call
> + * pf_pkt_addr_changed(). This might happen when we deal with
> + * IP tunnels.
> + */
> + KASSERT(pd.m->m_pkthdr.pf.statekey == NULL);
> pd.m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
> }
> if (pd.dir == PF_OUT &&
> Index: net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 pipex.c
> --- net/pipex.c 3 Nov 2015 21:33:56 - 1.84
> +++ net/pipex.c 7 Feb 2016 23:38:18 -
> @@ -1139,6 +1139,10 @@ pipex_ip_input(struct mbuf *m0, struct p
> goto drop;
> }
>
> +#if NPF > 0
> + pf_pkt_addr_changed(m0);
> +#endif
> +
> len = m0->m_pkthdr.len;
>
> #if NBPFILTER > 0
> Index: netinet/ip_gre.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_gre.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 ip_gre.c
> --- netinet/ip_gre.c 2 Dec 2015 08:47:00 - 1.58
> +++ netinet/ip_gre.c 7 Feb 2016 23:38:20 -
> @@ -337,6 +337,10 @@ gre_mobile_input(struct mbuf *m, ...)
> bpf_mtap_af(sc->sc_if.if_bpf, AF_INET, m, BPF_DIRECTION_IN);
> #endif
>
> +#if NPF > 0
> + pf_pkt_addr_changed(m);
> +#endif
> +
> niq_enqueue(&ipintrq, m);
> }
>
>
KASSERT() @ pf_test() is back
Hello,
I don't expect to see O.K. to patch below.
The patch is the part of the change, which has been backed out last weekend.
Too many things were wrong so I'm trying to untangle the code a bit.
Patch below is for brave hearts, who don't mind to see KASSERT() to fire:
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.965
diff -u -p -r1.965 pf.c
--- net/pf.c31 Jan 2016 00:18:07 - 1.965
+++ net/pf.c7 Feb 2016 23:38:16 -
@@ -6534,6 +6534,12 @@ done:
if (action == PF_PASS && qid)
pd.m->m_pkthdr.pf.qid = qid;
if (pd.dir == PF_IN && s && s->key[PF_SK_STACK]) {
+ /*
+* ASSERT() below fires whenever caller forgets to call
+* pf_pkt_addr_changed(). This might happen when we deal with
+* IP tunnels.
+*/
+ KASSERT(pd.m->m_pkthdr.pf.statekey == NULL);
pd.m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
}
if (pd.dir == PF_OUT &&
before we get any further with unlocking PF, we need to be sure how packet
handling looks like at 'global level'. Besides the change, which re-introduces
the KASSERT(), I'm adding few more changes, where .pf.statekey should be
removed from mbuf, so KASSERT() won't fire.
My plan is to commit patch as soon as CVS will be unlocked. The change won't
appear in 5.9. I hope to see some KASSERT() panic stories now.
thanks a lot
regards
sasha
8<---8<---8<--8<
Index: net/if_etherip.c
===
RCS file: /cvs/src/sys/net/if_etherip.c,v
retrieving revision 1.5
diff -u -p -r1.5 if_etherip.c
--- net/if_etherip.c25 Jan 2016 05:12:34 - 1.5
+++ net/if_etherip.c7 Feb 2016 23:38:09 -
@@ -499,6 +499,10 @@ ip_etherip_input(struct mbuf *m, ...)
}
m->m_flags &= ~(M_BCAST|M_MCAST);
+#if NPF > 0
+ pf_pkt_addr_changed(m);
+#endif
+
ml_enqueue(&ml, m);
if_input(ifp, &ml);
}
@@ -641,6 +645,10 @@ ip6_etherip_input(struct mbuf **mp, int
}
m->m_flags &= ~(M_BCAST|M_MCAST);
+
+#if NPF > 0
+ pf_pkt_addr_changed(m);
+#endif
ml_enqueue(&ml, m);
if_input(ifp, &ml);
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.965
diff -u -p -r1.965 pf.c
--- net/pf.c31 Jan 2016 00:18:07 - 1.965
+++ net/pf.c7 Feb 2016 23:38:16 -
@@ -6534,6 +6534,12 @@ done:
if (action == PF_PASS && qid)
pd.m->m_pkthdr.pf.qid = qid;
if (pd.dir == PF_IN && s && s->key[PF_SK_STACK]) {
+ /*
+* ASSERT() below fires whenever caller forgets to call
+* pf_pkt_addr_changed(). This might happen when we deal with
+* IP tunnels.
+*/
+ KASSERT(pd.m->m_pkthdr.pf.statekey == NULL);
pd.m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
}
if (pd.dir == PF_OUT &&
Index: net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.84
diff -u -p -r1.84 pipex.c
--- net/pipex.c 3 Nov 2015 21:33:56 - 1.84
+++ net/pipex.c 7 Feb 2016 23:38:18 -
@@ -1139,6 +1139,10 @@ pipex_ip_input(struct mbuf *m0, struct p
goto drop;
}
+#if NPF > 0
+ pf_pkt_addr_changed(m0);
+#endif
+
len = m0->m_pkthdr.len;
#if NBPFILTER > 0
Index: netinet/ip_gre.c
===
RCS file: /cvs/src/sys/netinet/ip_gre.c,v
retrieving revision 1.58
diff -u -p -r1.58 ip_gre.c
--- netinet/ip_gre.c2 Dec 2015 08:47:00 - 1.58
+++ netinet/ip_gre.c7 Feb 2016 23:38:20 -
@@ -337,6 +337,10 @@ gre_mobile_input(struct mbuf *m, ...)
bpf_mtap_af(sc->sc_if.if_bpf, AF_INET, m, BPF_DIRECTION_IN);
#endif
+#if NPF > 0
+ pf_pkt_addr_changed(m);
+#endif
+
niq_enqueue(&ipintrq, m);
}
