[PATCH v10 nf-next 0/3] conntrack: bridge: add double vlan, pppoe and pppoe-in-q

[PATCH v10 nf-next 0/3] conntrack: bridge: add double vlan, pppoe and pppoe-in-q

[Eric Woudstra]

Conntrack bridge only tracks untagged and 802.1q.

To make the bridge-fastpath experience more similar to the
forward-fastpath experience, add double vlan, pppoe and pppoe-in-q
tagged packets to bridge conntrack and to bridge filter chain.

Split from patch-set: bridge-fastpath and related improvements v9

Eric Woudstra (3):
  netfilter: bridge: Add conntrack double vlan and pppoe
  netfilter: nft_chain_filter: Add bridge double vlan and pppoe
  selftests: netfilter: Add conntrack_bridge.sh

 net/bridge/netfilter/nf_conntrack_bridge.c    |  83 +++++++--
 net/netfilter/nft_chain_filter.c              |  20 +-
 .../testing/selftests/net/netfilter/Makefile  |   1 +
 .../net/netfilter/conntrack_bridge.sh         | 176 ++++++++++++++++++
 4 files changed, 267 insertions(+), 13 deletions(-)
 create mode 100755 tools/testing/selftests/net/netfilter/conntrack_bridge.sh

-- 
2.47.1

[PATCH v10 nf-next 1/3] netfilter: bridge: Add conntrack double vlan and pppoe

[Eric Woudstra]

This adds the capability to conntrack 802.1ad, QinQ, PPPoE and PPPoE-in-Q
packets that are passing a bridge.

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 net/bridge/netfilter/nf_conntrack_bridge.c | 83 ++++++++++++++++++----
 1 file changed, 71 insertions(+), 12 deletions(-)

diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index 816bb0fde718..4b4e3751fb13 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -242,53 +242,112 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 {
 	struct nf_hook_state bridge_state = *state;
 	enum ip_conntrack_info ctinfo;
+	int ret, offset = 0;
 	struct nf_conn *ct;
-	u32 len;
-	int ret;
+	__be16 outer_proto;
+	u32 len, data_len;
 
 	ct = nf_ct_get(skb, &ctinfo);
 	if ((ct && !nf_ct_is_template(ct)) ||
 	    ctinfo == IP_CT_UNTRACKED)
 		return NF_ACCEPT;
 
+	switch (skb->protocol) {
+	case htons(ETH_P_PPP_SES): {
+		struct ppp_hdr {
+			struct pppoe_hdr hdr;
+			__be16 proto;
+		} *ph;
+
+		offset = PPPOE_SES_HLEN;
+		if (!pskb_may_pull(skb, offset))
+			return NF_ACCEPT;
+		outer_proto = skb->protocol;
+		ph = (struct ppp_hdr *)(skb->data);
+		switch (ph->proto) {
+		case htons(PPP_IP):
+			skb->protocol = htons(ETH_P_IP);
+			break;
+		case htons(PPP_IPV6):
+			skb->protocol = htons(ETH_P_IPV6);
+			break;
+		default:
+			nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+			return NF_ACCEPT;
+		}
+		data_len = ntohs(ph->hdr.length) - 2;
+		skb_pull_rcsum(skb, offset);
+		skb_reset_network_header(skb);
+		break;
+	}
+	case htons(ETH_P_8021Q): {
+		struct vlan_hdr *vhdr;
+
+		offset = VLAN_HLEN;
+		if (!pskb_may_pull(skb, offset))
+			return NF_ACCEPT;
+		outer_proto = skb->protocol;
+		vhdr = (struct vlan_hdr *)(skb->data);
+		skb->protocol = vhdr->h_vlan_encapsulated_proto;
+		data_len = U32_MAX;
+		skb_pull_rcsum(skb, offset);
+		skb_reset_network_header(skb);
+		break;
+	}
+	default:
+		data_len = U32_MAX;
+		break;
+	}
+
+	ret = NF_ACCEPT;
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
 		if (!pskb_may_pull(skb, sizeof(struct iphdr)))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		len = skb_ip_totlen(skb);
+		if (data_len < len)
+			len = data_len;
 		if (pskb_trim_rcsum(skb, len))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		if (nf_ct_br_ip_check(skb))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		bridge_state.pf = NFPROTO_IPV4;
 		ret = nf_ct_br_defrag4(skb, &bridge_state);
 		break;
 	case htons(ETH_P_IPV6):
 		if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
+		if (data_len < len)
+			len = data_len;
 		if (pskb_trim_rcsum(skb, len))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		if (nf_ct_br_ipv6_check(skb))
-			return NF_ACCEPT;
+			goto do_not_track;
 
 		bridge_state.pf = NFPROTO_IPV6;
 		ret = nf_ct_br_defrag6(skb, &bridge_state);
 		break;
 	default:
 		nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
-		return NF_ACCEPT;
+		goto do_not_track;
 	}
 
-	if (ret != NF_ACCEPT)
-		return ret;
+	if (ret == NF_ACCEPT)
+		ret = nf_conntrack_in(skb, &bridge_state);
 
-	return nf_conntrack_in(skb, &bridge_state);
+do_not_track:
+	if (offset) {
+		skb_push_rcsum(skb, offset);
+		skb_reset_network_header(skb);
+		skb->protocol = outer_proto;
+	}
+	return ret;
 }
 
 static unsigned int nf_ct_bridge_in(void *priv, struct sk_buff *skb,
-- 
2.47.1

[PATCH v10 nf-next 2/3] netfilter: nft_chain_filter: Add bridge double vlan and pppoe

[Eric Woudstra]

This adds the capability to evaluate 802.1ad, QinQ, PPPoE and PPPoE-in-Q
packets in the bridge filter chain.

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 net/netfilter/nft_chain_filter.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index 19a553550c76..7c7080c1a67d 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -232,11 +232,27 @@ nft_do_chain_bridge(void *priv,
 		    struct sk_buff *skb,
 		    const struct nf_hook_state *state)
 {
+	struct ethhdr *ethh = eth_hdr(skb);
 	struct nft_pktinfo pkt;
+	int thoff;
 
 	nft_set_pktinfo(&pkt, skb, state);
 
-	switch (eth_hdr(skb)->h_proto) {
+	switch (ethh->h_proto) {
+	case htons(ETH_P_PPP_SES):
+		thoff = PPPOE_SES_HLEN;
+		ethh += thoff;
+		break;
+	case htons(ETH_P_8021Q):
+		thoff = VLAN_HLEN;
+		ethh += thoff;
+		break;
+	default:
+		thoff = 0;
+		break;
+	}
+
+	switch (ethh->h_proto) {
 	case htons(ETH_P_IP):
 		nft_set_pktinfo_ipv4_validate(&pkt);
 		break;
@@ -248,6 +264,8 @@ nft_do_chain_bridge(void *priv,
 		break;
 	}
 
+	pkt.thoff += thoff;
+
 	return nft_do_chain(&pkt, priv);
 }
 
-- 
2.47.1

[PATCH v10 nf-next 3/3] selftests: netfilter: Add conntrack_bridge.sh

[Eric Woudstra]

Check conntrack bridge is functional in various vlan setups.

Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 .../testing/selftests/net/netfilter/Makefile  |   1 +
 .../net/netfilter/conntrack_bridge.sh         | 176 ++++++++++++++++++
 2 files changed, 177 insertions(+)
 create mode 100755 tools/testing/selftests/net/netfilter/conntrack_bridge.sh

diff --git a/tools/testing/selftests/net/netfilter/Makefile b/tools/testing/selftests/net/netfilter/Makefile
index ffe161fac8b5..bee403d423f5 100644
--- a/tools/testing/selftests/net/netfilter/Makefile
+++ b/tools/testing/selftests/net/netfilter/Makefile
@@ -8,6 +8,7 @@ MNL_LDLIBS := $(shell $(HOSTPKG_CONFIG) --libs libmnl 2>/dev/null || echo -lmnl)
 
 TEST_PROGS := br_netfilter.sh bridge_brouter.sh
 TEST_PROGS += br_netfilter_queue.sh
+TEST_PROGS += conntrack_bridge.sh
 TEST_PROGS += conntrack_dump_flush.sh
 TEST_PROGS += conntrack_icmp_related.sh
 TEST_PROGS += conntrack_ipip_mtu.sh
diff --git a/tools/testing/selftests/net/netfilter/conntrack_bridge.sh b/tools/testing/selftests/net/netfilter/conntrack_bridge.sh
new file mode 100755
index 000000000000..806551ef8cc2
--- /dev/null
+++ b/tools/testing/selftests/net/netfilter/conntrack_bridge.sh
@@ -0,0 +1,176 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Check conntrack bridge is functional in various vlan setups.
+#
+# Setup is:
+#
+# nsclient1 -> nsbr -> nsclient2
+# ping nsclient2 from nsclient1, checking that conntrack established
+# packets are seen.
+#
+
+source lib.sh
+
+if ! nft --version > /dev/null 2>&1;then
+	echo "SKIP: Could not run test without nft tool"
+	exit $ksft_skip
+fi
+
+cleanup() {
+	cleanup_all_ns
+}
+
+trap cleanup EXIT
+
+setup_ns nsclient1 nsclient2 nsbr
+
+ret=0
+
+add_addr()
+{
+	ns=$1
+	dev=$2
+	i=$3
+
+	ip -net "$ns" link set "$dev" up
+	ip -net "$ns" addr add "192.168.1.$i/24" dev "$dev"
+	ip -net "$ns" addr add "dead:1::$i/64" dev "$dev" nodad
+	ip -net "$ns" route add default dev "$dev"
+}
+
+del_addr()
+{
+	ns=$1
+	dev=$2
+	i=$3
+
+	ip -net "$ns" route del default dev "$dev"
+	ip -net "$ns" addr del "dead:1::$i/64" dev "$dev" nodad
+	ip -net "$ns" addr del "192.168.1.$i/24" dev "$dev"
+	ip -net "$ns" link set "$dev" down
+}
+
+send_pings()
+{
+	for ad in "$@"; do
+		if ! ip netns exec "$nsclient1" ping -c 1 -s 962 -q "$ad" >/dev/null; then
+			echo "ERROR: netns routing/connectivity broken to $ad" 1>&2
+			exit 1
+		fi
+	done
+}
+
+check_counter()
+{
+	ns=$1
+	name=$2
+	expect=$3
+	local lret=0
+
+	if ! ip netns exec "$ns" nft list counter bridge filter "$name" | grep -q "$expect"; then
+		echo "ERROR: counter $name in $ns has unexpected value (expected $expect)" 1>&2
+		ip netns exec "$ns" nft list counter bridge filter "$name" 1>&2
+		lret=1
+	fi
+	ip netns exec "$ns" nft reset counters >/dev/null
+
+	return $lret
+}
+
+BR=br0
+if ! ip -net "$nsbr" link add $BR type bridge; then
+	echo "SKIP: Can't create bridge $BR"
+	exit $ksft_skip
+fi
+
+DEV=veth0
+ip link add "$DEV" netns "$nsclient1" type veth peer name eth1 netns "$nsbr"
+ip link add "$DEV" netns "$nsclient2" type veth peer name eth2 netns "$nsbr"
+
+ip -net "$nsbr" link set eth1 master $BR up
+ip -net "$nsbr" link set eth2 master $BR up
+ip -net "$nsbr" link set $BR up
+
+ip netns exec "$nsbr" nft -f - <<EOF
+table bridge filter {
+	counter established { }
+	chain forward {
+		type filter hook forward priority 0; policy accept;
+		ct state "established" counter name "established"
+	}
+}
+EOF
+
+a=1;
+for ns in "$nsclient1" "$nsclient2"; do
+	add_addr "$ns" "$DEV" $a
+	((a++))
+done
+
+send_pings "192.168.1.2" "dead:1::2"
+expect="packets 2 bytes 2000"
+if ! check_counter "$nsbr" "established" "$expect"; then
+	msg+="\nFAIL: without vlan, established packets not seen"
+	ret=1
+fi
+
+a=1;
+for ns in "$nsclient1" "$nsclient2"; do
+	del_addr "$ns" "$DEV" $a
+	ip -net "$ns" link add link "$DEV" name "$DEV.10" type vlan id 10
+	ip -net "$ns" link set "$DEV" up
+	add_addr "$ns" "$DEV.10" $a
+	((a++))
+done
+
+send_pings "192.168.1.2" "dead:1::2"
+expect="packets 2 bytes 2000"
+if ! check_counter "$nsbr" "established" "$expect"; then
+	msg+="\nFAIL: with single vlan, established packets not seen"
+	ret=1
+fi
+
+a=1;
+for ns in "$nsclient1" "$nsclient2"; do
+	del_addr "$ns" "$DEV.10" $a
+	ip -net "$ns" link add link "$DEV.10" name "$DEV.10.20" type vlan id 20
+	ip -net "$ns" link set "$DEV.10" up
+	add_addr "$ns" "$DEV.10.20" $a
+	((a++))
+done
+
+send_pings "192.168.1.2" "dead:1::2"
+expect="packets 2 bytes 2008"
+if ! check_counter "$nsbr" "established" "$expect"; then
+	msg+="\nFAIL: with double q vlan, established packets not seen"
+	ret=1
+fi
+
+a=1;
+for ns in "$nsclient1" "$nsclient2"; do
+	del_addr "$ns" "$DEV.10.20" $a
+	ip -net "$ns" link del "$DEV.10.20"
+	ip -net "$ns" link del "$DEV.10"
+	ip -net "$ns" link add link "$DEV" name "$DEV.10" type vlan id 10 protocol 802.1ad
+	ip -net "$ns" link add link "$DEV.10" name "$DEV.10.20" type vlan id 20
+	ip -net "$ns" link set "$DEV.10" up
+	add_addr "$ns" "$DEV.10.20" $a
+	((a++))
+done
+
+send_pings "192.168.1.2" "dead:1::2"
+expect="packets 2 bytes 2008"
+if ! check_counter "$nsbr" "established" "$expect"; then
+	msg+="\nFAIL: with 802.1ad vlan, established packets not seen "
+	ret=1
+fi
+
+if [ $ret -eq 0 ];then
+	echo "PASS: established packets seen in all cases"
+else
+	echo -e "$msg"
+fi
+
+exit $ret
+
-- 
2.47.1

Re: [PATCH v10 nf-next 2/3] netfilter: nft_chain_filter: Add bridge double vlan and pppoe

[Pablo Neira Ayuso]

Hi,

On Sat, Mar 15, 2025 at 09:00:32PM +0100, Eric Woudstra wrote:
> This adds the capability to evaluate 802.1ad, QinQ, PPPoE and PPPoE-in-Q
> packets in the bridge filter chain.
> 
> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> ---
>  net/netfilter/nft_chain_filter.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
> index 19a553550c76..7c7080c1a67d 100644
> --- a/net/netfilter/nft_chain_filter.c
> +++ b/net/netfilter/nft_chain_filter.c
> @@ -232,11 +232,27 @@ nft_do_chain_bridge(void *priv,
>  		    struct sk_buff *skb,
>  		    const struct nf_hook_state *state)
>  {
> +	struct ethhdr *ethh = eth_hdr(skb);
>  	struct nft_pktinfo pkt;
> +	int thoff;
>  
>  	nft_set_pktinfo(&pkt, skb, state);
>  
> -	switch (eth_hdr(skb)->h_proto) {
> +	switch (ethh->h_proto) {
> +	case htons(ETH_P_PPP_SES):
> +		thoff = PPPOE_SES_HLEN;
> +		ethh += thoff;

This pointer arithmetics does not look correct, ethh is struct ethhdr,
neither void nor char.

> +		break;
> +	case htons(ETH_P_8021Q):
> +		thoff = VLAN_HLEN;
> +		ethh += thoff;

Same here.

> +		break;
> +	default:
> +		thoff = 0;
> +		break;
> +	}
> +
> +	switch (ethh->h_proto) {

This switch will match on the wrong offset.

>  	case htons(ETH_P_IP):
>  		nft_set_pktinfo_ipv4_validate(&pkt);
>  		break;
> @@ -248,6 +264,8 @@ nft_do_chain_bridge(void *priv,
>  		break;
>  	}
>  
> +	pkt.thoff += thoff;

And only transport offset is adjusted here.

>  	return nft_do_chain(&pkt, priv);
>  }
>  
> -- 
> 2.47.1
> 

Re: [PATCH v10 nf-next 3/3] selftests: netfilter: Add conntrack_bridge.sh

[Pablo Neira Ayuso]

On Sat, Mar 15, 2025 at 09:00:33PM +0100, Eric Woudstra wrote:
> Check conntrack bridge is functional in various vlan setups.

Only conntrack bridge support is tested here, patch 2/3 does not seem
to be covered :(

> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> ---
>  .../testing/selftests/net/netfilter/Makefile  |   1 +
>  .../net/netfilter/conntrack_bridge.sh         | 176 ++++++++++++++++++
>  2 files changed, 177 insertions(+)
>  create mode 100755 tools/testing/selftests/net/netfilter/conntrack_bridge.sh
> 
> diff --git a/tools/testing/selftests/net/netfilter/Makefile b/tools/testing/selftests/net/netfilter/Makefile
> index ffe161fac8b5..bee403d423f5 100644
> --- a/tools/testing/selftests/net/netfilter/Makefile
> +++ b/tools/testing/selftests/net/netfilter/Makefile
> @@ -8,6 +8,7 @@ MNL_LDLIBS := $(shell $(HOSTPKG_CONFIG) --libs libmnl 2>/dev/null || echo -lmnl)
>  
>  TEST_PROGS := br_netfilter.sh bridge_brouter.sh
>  TEST_PROGS += br_netfilter_queue.sh
> +TEST_PROGS += conntrack_bridge.sh
>  TEST_PROGS += conntrack_dump_flush.sh
>  TEST_PROGS += conntrack_icmp_related.sh
>  TEST_PROGS += conntrack_ipip_mtu.sh
> diff --git a/tools/testing/selftests/net/netfilter/conntrack_bridge.sh b/tools/testing/selftests/net/netfilter/conntrack_bridge.sh
> new file mode 100755
> index 000000000000..806551ef8cc2
> --- /dev/null
> +++ b/tools/testing/selftests/net/netfilter/conntrack_bridge.sh
> @@ -0,0 +1,176 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Check conntrack bridge is functional in various vlan setups.
> +#
> +# Setup is:
> +#
> +# nsclient1 -> nsbr -> nsclient2
> +# ping nsclient2 from nsclient1, checking that conntrack established
> +# packets are seen.
> +#
> +
> +source lib.sh
> +
> +if ! nft --version > /dev/null 2>&1;then
> +	echo "SKIP: Could not run test without nft tool"
> +	exit $ksft_skip
> +fi
> +
> +cleanup() {
> +	cleanup_all_ns
> +}
> +
> +trap cleanup EXIT
> +
> +setup_ns nsclient1 nsclient2 nsbr
> +
> +ret=0
> +
> +add_addr()
> +{
> +	ns=$1
> +	dev=$2
> +	i=$3
> +
> +	ip -net "$ns" link set "$dev" up
> +	ip -net "$ns" addr add "192.168.1.$i/24" dev "$dev"
> +	ip -net "$ns" addr add "dead:1::$i/64" dev "$dev" nodad
> +	ip -net "$ns" route add default dev "$dev"
> +}
> +
> +del_addr()
> +{
> +	ns=$1
> +	dev=$2
> +	i=$3
> +
> +	ip -net "$ns" route del default dev "$dev"
> +	ip -net "$ns" addr del "dead:1::$i/64" dev "$dev" nodad
> +	ip -net "$ns" addr del "192.168.1.$i/24" dev "$dev"
> +	ip -net "$ns" link set "$dev" down
> +}
> +
> +send_pings()
> +{
> +	for ad in "$@"; do
> +		if ! ip netns exec "$nsclient1" ping -c 1 -s 962 -q "$ad" >/dev/null; then
> +			echo "ERROR: netns routing/connectivity broken to $ad" 1>&2
> +			exit 1
> +		fi
> +	done
> +}
> +
> +check_counter()
> +{
> +	ns=$1
> +	name=$2
> +	expect=$3
> +	local lret=0
> +
> +	if ! ip netns exec "$ns" nft list counter bridge filter "$name" | grep -q "$expect"; then
> +		echo "ERROR: counter $name in $ns has unexpected value (expected $expect)" 1>&2
> +		ip netns exec "$ns" nft list counter bridge filter "$name" 1>&2
> +		lret=1
> +	fi
> +	ip netns exec "$ns" nft reset counters >/dev/null
> +
> +	return $lret
> +}
> +
> +BR=br0
> +if ! ip -net "$nsbr" link add $BR type bridge; then
> +	echo "SKIP: Can't create bridge $BR"
> +	exit $ksft_skip
> +fi
> +
> +DEV=veth0
> +ip link add "$DEV" netns "$nsclient1" type veth peer name eth1 netns "$nsbr"
> +ip link add "$DEV" netns "$nsclient2" type veth peer name eth2 netns "$nsbr"
> +
> +ip -net "$nsbr" link set eth1 master $BR up
> +ip -net "$nsbr" link set eth2 master $BR up
> +ip -net "$nsbr" link set $BR up
> +
> +ip netns exec "$nsbr" nft -f - <<EOF
> +table bridge filter {
> +	counter established { }
> +	chain forward {
> +		type filter hook forward priority 0; policy accept;
> +		ct state "established" counter name "established"
> +	}
> +}
> +EOF
> +
> +a=1;
> +for ns in "$nsclient1" "$nsclient2"; do
> +	add_addr "$ns" "$DEV" $a
> +	((a++))
> +done
> +
> +send_pings "192.168.1.2" "dead:1::2"
> +expect="packets 2 bytes 2000"
> +if ! check_counter "$nsbr" "established" "$expect"; then
> +	msg+="\nFAIL: without vlan, established packets not seen"
> +	ret=1
> +fi
> +
> +a=1;
> +for ns in "$nsclient1" "$nsclient2"; do
> +	del_addr "$ns" "$DEV" $a
> +	ip -net "$ns" link add link "$DEV" name "$DEV.10" type vlan id 10
> +	ip -net "$ns" link set "$DEV" up
> +	add_addr "$ns" "$DEV.10" $a
> +	((a++))
> +done
> +
> +send_pings "192.168.1.2" "dead:1::2"
> +expect="packets 2 bytes 2000"
> +if ! check_counter "$nsbr" "established" "$expect"; then
> +	msg+="\nFAIL: with single vlan, established packets not seen"
> +	ret=1
> +fi
> +
> +a=1;
> +for ns in "$nsclient1" "$nsclient2"; do
> +	del_addr "$ns" "$DEV.10" $a
> +	ip -net "$ns" link add link "$DEV.10" name "$DEV.10.20" type vlan id 20
> +	ip -net "$ns" link set "$DEV.10" up
> +	add_addr "$ns" "$DEV.10.20" $a
> +	((a++))
> +done
> +
> +send_pings "192.168.1.2" "dead:1::2"
> +expect="packets 2 bytes 2008"
> +if ! check_counter "$nsbr" "established" "$expect"; then
> +	msg+="\nFAIL: with double q vlan, established packets not seen"
> +	ret=1
> +fi
> +
> +a=1;
> +for ns in "$nsclient1" "$nsclient2"; do
> +	del_addr "$ns" "$DEV.10.20" $a
> +	ip -net "$ns" link del "$DEV.10.20"
> +	ip -net "$ns" link del "$DEV.10"
> +	ip -net "$ns" link add link "$DEV" name "$DEV.10" type vlan id 10 protocol 802.1ad
> +	ip -net "$ns" link add link "$DEV.10" name "$DEV.10.20" type vlan id 20
> +	ip -net "$ns" link set "$DEV.10" up
> +	add_addr "$ns" "$DEV.10.20" $a
> +	((a++))
> +done
> +
> +send_pings "192.168.1.2" "dead:1::2"
> +expect="packets 2 bytes 2008"
> +if ! check_counter "$nsbr" "established" "$expect"; then
> +	msg+="\nFAIL: with 802.1ad vlan, established packets not seen "
> +	ret=1
> +fi
> +
> +if [ $ret -eq 0 ];then
> +	echo "PASS: established packets seen in all cases"
> +else
> +	echo -e "$msg"
> +fi
> +
> +exit $ret
> +
> -- 
> 2.47.1
> 

Re: [PATCH v10 nf-next 2/3] netfilter: nft_chain_filter: Add bridge double vlan and pppoe

[Eric Woudstra]

On 3/19/25 12:04 AM, Pablo Neira Ayuso wrote:
> Hi,
> 
> On Sat, Mar 15, 2025 at 09:00:32PM +0100, Eric Woudstra wrote:
>> This adds the capability to evaluate 802.1ad, QinQ, PPPoE and PPPoE-in-Q
>> packets in the bridge filter chain.
>>
>> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> ---
>>  net/netfilter/nft_chain_filter.c | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
>> index 19a553550c76..7c7080c1a67d 100644
>> --- a/net/netfilter/nft_chain_filter.c
>> +++ b/net/netfilter/nft_chain_filter.c
>> @@ -232,11 +232,27 @@ nft_do_chain_bridge(void *priv,
>>  		    struct sk_buff *skb,
>>  		    const struct nf_hook_state *state)
>>  {
>> +	struct ethhdr *ethh = eth_hdr(skb);
>>  	struct nft_pktinfo pkt;
>> +	int thoff;
>>  
>>  	nft_set_pktinfo(&pkt, skb, state);
>>  
>> -	switch (eth_hdr(skb)->h_proto) {
>> +	switch (ethh->h_proto) {
>> +	case htons(ETH_P_PPP_SES):
>> +		thoff = PPPOE_SES_HLEN;
>> +		ethh += thoff;
> 
> This pointer arithmetics does not look correct, ethh is struct ethhdr,
> neither void nor char.
> 
>> +		break;
>> +	case htons(ETH_P_8021Q):
>> +		thoff = VLAN_HLEN;
>> +		ethh += thoff;
> 
> Same here.
> 
>> +		break;
>> +	default:
>> +		thoff = 0;
>> +		break;
>> +	}
>> +
>> +	switch (ethh->h_proto) {
> 
> This switch will match on the wrong offset.
> 
>>  	case htons(ETH_P_IP):
>>  		nft_set_pktinfo_ipv4_validate(&pkt);
>>  		break;
>> @@ -248,6 +264,8 @@ nft_do_chain_bridge(void *priv,
>>  		break;
>>  	}
>>  
>> +	pkt.thoff += thoff;
> 
> And only transport offset is adjusted here.
> 
>>  	return nft_do_chain(&pkt, priv);
>>  }
>>  
>> -- 
>> 2.47.1
>>

I will sort this out and send a new version after the merge window.

Re: [PATCH v10 nf-next 3/3] selftests: netfilter: Add conntrack_bridge.sh

[Eric Woudstra]

On 3/19/25 12:07 AM, Pablo Neira Ayuso wrote:
> On Sat, Mar 15, 2025 at 09:00:33PM +0100, Eric Woudstra wrote:
>> Check conntrack bridge is functional in various vlan setups.
> 
> Only conntrack bridge support is tested here, patch 2/3 does not seem
> to be covered :(
> 

I should add more details to this description.

I do add the nftables table bridge, chain forward, type filter
and check the counter for ct state established.

Without patch 2/3 this counter does not increase.

I can add more match criteria to the counter next version, to better
check patch 2/3 at l3 (and l4) level.