From: dlg Date: Wed, 21 Dec 2022 02:23:10 +0000 (+0000) Subject: prefix pf_state_key and pf_state_item struct bits to make them more unique. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=35751c747c9f1e1c8a27869d6c2516a9a1b31ef9;p=openbsd prefix pf_state_key and pf_state_item struct bits to make them more unique. this makes searching for the struct members easier, which in turn makes tweaking code around them a lot easier too. sk_refcnt in particular would have been a lot nicer to fiddle with than just refcnt because pf_state structs also have a refcnt, which is annoying. tweaks and ok sashan@ reads ok kn@ --- diff --git a/sys/net/pf.c b/sys/net/pf.c index 7203589d4bd..2086c18a14f 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1157 2022/12/16 02:05:44 dlg Exp $ */ +/* $OpenBSD: pf.c,v 1.1158 2022/12/21 02:23:10 dlg Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -315,7 +315,7 @@ struct pf_state_tree_id tree_id; struct pf_state_list pf_state_list = PF_STATE_LIST_INITIALIZER(pf_state_list); RB_GENERATE(pf_src_tree, pf_src_node, entry, pf_src_compare); -RB_GENERATE(pf_state_tree, pf_state_key, entry, pf_state_compare_key); +RB_GENERATE(pf_state_tree, pf_state_key, sk_entry, pf_state_compare_key); RB_GENERATE(pf_state_tree_id, pf_state, entry_id, pf_state_compare_id); @@ -736,24 +736,25 @@ pf_state_key_attach(struct pf_state_key *sk, struct pf_state *s, int idx) PF_ASSERT_LOCKED(); KASSERT(s->key[idx] == NULL); - sk->removed = 0; + sk->sk_removed = 0; cur = RB_INSERT(pf_state_tree, &pf_statetbl, sk); if (cur != NULL) { - sk->removed = 1; + sk->sk_removed = 1; /* key exists. check for same kif, if none, add to key */ - TAILQ_FOREACH(si, &cur->states, entry) { - if (si->s->kif == s->kif && - ((si->s->key[PF_SK_WIRE]->af == sk->af && - si->s->direction == s->direction) || - (si->s->key[PF_SK_WIRE]->af != - si->s->key[PF_SK_STACK]->af && - sk->af == si->s->key[PF_SK_STACK]->af && - si->s->direction != s->direction))) { + TAILQ_FOREACH(si, &cur->sk_states, si_entry) { + struct pf_state *sist = si->si_st; + if (sist->kif == s->kif && + ((sist->key[PF_SK_WIRE]->af == sk->af && + sist->direction == s->direction) || + (sist->key[PF_SK_WIRE]->af != + sist->key[PF_SK_STACK]->af && + sk->af == sist->key[PF_SK_STACK]->af && + sist->direction != s->direction))) { int reuse = 0; if (sk->proto == IPPROTO_TCP && - si->s->src.state >= TCPS_FIN_WAIT_2 && - si->s->dst.state >= TCPS_FIN_WAIT_2) + sist->src.state >= TCPS_FIN_WAIT_2 && + sist->dst.state >= TCPS_FIN_WAIT_2) reuse = 1; if (pf_status.debug >= LOG_NOTICE) { log(LOG_NOTICE, @@ -766,16 +767,16 @@ pf_state_key_attach(struct pf_state_key *sk, struct pf_state *s, int idx) (idx == PF_SK_WIRE) ? sk : NULL, (idx == PF_SK_STACK) ? sk : NULL); addlog(", existing: "); - pf_print_state_parts(si->s, + pf_print_state_parts(sist, (idx == PF_SK_WIRE) ? sk : NULL, (idx == PF_SK_STACK) ? sk : NULL); addlog("\n"); } if (reuse) { - pf_set_protostate(si->s, PF_PEER_BOTH, + pf_set_protostate(sist, PF_PEER_BOTH, TCPS_CLOSED); /* remove late or sks can go away */ - olds = si->s; + olds = sist; } else { pf_state_key_unref(sk); return (NULL); /* collision! */ @@ -789,10 +790,10 @@ pf_state_key_attach(struct pf_state_key *sk, struct pf_state *s, int idx) } if ((si = pool_get(&pf_state_item_pl, PR_NOWAIT)) == NULL) { - if (TAILQ_EMPTY(&sk->states)) { + if (TAILQ_EMPTY(&sk->sk_states)) { KASSERT(cur == NULL); RB_REMOVE(pf_state_tree, &pf_statetbl, sk); - sk->removed = 1; + sk->sk_removed = 1; pf_state_key_unref(sk); } @@ -800,13 +801,13 @@ pf_state_key_attach(struct pf_state_key *sk, struct pf_state *s, int idx) } s->key[idx] = pf_state_key_ref(sk); /* give a ref to state */ - si->s = pf_state_ref(s); + si->si_st = pf_state_ref(s); /* list is sorted, if-bound states before floating */ if (s->kif == pfi_all) - TAILQ_INSERT_TAIL(&sk->states, si, entry); + TAILQ_INSERT_TAIL(&sk->sk_states, si, si_entry); else - TAILQ_INSERT_HEAD(&sk->states, si, entry); + TAILQ_INSERT_HEAD(&sk->sk_states, si, si_entry); if (olds) pf_remove_state(olds); @@ -838,19 +839,19 @@ pf_state_key_detach(struct pf_state *s, int idx) if (sk == NULL) return; - TAILQ_FOREACH(si, &sk->states, entry) { - if (si->s == s) + TAILQ_FOREACH(si, &sk->sk_states, si_entry) { + if (si->si_st == s) break; } if (si == NULL) return; - TAILQ_REMOVE(&sk->states, si, entry); + TAILQ_REMOVE(&sk->sk_states, si, si_entry); pool_put(&pf_state_item_pl, si); - if (TAILQ_EMPTY(&sk->states)) { + if (TAILQ_EMPTY(&sk->sk_states)) { RB_REMOVE(pf_state_tree, &pf_statetbl, sk); - sk->removed = 1; + sk->sk_removed = 1; pf_state_key_unlink_reverse(sk); pf_state_key_unlink_inpcb(sk); pf_state_key_unref(sk); @@ -867,9 +868,9 @@ pf_alloc_state_key(int pool_flags) if ((sk = pool_get(&pf_state_key_pl, pool_flags)) == NULL) return (NULL); - PF_REF_INIT(sk->refcnt); - TAILQ_INIT(&sk->states); - sk->removed = 1; + PF_REF_INIT(sk->sk_refcnt); + TAILQ_INIT(&sk->sk_states); + sk->sk_removed = 1; return (sk); } @@ -1137,8 +1138,8 @@ pf_find_state(struct pf_pdesc *pd, struct pf_state_key_cmp *key, pkt_sk = NULL; } - if (pkt_sk && pf_state_key_isvalid(pkt_sk->reverse)) - sk = pkt_sk->reverse; + if (pkt_sk && pf_state_key_isvalid(pkt_sk->sk_reverse)) + sk = pkt_sk->sk_reverse; if (pkt_sk == NULL) { /* here we deal with local outbound packet */ @@ -1161,7 +1162,7 @@ pf_find_state(struct pf_pdesc *pd, struct pf_state_key_cmp *key, pf_compare_state_keys(pkt_sk, sk, pd->kif, pd->dir) == 0) pf_state_key_link_reverse(sk, pkt_sk); else if (pd->dir == PF_OUT && pd->m->m_pkthdr.pf.inp && - !pd->m->m_pkthdr.pf.inp->inp_pf_sk && !sk->inp) + !pd->m->m_pkthdr.pf.inp->inp_pf_sk && !sk->sk_inp) pf_state_key_link_inpcb(sk, pd->m->m_pkthdr.pf.inp); } @@ -1170,17 +1171,19 @@ pf_find_state(struct pf_pdesc *pd, struct pf_state_key_cmp *key, pf_pkt_addr_changed(pd->m); /* list is sorted, if-bound states before floating ones */ - TAILQ_FOREACH(si, &sk->states, entry) - if (si->s->timeout != PFTM_PURGE && - (si->s->kif == pfi_all || si->s->kif == pd->kif) && - ((si->s->key[PF_SK_WIRE]->af == si->s->key[PF_SK_STACK]->af - && sk == (pd->dir == PF_IN ? si->s->key[PF_SK_WIRE] : - si->s->key[PF_SK_STACK])) || - (si->s->key[PF_SK_WIRE]->af != si->s->key[PF_SK_STACK]->af - && pd->dir == PF_IN && (sk == si->s->key[PF_SK_STACK] || - sk == si->s->key[PF_SK_WIRE])))) { - s = si->s; + TAILQ_FOREACH(si, &sk->sk_states, si_entry) { + struct pf_state *sist = si->si_st; + if (sist->timeout != PFTM_PURGE && + (sist->kif == pfi_all || sist->kif == pd->kif) && + ((sist->key[PF_SK_WIRE]->af == sist->key[PF_SK_STACK]->af && + sk == (pd->dir == PF_IN ? sist->key[PF_SK_WIRE] : + sist->key[PF_SK_STACK])) || + (sist->key[PF_SK_WIRE]->af != sist->key[PF_SK_STACK]->af + && pd->dir == PF_IN && (sk == sist->key[PF_SK_STACK] || + sk == sist->key[PF_SK_WIRE])))) { + s = sist; break; + } } if (s == NULL) @@ -1210,20 +1213,22 @@ pf_find_state_all(struct pf_state_key_cmp *key, u_int dir, int *more) sk = RB_FIND(pf_state_tree, &pf_statetbl, (struct pf_state_key *)key); if (sk != NULL) { - TAILQ_FOREACH(si, &sk->states, entry) + TAILQ_FOREACH(si, &sk->sk_states, si_entry) { + struct pf_state *sist = si->si_st; if (dir == PF_INOUT || - (sk == (dir == PF_IN ? si->s->key[PF_SK_WIRE] : - si->s->key[PF_SK_STACK]))) { + (sk == (dir == PF_IN ? sist->key[PF_SK_WIRE] : + sist->key[PF_SK_STACK]))) { if (more == NULL) - return (si->s); + return (sist); if (ret) (*more)++; else ret = si; } + } } - return (ret ? ret->s : NULL); + return (ret ? ret->si_st : NULL); } void @@ -1732,27 +1737,28 @@ pf_remove_divert_state(struct pf_state_key *sk) PF_LOCK(); PF_STATE_ENTER_WRITE(); - TAILQ_FOREACH(si, &sk->states, entry) { - if (sk == si->s->key[PF_SK_STACK] && si->s->rule.ptr && - (si->s->rule.ptr->divert.type == PF_DIVERT_TO || - si->s->rule.ptr->divert.type == PF_DIVERT_REPLY)) { - if (si->s->key[PF_SK_STACK]->proto == IPPROTO_TCP && - si->s->key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]) { + TAILQ_FOREACH(si, &sk->sk_states, si_entry) { + struct pf_state *sist = si->si_st; + if (sk == sist->key[PF_SK_STACK] && sist->rule.ptr && + (sist->rule.ptr->divert.type == PF_DIVERT_TO || + sist->rule.ptr->divert.type == PF_DIVERT_REPLY)) { + if (sist->key[PF_SK_STACK]->proto == IPPROTO_TCP && + sist->key[PF_SK_WIRE] != sist->key[PF_SK_STACK]) { /* * If the local address is translated, keep * the state for "tcp.closed" seconds to * prevent its source port from being reused. */ - if (si->s->src.state < TCPS_FIN_WAIT_2 || - si->s->dst.state < TCPS_FIN_WAIT_2) { - pf_set_protostate(si->s, PF_PEER_BOTH, + if (sist->src.state < TCPS_FIN_WAIT_2 || + sist->dst.state < TCPS_FIN_WAIT_2) { + pf_set_protostate(sist, PF_PEER_BOTH, TCPS_TIME_WAIT); - si->s->timeout = PFTM_TCP_CLOSED; - si->s->expire = getuptime(); + sist->timeout = PFTM_TCP_CLOSED; + sist->expire = getuptime(); } - si->s->state_flags |= PFSTATE_INP_UNLINKED; + sist->state_flags |= PFSTATE_INP_UNLINKED; } else - pf_remove_state(si->s); + pf_remove_state(sist); break; } } @@ -7605,7 +7611,7 @@ done: pf_mbuf_link_state_key(pd.m, s->key[PF_SK_STACK]); if (pd.dir == PF_OUT && pd.m->m_pkthdr.pf.inp && !pd.m->m_pkthdr.pf.inp->inp_pf_sk && - s && s->key[PF_SK_STACK] && !s->key[PF_SK_STACK]->inp) + s && s->key[PF_SK_STACK] && !s->key[PF_SK_STACK]->sk_inp) pf_state_key_link_inpcb(s->key[PF_SK_STACK], pd.m->m_pkthdr.pf.inp); @@ -7784,7 +7790,7 @@ pf_ouraddr(struct mbuf *m) sk = m->m_pkthdr.pf.statekey; if (sk != NULL) { - if (sk->inp != NULL) + if (sk->sk_inp != NULL) return (1); } @@ -7811,7 +7817,7 @@ pf_inp_lookup(struct mbuf *m) if (!pf_state_key_isvalid(sk)) pf_mbuf_unlink_state_key(m); else - inp = m->m_pkthdr.pf.statekey->inp; + inp = m->m_pkthdr.pf.statekey->sk_inp; if (inp && inp->inp_pf_sk) KASSERT(m->m_pkthdr.pf.statekey == inp->inp_pf_sk); @@ -7835,7 +7841,7 @@ pf_inp_link(struct mbuf *m, struct inpcb *inp) * state, which might be just being marked as deleted by another * thread. */ - if (inp && !sk->inp && !inp->inp_pf_sk) + if (inp && !sk->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. */ @@ -7853,7 +7859,7 @@ pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev) { struct pf_state_key *old_reverse; - old_reverse = atomic_cas_ptr(&sk->reverse, NULL, skrev); + old_reverse = atomic_cas_ptr(&sk->sk_reverse, NULL, skrev); if (old_reverse != NULL) KASSERT(old_reverse == skrev); else { @@ -7865,7 +7871,7 @@ pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev) * pf_state_key_unlink_reverse() does not check whether keys * are identical or not. */ - old_reverse = atomic_cas_ptr(&skrev->reverse, NULL, sk); + old_reverse = atomic_cas_ptr(&skrev->sk_reverse, NULL, sk); if (old_reverse != NULL) KASSERT(old_reverse == sk); @@ -7894,7 +7900,7 @@ struct pf_state_key * pf_state_key_ref(struct pf_state_key *sk) { if (sk != NULL) - PF_REF_TAKE(sk->refcnt); + PF_REF_TAKE(sk->sk_refcnt); return (sk); } @@ -7902,13 +7908,13 @@ pf_state_key_ref(struct pf_state_key *sk) void pf_state_key_unref(struct pf_state_key *sk) { - if (PF_REF_RELE(sk->refcnt)) { + if (PF_REF_RELE(sk->sk_refcnt)) { /* state key must be removed from tree */ KASSERT(!pf_state_key_isvalid(sk)); /* state key must be unlinked from reverse key */ - KASSERT(sk->reverse == NULL); + KASSERT(sk->sk_reverse == NULL); /* state key must be unlinked from socket */ - KASSERT(sk->inp == NULL); + KASSERT(sk->sk_inp == NULL); pool_put(&pf_state_key_pl, sk); } } @@ -7916,7 +7922,7 @@ pf_state_key_unref(struct pf_state_key *sk) int pf_state_key_isvalid(struct pf_state_key *sk) { - return ((sk != NULL) && (sk->removed == 0)); + return ((sk != NULL) && (sk->sk_removed == 0)); } void @@ -7958,8 +7964,8 @@ pf_mbuf_unlink_inpcb(struct mbuf *m) void pf_state_key_link_inpcb(struct pf_state_key *sk, struct inpcb *inp) { - KASSERT(sk->inp == NULL); - sk->inp = in_pcbref(inp); + KASSERT(sk->sk_inp == NULL); + sk->sk_inp = in_pcbref(inp); KASSERT(inp->inp_pf_sk == NULL); inp->inp_pf_sk = pf_state_key_ref(sk); } @@ -7970,8 +7976,8 @@ 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; + KASSERT(sk->sk_inp == inp); + sk->sk_inp = NULL; inp->inp_pf_sk = NULL; pf_state_key_unref(sk); in_pcbunref(inp); @@ -7981,11 +7987,11 @@ pf_inpcb_unlink_state_key(struct inpcb *inp) void pf_state_key_unlink_inpcb(struct pf_state_key *sk) { - struct inpcb *inp = sk->inp; + struct inpcb *inp = sk->sk_inp; if (inp != NULL) { KASSERT(inp->inp_pf_sk == sk); - sk->inp = NULL; + sk->sk_inp = NULL; inp->inp_pf_sk = NULL; pf_state_key_unref(sk); in_pcbunref(inp); @@ -7995,13 +8001,13 @@ pf_state_key_unlink_inpcb(struct pf_state_key *sk) void pf_state_key_unlink_reverse(struct pf_state_key *sk) { - struct pf_state_key *skrev = sk->reverse; + struct pf_state_key *skrev = sk->sk_reverse; /* Note that sk and skrev may be equal, then we unref twice. */ if (skrev != NULL) { - KASSERT(skrev->reverse == sk); - sk->reverse = NULL; - skrev->reverse = NULL; + KASSERT(skrev->sk_reverse == sk); + sk->sk_reverse = NULL; + skrev->sk_reverse = NULL; pf_state_key_unref(skrev); pf_state_key_unref(sk); } diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c index a8cf59e0aae..e1081f3b931 100644 --- a/sys/net/pf_ioctl.c +++ b/sys/net/pf_ioctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_ioctl.c,v 1.392 2022/11/25 20:27:53 bluhm Exp $ */ +/* $OpenBSD: pf_ioctl.c,v 1.393 2022/12/21 02:23:10 dlg Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1762,24 +1762,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) if (sk == NULL) continue; - TAILQ_FOREACH_SAFE(si, &sk->states, entry, sit) - if (((si->s->key[PF_SK_WIRE]->af == - si->s->key[PF_SK_STACK]->af && + TAILQ_FOREACH_SAFE(si, &sk->sk_states, + si_entry, sit) { + struct pf_state *sist = si->si_st; + if (((sist->key[PF_SK_WIRE]->af == + sist->key[PF_SK_STACK]->af && sk == (dirs[i] == PF_IN ? - si->s->key[PF_SK_WIRE] : - si->s->key[PF_SK_STACK])) || - (si->s->key[PF_SK_WIRE]->af != - si->s->key[PF_SK_STACK]->af && + sist->key[PF_SK_WIRE] : + sist->key[PF_SK_STACK])) || + (sist->key[PF_SK_WIRE]->af != + sist->key[PF_SK_STACK]->af && dirs[i] == PF_IN && - (sk == si->s->key[PF_SK_STACK] || - sk == si->s->key[PF_SK_WIRE]))) && + (sk == sist->key[PF_SK_STACK] || + sk == sist->key[PF_SK_WIRE]))) && (!psk->psk_ifname[0] || - (si->s->kif != pfi_all && + (sist->kif != pfi_all && !strcmp(psk->psk_ifname, - si->s->kif->pfik_name)))) { - pf_remove_state(si->s); + sist->kif->pfik_name)))) { + pf_remove_state(sist); killed++; } + } } if (killed) psk->psk_killed = killed; diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 52d18b4545d..719ad0a8728 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar.h,v 1.523 2022/12/19 04:35:33 dlg Exp $ */ +/* $OpenBSD: pfvar.h,v 1.524 2022/12/21 02:23:10 dlg Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1033,7 +1033,7 @@ struct pfr_ktable { #define pfrkt_tzero pfrkt_ts.pfrts_tzero RB_HEAD(pf_state_tree, pf_state_key); -RB_PROTOTYPE(pf_state_tree, pf_state_key, entry, pf_state_compare_key) +RB_PROTOTYPE(pf_state_tree, pf_state_key, sk_entry, pf_state_compare_key) RB_HEAD(pf_state_tree_ext_gwy, pf_state_key); RB_PROTOTYPE(pf_state_tree_ext_gwy, pf_state_key, diff --git a/sys/net/pfvar_priv.h b/sys/net/pfvar_priv.h index b9114c341cc..cd5b308e4dc 100644 --- a/sys/net/pfvar_priv.h +++ b/sys/net/pfvar_priv.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar_priv.h,v 1.25 2022/12/19 04:35:34 dlg Exp $ */ +/* $OpenBSD: pfvar_priv.h,v 1.26 2022/12/21 02:23:10 dlg Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -41,8 +41,9 @@ #include struct pf_state_item { - TAILQ_ENTRY(pf_state_item) entry; - struct pf_state *s; + TAILQ_ENTRY(pf_state_item) + si_entry; + struct pf_state *si_st; }; TAILQ_HEAD(pf_statelisthead, pf_state_item); @@ -54,12 +55,12 @@ struct pf_state_key { sa_family_t af; u_int8_t proto; - RB_ENTRY(pf_state_key) entry; - struct pf_statelisthead states; - struct pf_state_key *reverse; - struct inpcb *inp; - pf_refcnt_t refcnt; - u_int8_t removed; + RB_ENTRY(pf_state_key) sk_entry; + struct pf_statelisthead sk_states; + struct pf_state_key *sk_reverse; + struct inpcb *sk_inp; + pf_refcnt_t sk_refcnt; + u_int8_t sk_removed; }; #define PF_REVERSED_KEY(key, family) \ ((key[PF_SK_WIRE]->af != key[PF_SK_STACK]->af) && \