Hi,
The functions to link the pf state keys always confused me a bit.
I think the new naming and code of the functions is more consistent.
ok?
bluhm
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.250
diff -u -p -r1.250 uipc_mbuf.c
--- kern/uipc_mbuf.c 12 Oct 2017 09:14:16 -0000 1.250
+++ kern/uipc_mbuf.c 26 Dec 2017 20:15:15 -0000
@@ -307,7 +307,7 @@ m_resethdr(struct mbuf *m)
m_tag_delete_chain(m);
#if NPF > 0
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
#endif /* NPF > 0 */
/* like m_inithdr(), but keep any associated data and mbufs */
@@ -407,7 +407,7 @@ m_free(struct mbuf *m)
if (m->m_flags & M_PKTHDR) {
m_tag_delete_chain(m);
#if NPF > 0
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
#endif /* NPF > 0 */
}
if (m->m_flags & M_EXT)
@@ -1325,7 +1325,7 @@ m_dup_pkthdr(struct mbuf *to, struct mbu
to->m_pkthdr = from->m_pkthdr;
#if NPF > 0
- pf_pkt_state_key_ref(to);
+ pf_mbuf_link_state_key(to, from->m_pkthdr.pf.statekey);
#endif /* NPF > 0 */
SLIST_INIT(&to->m_pkthdr.ph_tags);
Index: net/if.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.531
diff -u -p -r1.531 if.c
--- net/if.c 15 Dec 2017 01:37:30 -0000 1.531
+++ net/if.c 26 Dec 2017 20:15:15 -0000
@@ -697,7 +697,7 @@ if_enqueue(struct ifnet *ifp, struct mbu
#endif
#if NPF > 0
- pf_pkt_unlink_state_key(m);
+ pf_pkt_addr_changed(m);
#endif /* NPF > 0 */
/*
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1051
diff -u -p -r1.1051 pf.c
--- net/pf.c 24 Dec 2017 14:18:19 -0000 1.1051
+++ net/pf.c 26 Dec 2017 20:52:25 -0000
@@ -245,12 +245,17 @@ int pf_match_rule(struct
pf_test_ctx
void pf_counters_inc(int, struct pf_pdesc *,
struct pf_state *, struct pf_rule *,
struct pf_rule *);
-void pf_state_key_link(struct pf_state_key *,
+
+int pf_state_key_isvalid(struct pf_state_key *);
+struct pf_state_key *pf_state_key_ref(struct pf_state_key *);
+void pf_state_key_unref(struct pf_state_key *);
+void pf_state_key_link_reverse(struct pf_state_key *,
struct pf_state_key *);
-void pf_inpcb_unlink_state_key(struct inpcb *);
void pf_state_key_unlink_reverse(struct pf_state_key *);
void pf_state_key_link_inpcb(struct pf_state_key *,
struct inpcb *);
+void pf_state_key_unlink_inpcb(struct pf_state_key *);
+void pf_inpcb_unlink_state_key(struct inpcb *);
#if NPFLOG > 0
void pf_log_matches(struct pf_pdesc *, struct pf_rule *,
@@ -805,9 +810,7 @@ pf_state_key_detach(struct pf_state *s,
RB_REMOVE(pf_state_tree, &pf_statetbl, sk);
sk->removed = 1;
pf_state_key_unlink_reverse(sk);
- pf_inpcb_unlink_state_key(sk->inp);
- sk->inp = NULL;
- pf_state_key_unref(sk);
+ pf_state_key_unlink_inpcb(sk);
}
}
@@ -1060,7 +1063,7 @@ pf_find_state(struct pfi_kif *kif, struc
pkt_sk = m->m_pkthdr.pf.statekey;
if (!pf_state_key_isvalid(pkt_sk)) {
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
pkt_sk = NULL;
}
@@ -1086,7 +1089,7 @@ pf_find_state(struct pfi_kif *kif, struc
return (NULL);
if (dir == PF_OUT && pkt_sk &&
pf_compare_state_keys(pkt_sk, sk, kif, dir) == 0)
- pf_state_key_link(sk, pkt_sk);
+ pf_state_key_link_reverse(sk, pkt_sk);
else if (dir == PF_OUT && m->m_pkthdr.pf.inp &&
!m->m_pkthdr.pf.inp->inp_pf_sk && !sk->inp)
pf_state_key_link_inpcb(sk, m->m_pkthdr.pf.inp);
@@ -7141,7 +7144,7 @@ pf_ouraddr(struct mbuf *m)
void
pf_pkt_addr_changed(struct mbuf *m)
{
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
m->m_pkthdr.pf.inp = NULL;
}
@@ -7152,7 +7155,7 @@ pf_inp_lookup(struct mbuf *m)
struct pf_state_key *sk = m->m_pkthdr.pf.statekey;
if (!pf_state_key_isvalid(sk))
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
else
inp = m->m_pkthdr.pf.statekey->inp;
@@ -7168,7 +7171,7 @@ pf_inp_link(struct mbuf *m, struct inpcb
struct pf_state_key *sk = m->m_pkthdr.pf.statekey;
if (!pf_state_key_isvalid(sk)) {
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
return;
}
@@ -7177,32 +7180,29 @@ pf_inp_link(struct mbuf *m, struct inpcb
* state, which might be just being marked as deleted by another
* thread.
*/
- if (inp && !sk->inp && !inp->inp_pf_sk) {
- sk->inp = inp;
- inp->inp_pf_sk = pf_state_key_ref(sk);
- }
+ if (inp && !sk->inp && !inp->inp_pf_sk)
+ pf_state_key_link_inpcb(sk, inp);
+
/* The statekey has finished finding the inp, it is no longer needed. */
- pf_pkt_unlink_state_key(m);
+ pf_mbuf_unlink_state_key(m);
}
void
pf_inp_unlink(struct inpcb *inp)
{
- if (inp->inp_pf_sk) {
- inp->inp_pf_sk->inp = NULL;
- pf_inpcb_unlink_state_key(inp);
- }
+ pf_inpcb_unlink_state_key(inp);
}
void
-pf_state_key_link(struct pf_state_key *sk, struct pf_state_key *pkt_sk)
+pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev)
{
/*
* Assert will not wire as long as we are called by pf_find_state()
*/
- KASSERT((pkt_sk->reverse == NULL) && (sk->reverse == NULL));
- pkt_sk->reverse = pf_state_key_ref(sk);
- sk->reverse = pf_state_key_ref(pkt_sk);
+ KASSERT(sk->reverse == NULL);
+ sk->reverse = pf_state_key_ref(skrev);
+ KASSERT(skrev->reverse == NULL);
+ skrev->reverse = pf_state_key_ref(sk);
}
#if NPFLOG > 0
@@ -7234,7 +7234,7 @@ pf_state_key_ref(struct pf_state_key *sk
void
pf_state_key_unref(struct pf_state_key *sk)
{
- if ((sk != NULL) && PF_REF_RELE(sk->refcnt)) {
+ if (PF_REF_RELE(sk->refcnt)) {
/* state key must be removed from tree */
KASSERT(!pf_state_key_isvalid(sk));
/* state key must be unlinked from reverse key */
@@ -7252,16 +7252,20 @@ pf_state_key_isvalid(struct pf_state_key
}
void
-pf_pkt_unlink_state_key(struct mbuf *m)
+pf_mbuf_unlink_state_key(struct mbuf *m)
{
- pf_state_key_unref(m->m_pkthdr.pf.statekey);
- m->m_pkthdr.pf.statekey = NULL;
+ struct pf_state_key *sk = m->m_pkthdr.pf.statekey;
+
+ if (sk != NULL) {
+ m->m_pkthdr.pf.statekey = NULL;
+ pf_state_key_unref(sk);
+ }
}
void
-pf_pkt_state_key_ref(struct mbuf *m)
+pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk)
{
- pf_state_key_ref(m->m_pkthdr.pf.statekey);
+ m->m_pkthdr.pf.statekey = pf_state_key_ref(sk);
}
void
@@ -7276,19 +7280,39 @@ pf_state_key_link_inpcb(struct pf_state_
void
pf_inpcb_unlink_state_key(struct inpcb *inp)
{
+ struct pf_state_key *sk = inp->inp_pf_sk;
+
+ if (sk != NULL) {
+ KASSERT(sk->inp == inp);
+ sk->inp = NULL;
+ inp->inp_pf_sk = NULL;
+ pf_state_key_unref(sk);
+ }
+}
+
+void
+pf_state_key_unlink_inpcb(struct pf_state_key *sk)
+{
+ struct inpcb *inp = sk->inp;
+
if (inp != NULL) {
- pf_state_key_unref(inp->inp_pf_sk);
+ KASSERT(inp->inp_pf_sk == sk);
+ sk->inp = NULL;
inp->inp_pf_sk = NULL;
+ pf_state_key_unref(sk);
}
}
void
pf_state_key_unlink_reverse(struct pf_state_key *sk)
{
- if ((sk != NULL) && (sk->reverse != NULL)) {
- pf_state_key_unref(sk->reverse->reverse);
- sk->reverse->reverse = NULL;
- pf_state_key_unref(sk->reverse);
+ struct pf_state_key *skrev = sk->reverse;
+
+ if (skrev != NULL) {
+ KASSERT(skrev->reverse == sk);
sk->reverse = NULL;
+ skrev->reverse = NULL;
+ pf_state_key_unref(skrev);
+ pf_state_key_unref(sk);
}
}
Index: net/pfvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.469
diff -u -p -r1.469 pfvar.h
--- net/pfvar.h 28 Nov 2017 16:05:46 -0000 1.469
+++ net/pfvar.h 26 Dec 2017 20:15:15 -0000
@@ -1905,11 +1905,9 @@ int pf_map_addr(sa_family_t,
struct p
struct pf_pool *, enum pf_sn_types);
int pf_postprocess_addr(struct pf_state *);
-struct pf_state_key *pf_state_key_ref(struct pf_state_key *);
-void pf_state_key_unref(struct pf_state_key *);
-int pf_state_key_isvalid(struct pf_state_key *);
-void pf_pkt_unlink_state_key(struct mbuf *);
-void pf_pkt_state_key_ref(struct mbuf *);
+void pf_mbuf_link_state_key(struct mbuf *,
+ struct pf_state_key *);
+void pf_mbuf_unlink_state_key(struct mbuf *);
u_int8_t pf_get_wscale(struct pf_pdesc *);
u_int16_t pf_get_mss(struct pf_pdesc *);
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.335
diff -u -p -r1.335 ip_input.c
--- netinet/ip_input.c 4 Dec 2017 13:40:34 -0000 1.335
+++ netinet/ip_input.c 26 Dec 2017 20:15:15 -0000
@@ -1459,7 +1459,7 @@ ip_forward(struct mbuf *m, struct ifnet
m_copydata(m, 0, len, mfake.m_pktdat);
mfake.m_pkthdr.len = mfake.m_len = len;
#if NPF > 0
- pf_pkt_unlink_state_key(&mfake);
+ pf_pkt_addr_changed(&mfake);
#endif /* NPF > 0 */
fake = 1;
}
