diff mbox series

[RFC,v1,net-next,02/12] netfilter: bridge: Add conntrack double vlan and pppoe

Message ID 20241013185509.4430-3-ericwouds@gmail.com
State RFC
Headers show
Series bridge-fastpath and related improvements | expand

Commit Message

Eric Woudstra Oct. 13, 2024, 6:54 p.m. UTC
This adds the capability to conntrack 802.1ad, QinQ, PPPoE and PPPoE-in-Q
packets that are passing a bridge.

Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 net/bridge/netfilter/nf_conntrack_bridge.c | 86 ++++++++++++++++++----
 1 file changed, 73 insertions(+), 13 deletions(-)

Comments

Vladimir Oltean Oct. 18, 2024, 1:17 p.m. UTC | #1
On Sun, Oct 13, 2024 at 08:54:58PM +0200, Eric Woudstra wrote:
> This adds the capability to conntrack 802.1ad, QinQ, PPPoE and PPPoE-in-Q
> packets that are passing a bridge.
> 
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> ---

Whatever you choose to do forward with these patches, please squash this
build fix here (you can drop my authorship info and commit message):

From e73315196c3143de2af2fe39e3b0e95391849d6c Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Fri, 18 Oct 2024 13:59:27 +0300
Subject: [PATCH] netfilter: bridge: fix build failures in nf_ct_bridge_pre()

clang-16 fails to build, stating:

net/bridge/netfilter/nf_conntrack_bridge.c:257:3: error: expected expression
                struct ppp_hdr {
                ^
net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
                data_len = ntohs(ph->hdr.length) - 2;
                                 ^
net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
net/bridge/netfilter/nf_conntrack_bridge.c:265:11: error: use of undeclared identifier 'ph'
                switch (ph->proto) {
                        ^

net/bridge/netfilter/nf_conntrack_bridge.c:278:3: error: expected expression
                struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data);
                ^
net/bridge/netfilter/nf_conntrack_bridge.c:283:17: error: use of undeclared identifier 'vhdr'
                inner_proto = vhdr->h_vlan_encapsulated_proto;
                              ^

One cannot have variable declarations placed this way in a switch/case
statement, a new scope must be opened.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/netfilter/nf_conntrack_bridge.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index fb2f79396aa0..31e2bcd71735 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -253,7 +253,7 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 		return NF_ACCEPT;
 
 	switch (skb->protocol) {
-	case htons(ETH_P_PPP_SES):
+	case htons(ETH_P_PPP_SES): {
 		struct ppp_hdr {
 			struct pppoe_hdr hdr;
 			__be16 proto;
@@ -273,7 +273,8 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 			return NF_ACCEPT;
 		}
 		break;
-	case htons(ETH_P_8021Q):
+	}
+	case htons(ETH_P_8021Q): {
 		struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data);
 
 		data_len = 0xffffffff;
@@ -281,6 +282,7 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 		outer_proto = skb->protocol;
 		inner_proto = vhdr->h_vlan_encapsulated_proto;
 		break;
+	}
 	default:
 		data_len = 0xffffffff;
 		break;
Eric Woudstra Oct. 18, 2024, 6:53 p.m. UTC | #2
On 10/18/24 3:17 PM, Vladimir Oltean wrote:
> On Sun, Oct 13, 2024 at 08:54:58PM +0200, Eric Woudstra wrote:
>> This adds the capability to conntrack 802.1ad, QinQ, PPPoE and PPPoE-in-Q
>> packets that are passing a bridge.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> ---
> 
> Whatever you choose to do forward with these patches, please squash this
> build fix here (you can drop my authorship info and commit message):

Thanks, I had already fixed the errors from patchwork.kernel.org->checks
for the next version of the rfc patch. This is indeed one of them.

> From e73315196c3143de2af2fe39e3b0e95391849d6c Mon Sep 17 00:00:00 2001
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Date: Fri, 18 Oct 2024 13:59:27 +0300
> Subject: [PATCH] netfilter: bridge: fix build failures in nf_ct_bridge_pre()
> 
> clang-16 fails to build, stating:
> 
> net/bridge/netfilter/nf_conntrack_bridge.c:257:3: error: expected expression
>                 struct ppp_hdr {
>                 ^
> net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
>                 data_len = ntohs(ph->hdr.length) - 2;
>                                  ^
> net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
> net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
> net/bridge/netfilter/nf_conntrack_bridge.c:262:20: error: use of undeclared identifier 'ph'
> net/bridge/netfilter/nf_conntrack_bridge.c:265:11: error: use of undeclared identifier 'ph'
>                 switch (ph->proto) {
>                         ^
> 
> net/bridge/netfilter/nf_conntrack_bridge.c:278:3: error: expected expression
>                 struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data);
>                 ^
> net/bridge/netfilter/nf_conntrack_bridge.c:283:17: error: use of undeclared identifier 'vhdr'
>                 inner_proto = vhdr->h_vlan_encapsulated_proto;
>                               ^
> 
> One cannot have variable declarations placed this way in a switch/case
> statement, a new scope must be opened.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/netfilter/nf_conntrack_bridge.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
> index fb2f79396aa0..31e2bcd71735 100644
> --- a/net/bridge/netfilter/nf_conntrack_bridge.c
> +++ b/net/bridge/netfilter/nf_conntrack_bridge.c
> @@ -253,7 +253,7 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
>  		return NF_ACCEPT;
>  
>  	switch (skb->protocol) {
> -	case htons(ETH_P_PPP_SES):
> +	case htons(ETH_P_PPP_SES): {
>  		struct ppp_hdr {
>  			struct pppoe_hdr hdr;
>  			__be16 proto;
> @@ -273,7 +273,8 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
>  			return NF_ACCEPT;
>  		}
>  		break;
> -	case htons(ETH_P_8021Q):
> +	}
> +	case htons(ETH_P_8021Q): {
>  		struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data);
>  
>  		data_len = 0xffffffff;
> @@ -281,6 +282,7 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
>  		outer_proto = skb->protocol;
>  		inner_proto = vhdr->h_vlan_encapsulated_proto;
>  		break;
> +	}
>  	default:
>  		data_len = 0xffffffff;
>  		break;
diff mbox series

Patch

diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index 816bb0fde718..fb2f79396aa0 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -241,56 +241,116 @@  static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 				     const struct nf_hook_state *state)
 {
 	struct nf_hook_state bridge_state = *state;
+	__be16 outer_proto, inner_proto;
 	enum ip_conntrack_info ctinfo;
+	int ret, offset = 0;
 	struct nf_conn *ct;
-	u32 len;
-	int ret;
+	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 = (struct ppp_hdr *)(skb->data);
+
+		data_len = ntohs(ph->hdr.length) - 2;
+		offset = PPPOE_SES_HLEN;
+		outer_proto = skb->protocol;
+		switch (ph->proto) {
+		case htons(PPP_IP):
+			inner_proto = htons(ETH_P_IP);
+			break;
+		case htons(PPP_IPV6):
+			inner_proto = htons(ETH_P_IPV6);
+			break;
+		default:
+			return NF_ACCEPT;
+		}
+		break;
+	case htons(ETH_P_8021Q):
+		struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data);
+
+		data_len = 0xffffffff;
+		offset = VLAN_HLEN;
+		outer_proto = skb->protocol;
+		inner_proto = vhdr->h_vlan_encapsulated_proto;
+		break;
+	default:
+		data_len = 0xffffffff;
+		break;
+	}
+
+	if (offset) {
+		switch (inner_proto) {
+		case htons(ETH_P_IP):
+		case htons(ETH_P_IPV6):
+			if (!pskb_may_pull(skb, offset))
+				return NF_ACCEPT;
+			skb_pull_rcsum(skb, offset);
+			skb_reset_network_header(skb);
+			skb->protocol = inner_proto;
+			break;
+		default:
+			return NF_ACCEPT;
+		}
+	}
+
+	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,
 				    const struct nf_hook_state *state)
 {