git: 7cd3854f827f - main - pf: Fix interface counters for af-to rules
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 28 Sep 2025 17:25:45 UTC
The branch main has been updated by ks:
URL: https://cgit.FreeBSD.org/src/commit/?id=7cd3854f827faaad1ecf414d20bdf6802cfa60f8
commit 7cd3854f827faaad1ecf414d20bdf6802cfa60f8
Author: Kajetan Staszkiewicz <ks@FreeBSD.org>
AuthorDate: 2025-09-08 17:53:48 +0000
Commit: Kajetan Staszkiewicz <ks@FreeBSD.org>
CommitDate: 2025-09-28 17:23:02 +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
---
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 9f250476cb75..2c6d62078e6a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -9231,25 +9231,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)
@@ -9548,26 +9561,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)
@@ -10720,21 +10746,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;
}
@@ -10745,22 +10782,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;
@@ -10792,7 +10839,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]);
@@ -10803,7 +10850,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 */
@@ -10819,12 +10866,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()