The branch stable/15 has been updated by ks: URL: https://cgit.FreeBSD.org/src/commit/?id=ad428a318b60c0278a146b04fbc7520c322e55ef
commit ad428a318b60c0278a146b04fbc7520c322e55ef Author: Kajetan Staszkiewicz <[email protected]> AuthorDate: 2025-09-08 17:53:48 +0000 Commit: Kajetan Staszkiewicz <[email protected]> CommitDate: 2025-10-01 16:00:47 +0000 pf: Fix interface counters for af-to rules An inbound af-to rule creates a state bypassing outbound pf_test(). In such case increase counters of the outbound interface directly in pf_route() for post-af-to address family. For outbound af-to rules ensure that post-af-to address family is used to increase interface counters. Reviewed by: kp Sponsored by: InnoGames GmbH Differential Revision: https://reviews.freebsd.org/D52448 (cherry picked from commit 7cd3854f827faaad1ecf414d20bdf6802cfa60f8) --- sys/netpfil/pf/pf.c | 151 +++++++++++++++++++++++++-------------- tests/sys/netpfil/pf/counters.sh | 18 ++++- 2 files changed, 115 insertions(+), 54 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 66096133acd4..fb13f22bf0b1 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -9203,25 +9203,38 @@ pf_route(struct pf_krule *r, struct ifnet *oifp, if (r->rt == PF_DUPTO || (pd->af != pd->naf && s->direction == PF_IN)) skip_test = true; - if (pd->dir == PF_IN && !skip_test) { - if (pf_test(AF_INET, PF_OUT, PFIL_FWD, ifp, &m0, inp, - &pd->act) != PF_PASS) { - action = PF_DROP; - SDT_PROBE1(pf, ip, route_to, drop, __LINE__); - goto bad; - } else if (m0 == NULL) { - action = PF_DROP; - SDT_PROBE1(pf, ip, route_to, drop, __LINE__); - goto done; - } - if (m0->m_len < sizeof(struct ip)) { - DPFPRINTF(PF_DEBUG_URGENT, - "%s: m0->m_len < sizeof(struct ip)", __func__); - SDT_PROBE1(pf, ip, route_to, drop, __LINE__); - action = PF_DROP; - goto bad; + if (pd->dir == PF_IN) { + if (skip_test) { + struct pfi_kkif *out_kif = (struct pfi_kkif *)ifp->if_pf_kif; + MPASS(s != NULL); + pf_counter_u64_critical_enter(); + pf_counter_u64_add_protected( + &out_kif->pfik_bytes[pd->naf == AF_INET6][1] + [action != PF_PASS && action != PF_AFRT], pd->tot_len); + pf_counter_u64_add_protected( + &out_kif->pfik_packets[pd->naf == AF_INET6][1] + [action != PF_PASS && action != PF_AFRT], 1); + pf_counter_u64_critical_exit(); + } else { + if (pf_test(AF_INET, PF_OUT, PFIL_FWD, ifp, &m0, inp, + &pd->act) != PF_PASS) { + action = PF_DROP; + SDT_PROBE1(pf, ip, route_to, drop, __LINE__); + goto bad; + } else if (m0 == NULL) { + action = PF_DROP; + SDT_PROBE1(pf, ip, route_to, drop, __LINE__); + goto done; + } + if (m0->m_len < sizeof(struct ip)) { + DPFPRINTF(PF_DEBUG_URGENT, + "%s: m0->m_len < sizeof(struct ip)", __func__); + SDT_PROBE1(pf, ip, route_to, drop, __LINE__); + action = PF_DROP; + goto bad; + } + ip = mtod(m0, struct ip *); } - ip = mtod(m0, struct ip *); } if (ifp->if_flags & IFF_LOOPBACK) @@ -9520,26 +9533,39 @@ pf_route6(struct pf_krule *r, struct ifnet *oifp, if (r->rt == PF_DUPTO || (pd->af != pd->naf && s->direction == PF_IN)) skip_test = true; - if (pd->dir == PF_IN && !skip_test) { - if (pf_test(AF_INET6, PF_OUT, PFIL_FWD | PF_PFIL_NOREFRAGMENT, - ifp, &m0, inp, &pd->act) != PF_PASS) { - action = PF_DROP; - SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); - goto bad; - } else if (m0 == NULL) { - action = PF_DROP; - SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); - goto done; - } - if (m0->m_len < sizeof(struct ip6_hdr)) { - DPFPRINTF(PF_DEBUG_URGENT, - "%s: m0->m_len < sizeof(struct ip6_hdr)", - __func__); - action = PF_DROP; - SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); - goto bad; + if (pd->dir == PF_IN) { + if (skip_test) { + struct pfi_kkif *out_kif = (struct pfi_kkif *)ifp->if_pf_kif; + MPASS(s != NULL); + pf_counter_u64_critical_enter(); + pf_counter_u64_add_protected( + &out_kif->pfik_bytes[pd->naf == AF_INET6][1] + [action != PF_PASS && action != PF_AFRT], pd->tot_len); + pf_counter_u64_add_protected( + &out_kif->pfik_packets[pd->naf == AF_INET6][1] + [action != PF_PASS && action != PF_AFRT], 1); + pf_counter_u64_critical_exit(); + } else { + if (pf_test(AF_INET6, PF_OUT, PFIL_FWD | PF_PFIL_NOREFRAGMENT, + ifp, &m0, inp, &pd->act) != PF_PASS) { + action = PF_DROP; + SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); + goto bad; + } else if (m0 == NULL) { + action = PF_DROP; + SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); + goto done; + } + if (m0->m_len < sizeof(struct ip6_hdr)) { + DPFPRINTF(PF_DEBUG_URGENT, + "%s: m0->m_len < sizeof(struct ip6_hdr)", + __func__); + action = PF_DROP; + SDT_PROBE1(pf, ip6, route_to, drop, __LINE__); + goto bad; + } + ip6 = mtod(m0, struct ip6_hdr *); } - ip6 = mtod(m0, struct ip6_hdr *); } if (ifp->if_flags & IFF_LOOPBACK) @@ -10665,21 +10691,32 @@ pf_counters_inc(int action, struct pf_pdesc *pd, struct pf_kstate *s, struct pf_addr *dst_host = pd->dst; struct pf_state_key *key; int dir_out = (pd->dir == PF_OUT); - int op_pass = (r->action == PF_PASS); - sa_family_t af = pd->af; + int op_r_pass = (r->action == PF_PASS); + int op_pass = (action == PF_PASS || action == PF_AFRT); int s_dir_in, s_dir_out, s_dir_rev; + sa_family_t af = pd->af; pf_counter_u64_critical_enter(); + /* + * Set AF for interface counters, it will be later overwritten for + * rule and state counters with value from proper state key. + */ + if (action == PF_AFRT) { + MPASS(s != NULL); + if (s->direction == PF_OUT && dir_out) + af = pd->naf; + } + pf_counter_u64_add_protected( - &pd->kif->pfik_bytes[pd->af == AF_INET6][dir_out][action != PF_PASS], + &pd->kif->pfik_bytes[af == AF_INET6][dir_out][!op_pass], pd->tot_len); pf_counter_u64_add_protected( - &pd->kif->pfik_packets[pd->af == AF_INET6][dir_out][action != PF_PASS], + &pd->kif->pfik_packets[af == AF_INET6][dir_out][!op_pass], 1); /* If the rule has failed to apply, don't increase its counters */ - if (!(action == PF_PASS || action == PF_AFRT || r->action == PF_DROP)) { + if (!(op_pass || r->action == PF_DROP)) { pf_counter_u64_critical_exit(); return; } @@ -10690,22 +10727,32 @@ pf_counters_inc(int action, struct pf_pdesc *pd, struct pf_kstate *s, /* * For af-to on the inbound direction we can determine - * the direction only by checking direction of AF translation, - * since the state is always "in" and so is packet's direction. + * the direction of passing packet only by checking direction + * of AF translation. The af-to in "in" direction covers both + * the inbound and the outbound side of state tracking, + * so pd->dir is always PF_IN. We set dir_out and s_dir_rev + * in a way to count packets as if the state was outbound, + * because pfctl -ss shows the state with "->", as if it was + * oubound. */ - if (pd->af != pd->naf && s->direction == PF_IN) { + if (action == PF_AFRT && s->direction == PF_IN) { dir_out = (pd->naf == s->rule->naf); s_dir_in = 1; s_dir_out = 0; - s_dir_rev = (pd->naf != s->rule->naf); - } - else { + s_dir_rev = (pd->naf == s->rule->af); + } else { dir_out = (pd->dir == PF_OUT); s_dir_in = (s->direction == PF_IN); s_dir_out = (s->direction == PF_OUT); s_dir_rev = (pd->dir != s->direction); } + /* pd->tot_len is a problematic with af-to rules. Sure, we can + * agree that it's the post-af-to packet length that was + * forwarded through a state, but what about tables which match + * on pre-af-to addresses? We don't have access the the original + * packet length anymore. + */ s->packets[s_dir_rev]++; s->bytes[s_dir_rev] += pd->tot_len; @@ -10737,7 +10784,7 @@ pf_counters_inc(int action, struct pf_pdesc *pd, struct pf_kstate *s, s->nat_rule->action == PF_BINAT) { nr = s->nat_rule; pf_rule_counters_inc(pd, s->nat_rule, dir_out, - op_pass, af, src_host, dst_host); + op_r_pass, af, src_host, dst_host); /* Use post-NAT addresses from now on */ key = s->key[s_dir_in]; src_host = &(key->addr[s_dir_out]); @@ -10748,7 +10795,7 @@ pf_counters_inc(int action, struct pf_pdesc *pd, struct pf_kstate *s, } SLIST_FOREACH(ri, mr, entry) { - pf_rule_counters_inc(pd, ri->r, dir_out, op_pass, af, + pf_rule_counters_inc(pd, ri->r, dir_out, op_r_pass, af, src_host, dst_host); if (s && s->nat_rule == ri->r) { /* Use post-NAT addresses after a match NAT rule */ @@ -10764,12 +10811,12 @@ pf_counters_inc(int action, struct pf_pdesc *pd, struct pf_kstate *s, } if (a != NULL) { - pf_rule_counters_inc(pd, a, dir_out, op_pass, af, + pf_rule_counters_inc(pd, a, dir_out, op_r_pass, af, src_host, dst_host); } if (r != nr) { - pf_rule_counters_inc(pd, r, dir_out, op_pass, af, + pf_rule_counters_inc(pd, r, dir_out, op_r_pass, af, src_host, dst_host); } diff --git a/tests/sys/netpfil/pf/counters.sh b/tests/sys/netpfil/pf/counters.sh index a0119b4710c1..20d7dc3c6d89 100644 --- a/tests/sys/netpfil/pf/counters.sh +++ b/tests/sys/netpfil/pf/counters.sh @@ -684,7 +684,7 @@ nat64_in_body() "table <tbl_dst_match> { 64:ff9b::${net_server1_4_host_server} }" \ "table <tbl_src_pass> { ${net_tester_6_host_tester} }" \ "table <tbl_dst_pass> { 64:ff9b::${net_server1_4_host_server} }" \ - "block" \ + "block log" \ "pass inet6 proto icmp6 icmp6-type { neighbrsol, neighbradv }" \ "match in on ${epair_tester}b inet6 proto tcp from <tbl_src_match> to <tbl_dst_match> scrub (random-id)" \ "pass in on ${epair_tester}b inet6 proto tcp from <tbl_src_pass> to <tbl_dst_pass> \ @@ -726,6 +726,13 @@ nat64_in_body() ; do grep -qE "${state_regexp}" $states || atf_fail "State not found for '${state_regexp}'" done + + echo " === interfaces === " + echo " === tester === " + jexec router pfctl -qvvsI -i ${epair_tester}b + echo " === server === " + jexec router pfctl -qvvsI -i ${epair_server1}a + echo " === " } nat64_in_cleanup() @@ -753,7 +760,7 @@ nat64_out_body() "table <tbl_dst_match> { 64:ff9b::${net_server1_4_host_server} }" \ "table <tbl_src_pass> { ${net_tester_6_host_tester} }" \ "table <tbl_dst_pass> { 64:ff9b::${net_server1_4_host_server} }" \ - "block" \ + "block log " \ "pass inet6 proto icmp6 icmp6-type { neighbrsol, neighbradv }" \ "pass in on ${epair_tester}b inet6 proto tcp keep state" \ "match out on ${epair_server1}a inet6 proto tcp from <tbl_src_match> to <tbl_dst_match> scrub (random-id)" \ @@ -794,6 +801,13 @@ nat64_out_body() ; do grep -qE "${state_regexp}" $states || atf_fail "State not found for '${state_regexp}'" done + + echo " === interfaces === " + echo " === tester === " + jexec router pfctl -qvvsI -i ${epair_tester}b + echo " === server === " + jexec router pfctl -qvvsI -i ${epair_server1}a + echo " === " } nat64_out_cleanup()
