From nobody Sun Sep 28 17:25:45 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4cZWS94qHsz6915P; Sun, 28 Sep 2025 17:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cZWS93G5zz42Jb; Sun, 28 Sep 2025 17:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759080345; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/QnEqRRlhnl0uJ2GQ4JX1NAIjw34Xcc6FG2MvWenWxk=; b=k9PtotFmiDXRzd9CshknJ8ZLKqI9jUXpx2YeMw8P+PH9juqV1orawJN0JMWfWnhU7IgYY+ ZtsRYW4V/LqCXGZkGLzxPg0b5cmggHM06OCTto7yrjpEv/T4ZDJn0PXWK8CywzPuc8FgKP M7fUCH00jQ5hgZ/ywFUhNy0wZ92IP2mrLq3bkFJ2jMlk45hUaN33X3Xa+Ao/jZqRijaRqB X2Nam3Oy9Nx9AP/MQ2k2u8SSClrrbe3kYrwvB/P7g0+XtIdEgF0e3nxROjULjwBmZNCO8H uFIqZ8zNmImx1TjgM/90JaklLQiowLFD49lQVW7FtNzgc+jeM/y/MrKtmCTN4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759080345; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/QnEqRRlhnl0uJ2GQ4JX1NAIjw34Xcc6FG2MvWenWxk=; b=DeM4v60OgOhTv/l+4CBBAp/kEm/mw6iMMnFnAJ+JG0MA0z296zM0xsm5oib9x13jWO8IJH QG+hydzfecObw4/jveUP26R1fumwYuKq94mfQSSWi7EaGKQ45TWy9taIkhy/OnOCL+aI7F aTS9igED7nVdlzAKicFBQ8vkMs8vKovkpFZCOzYdZldbxo0vcQchkIBQsaI3OYMtn2za21 KGXLZChWmK5xrwtyqejheGxvwzJLIk3QdyuNvLfWJaM3FwXObH/tWrYownmFwI1/dQenm3 Me01AANRH3Z33Budz2Jmsg1vWEoHWjUmOyGF4wAKlSVOXKHp8wEy5MFoJTAcpA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1759080345; a=rsa-sha256; cv=none; b=nif7YrQIAbBLF8YdJuVRC6e6kgrJnr4ENpqeMhQTLTfv+vg1BCv0WpQ8fbs/2VIvjW1QpQ 91zNbGk1UrkBZUjIJB+b1O1l53p8nUm/L+eTFiv7xqdk4gpuj79SWBJccGwPrBMxF4bpmJ 46gRDrGU6EsbYJaGHLDJrfnsZUnROT12MS2vgOI9fu+Yc5uJIcNjq5SVjfqLh5r4KNzIWC lo+qq+iyxoNQQ/e1BKL0yatKmTXI62oVsbUhBSEMQVXI+vDY3kDn0j4YHKs40MC54NekOH tTGih/ROdWQF5MCHKhwpJw2cQrfe86Qk5hmQxevNm7sM1+dPyhyh30cZsulJ4w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4cZWS92qhBzDXW; Sun, 28 Sep 2025 17:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 58SHPjo1027432; Sun, 28 Sep 2025 17:25:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 58SHPj2B027429; Sun, 28 Sep 2025 17:25:45 GMT (envelope-from git) Date: Sun, 28 Sep 2025 17:25:45 GMT Message-Id: <202509281725.58SHPj2B027429@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kajetan Staszkiewicz Subject: git: 7cd3854f827f - main - pf: Fix interface counters for af-to rules List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: ks X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7cd3854f827faaad1ecf414d20bdf6802cfa60f8 Auto-Submitted: auto-generated The branch main has been updated by ks: URL: https://cgit.FreeBSD.org/src/commit/?id=7cd3854f827faaad1ecf414d20bdf6802cfa60f8 commit 7cd3854f827faaad1ecf414d20bdf6802cfa60f8 Author: Kajetan Staszkiewicz AuthorDate: 2025-09-08 17:53:48 +0000 Commit: Kajetan Staszkiewicz 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 { 64:ff9b::${net_server1_4_host_server} }" \ "table { ${net_tester_6_host_tester} }" \ "table { 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 to scrub (random-id)" \ "pass in on ${epair_tester}b inet6 proto tcp from to \ @@ -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 { 64:ff9b::${net_server1_4_host_server} }" \ "table { ${net_tester_6_host_tester} }" \ "table { 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 to 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()