KASSERT() @ pf_test() is back

[Alexandr Nedvedicky]

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.c 31 Jan 2016 00:18:07 -0000 1.965
+++ net/pf.c 7 Feb 2016 23:38:16 -0000
@@ -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.c 25 Jan 2016 05:12:34 -0000 1.5
+++ net/if_etherip.c 7 Feb 2016 23:38:09 -0000
@@ -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 -0000 1.965
+++ net/pf.c 7 Feb 2016 23:38:16 -0000
@@ -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 -0000 1.84
+++ net/pipex.c 7 Feb 2016 23:38:18 -0000
@@ -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 -0000 1.58
+++ netinet/ip_gre.c 7 Feb 2016 23:38:20 -0000
@@ -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);
}