[PATCH nf-next v2 1/3] nf_flow_table_offload: offload the vlan encap in the flowtable

[PATCH nf-next v2 1/3] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenxu]

From: wenxu <wenxu@chinatelecom.cn>

This patch put the vlan dev process in the FLOW_OFFLOAD_XMIT_DIRECT
mode. Xmit the packet with vlan can offload to the real dev directly.

It can support all kinds of VLAN dev path:
br0.100-->br0(vlan filter enable)-->eth
br0(vlan filter enable)-->eth
br0(vlan filter disable)-->eth.100-->eth

The packet xmit and recv offload to the 'eth' in both original and
reply direction.

Signed-off-by: wenxu <wenxu@chinatelecom.cn>
---
 net/netfilter/nf_flow_table_ip.c | 19 +++++++++++++++++++
 net/netfilter/nft_flow_offload.c |  9 ++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index b350fe9..5da651d 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto,
 	return false;
 }
 
+static void nf_flow_encap_push(struct sk_buff *skb,
+			       struct flow_offload_tuple_rhash *tuplehash)
+{
+	int i;
+
+	for (i = 0; i < tuplehash->tuple.encap_num; i++) {
+		switch (tuplehash->tuple.encap[i].proto) {
+		case htons(ETH_P_8021Q):
+		case htons(ETH_P_8021AD):
+			skb_vlan_push(skb,
+				      tuplehash->tuple.encap[i].proto,
+				      tuplehash->tuple.encap[i].id);
+			break;
+		}
+	}
+}
+
 static void nf_flow_encap_pop(struct sk_buff *skb,
 			      struct flow_offload_tuple_rhash *tuplehash)
 {
@@ -417,6 +434,7 @@ static unsigned int nf_flow_queue_xmit(struct net *net, struct sk_buff *skb,
 		ret = NF_STOLEN;
 		break;
 	case FLOW_OFFLOAD_XMIT_DIRECT:
+		nf_flow_encap_push(skb, &flow->tuplehash[!dir]);
 		ret = nf_flow_queue_xmit(state->net, skb, tuplehash, ETH_P_IP);
 		if (ret == NF_DROP)
 			flow_offload_teardown(flow);
@@ -678,6 +696,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev,
 		ret = NF_STOLEN;
 		break;
 	case FLOW_OFFLOAD_XMIT_DIRECT:
+		nf_flow_encap_push(skb, &flow->tuplehash[!dir]);
 		ret = nf_flow_queue_xmit(state->net, skb, tuplehash, ETH_P_IPV6);
 		if (ret == NF_DROP)
 			flow_offload_teardown(flow);
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index a25c88b..bfe7a3a 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -123,13 +123,16 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
 				info->indev = NULL;
 				break;
 			}
-			if (!info->outdev)
-				info->outdev = path->dev;
 			info->encap[info->num_encaps].id = path->encap.id;
 			info->encap[info->num_encaps].proto = path->encap.proto;
 			info->num_encaps++;
-			if (path->type == DEV_PATH_PPPOE)
+			if (path->type == DEV_PATH_PPPOE) {
+				if (!info->outdev)
+					info->outdev = path->dev;
 				memcpy(info->h_dest, path->encap.h_dest, ETH_ALEN);
+			}
+			if (path->type == DEV_PATH_VLAN)
+				info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
 			break;
 		case DEV_PATH_BRIDGE:
 			if (is_zero_ether_addr(info->h_source))
-- 
1.8.3.1

[PATCH nf-next v2 2/3] nf_flow_table_offload: offload the PPPoE encap in the flowtable

[wenxu]

From: wenxu <wenxu@chinatelecom.cn>

This patch put the pppoe process in the FLOW_OFFLOAD_XMIT_DIRECT
mode. Xmit the packet with PPPoE can offload to the underlay device
directly.

It can support all kinds of VLAN dev path:
pppoe-->eth
pppoe-->br0.100-->br0(vlan filter enable)-->eth
pppoe-->eth.100-->eth

The packet xmit and recv offload to the 'eth' in both original and
reply direction.

Signed-off-by: wenxu <wenxu@chinatelecom.cn>
---
 include/net/netfilter/nf_flow_table.h | 34 ++++++++++++++++++++++++++++++++++
 net/netfilter/nf_flow_table_ip.c      |  3 +++
 net/netfilter/nft_flow_offload.c      | 11 +++--------
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 64daafd..a0b310e 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -319,6 +319,40 @@ int nf_flow_rule_route_ipv6(struct net *net, const struct flow_offload *flow,
 int nf_flow_table_offload_init(void);
 void nf_flow_table_offload_exit(void);
 
+static inline int nf_flow_ppoe_push(struct sk_buff *skb, u16 id)
+{
+	struct ppp_hdr {
+		struct pppoe_hdr hdr;
+		__be16 proto;
+	} *ph;
+	int data_len = skb->len + 2;
+	__be16 proto;
+
+	if (skb_cow_head(skb, PPPOE_SES_HLEN))
+		return -1;
+
+	if (skb->protocol == htons(ETH_P_IP))
+		proto = htons(PPP_IP);
+	else if (skb->protocol == htons(ETH_P_IPV6))
+		proto = htons(PPP_IPV6);
+	else
+		return -1;
+
+	__skb_push(skb, PPPOE_SES_HLEN);
+	skb_reset_network_header(skb);
+
+	ph = (struct ppp_hdr *)(skb->data);
+	ph->hdr.ver  = 1;
+	ph->hdr.type = 1;
+	ph->hdr.code = 0;
+	ph->hdr.sid  = htons(id);
+	ph->hdr.length = htons(data_len);
+	ph->proto = proto;
+	skb->protocol = htons(ETH_P_PPP_SES);
+
+	return 0;
+}
+
 static inline __be16 nf_flow_pppoe_proto(const struct sk_buff *skb)
 {
 	__be16 proto;
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 5da651d..a0c640e 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -304,6 +304,9 @@ static void nf_flow_encap_push(struct sk_buff *skb,
 				      tuplehash->tuple.encap[i].proto,
 				      tuplehash->tuple.encap[i].id);
 			break;
+		case htons(ETH_P_PPP_SES):
+			nf_flow_ppoe_push(skb, tuplehash->tuple.encap[i].id);
+			break;
 		}
 	}
 }
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index bfe7a3a..9296a1f 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -126,13 +126,9 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
 			info->encap[info->num_encaps].id = path->encap.id;
 			info->encap[info->num_encaps].proto = path->encap.proto;
 			info->num_encaps++;
-			if (path->type == DEV_PATH_PPPOE) {
-				if (!info->outdev)
-					info->outdev = path->dev;
+			if (path->type == DEV_PATH_PPPOE)
 				memcpy(info->h_dest, path->encap.h_dest, ETH_ALEN);
-			}
-			if (path->type == DEV_PATH_VLAN)
-				info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
+			info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
 			break;
 		case DEV_PATH_BRIDGE:
 			if (is_zero_ether_addr(info->h_source))
@@ -160,8 +156,7 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
 			break;
 		}
 	}
-	if (!info->outdev)
-		info->outdev = info->indev;
+	info->outdev = info->indev;
 
 	info->hw_outdev = info->indev;
 
-- 
1.8.3.1

[PATCH nf-next v2 3/3] selftests: netfilter: flowtable vlan filtering bridge support

[wenxu]

From: wenxu <wenxu@chinatelecom.cn>

Add vlan_filtering enabled bridge and vlan case.
Add a vlan_filtering bridge device to the Router1 (nsr1) container
and attach the veth0 device to the bridge. Set the IP address to
the bridge device to exercise the bridge forwarding path.
The veth0 add in the vlan 10 domain and the br0 also add in the
vlan 10 domain with untaged.

Signed-off-by: wenxu <wenxu@chinatelecom.cn>
---
v2: fix set up the br0
    change iif br0 to iifname br0 for br0 destroy
    All the test PASS

 tools/testing/selftests/netfilter/nft_flowtable.sh | 28 +++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_flowtable.sh b/tools/testing/selftests/netfilter/nft_flowtable.sh
index d4ffebb..13e03e3 100755
--- a/tools/testing/selftests/netfilter/nft_flowtable.sh
+++ b/tools/testing/selftests/netfilter/nft_flowtable.sh
@@ -37,6 +37,7 @@ checktool "nft --version" "run test without nft tool"
 checktool "ip -Version" "run test without ip tool"
 checktool "which nc" "run test without nc (netcat)"
 checktool "ip netns add nsr1" "create net namespace"
+checktool "bridge -Version" "run test without bridge tool"
 
 ip netns add ns1
 ip netns add ns2
@@ -388,7 +389,7 @@ flush table ip nat
 table ip nat {
    chain prerouting {
       type nat hook prerouting priority 0; policy accept;
-      meta iif "br0" ip daddr 10.6.6.6 tcp dport 1666 counter dnat ip to 10.0.2.99:12345
+      meta iifname "br0" ip daddr 10.6.6.6 tcp dport 1666 counter dnat ip to 10.0.2.99:12345
    }
 
    chain postrouting {
@@ -431,12 +432,33 @@ else
 	ret=1
 fi
 
-# restore test topology (remove bridge and VLAN)
-ip -net nsr1 link set veth0 nomaster
+# Another test:
+# Add vlan filtering bridge interface br0 to Router1, with NAT and VLAN.
+ip -net nsr1 link set veth0.10 nomaster
 ip -net nsr1 link set veth0 down
 ip -net nsr1 link set veth0.10 down
 ip -net nsr1 link delete veth0.10 type vlan
 ip -net nsr1 link delete br0 type bridge
+ip -net nsr1 link add name br0 type bridge vlan_filtering 1
+ip -net nsr1 link set up dev veth0
+ip -net nsr1 link set veth0 master br0
+ip -net nsr1 link set up dev br0
+ip -net nsr1 addr add 10.0.1.1/24 dev br0
+bridge -n nsr1 vlan add dev veth0 vid 10 pvid
+bridge -n nsr1 vlan add dev br0 vid 10 pvid untagged self
+
+if test_tcp_forwarding_nat ns1 ns2; then
+	echo "PASS: flow offloaded for ns1/ns2 with vlan filtering bridge NAT and VLAN"
+else
+	echo "FAIL: flow offload for ns1/ns2 with vlan filtering bridge NAT and VLAN" 1>&2
+	ip netns exec nsr1 nft list ruleset
+	ret=1
+fi
+
+# restore test topology (remove bridge and VLAN)
+ip -net nsr1 link set veth0 nomaster
+ip -net nsr1 link set veth0 down
+ip -net nsr1 link delete br0 type bridge
 ip -net ns1 addr flush dev eth0.10
 ip -net ns1 link set eth0.10 down
 ip -net ns1 link set eth0 down
-- 
1.8.3.1

Re: [PATCH nf-next v2 1/3] nf_flow_table_offload: offload the vlan encap in the flowtable

[Pablo Neira Ayuso]

On Thu, May 26, 2022 at 02:57:30AM -0400, wenxu@chinatelecom.cn wrote:
[...]
> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index b350fe9..5da651d 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto,
>  	return false;
>  }
>  
> +static void nf_flow_encap_push(struct sk_buff *skb,
> +			       struct flow_offload_tuple_rhash *tuplehash)
> +{
> +	int i;
> +
> +	for (i = 0; i < tuplehash->tuple.encap_num; i++) {
> +		switch (tuplehash->tuple.encap[i].proto) {
> +		case htons(ETH_P_8021Q):
> +		case htons(ETH_P_8021AD):
> +			skb_vlan_push(skb,

Nit: skb_vlan_push() might fail.

> +				      tuplehash->tuple.encap[i].proto,
> +				      tuplehash->tuple.encap[i].id);
> +			break;
> +		}
> +	}
> +}

If I understand correctly, the goal of this patchset is to move the
existing vlan and ppp support to use the XMIT_DIRECT path?

So this already works but you would prefer to not use XMIT_NEIGH?

The scenarios you describe already work fine with the existing
codebase? I am assuming 'eth' provides Internet access? You refer to
this in the patch description:

 br0.100-->br0(vlan filter enable)-->eth
 br0(vlan filter enable)-->eth
 br0(vlan filter disable)-->eth.100-->eth

Re: [PATCH nf-next v2 2/3] nf_flow_table_offload: offload the PPPoE encap in the flowtable

[Pablo Neira Ayuso]

On Thu, May 26, 2022 at 02:57:31AM -0400, wenxu@chinatelecom.cn wrote:
> From: wenxu <wenxu@chinatelecom.cn>
> 
> This patch put the pppoe process in the FLOW_OFFLOAD_XMIT_DIRECT
> mode. Xmit the packet with PPPoE can offload to the underlay device
> directly.
> 
> It can support all kinds of VLAN dev path:
> pppoe-->eth
> pppoe-->br0.100-->br0(vlan filter enable)-->eth
> pppoe-->eth.100-->eth
> 
> The packet xmit and recv offload to the 'eth' in both original and
> reply direction.

This should provide a significant speed up in packet forwarding, since
packets are not passed up to userspace anymore via ppp driver.

BTW, what userspace ppp software are you using to test this?

Re: Re: [PATCH nf-next v2 1/3] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenxu]

>On Thu, May 26, 2022 at 02:57:30AM -0400, wenxu@chinatelecom.cn wrote:



>[...]



>> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c



>> index b350fe9..5da651d 100644



>> --- a/net/netfilter/nf_flow_table_ip.c



>> +++ b/net/netfilter/nf_flow_table_ip.c



>> @@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto,



>>  	return false;



>>  }



>>  



>> +static void nf_flow_encap_push(struct sk_buff *skb,



>> +			       struct flow_offload_tuple_rhash *tuplehash)



>> +{



>> +	int i;



>> +



>> +	for (i = 0; i < tuplehash->tuple.encap_num; i++) {



>> +		switch (tuplehash->tuple.encap[i].proto) {



>> +		case htons(ETH_P_8021Q):



>> +		case htons(ETH_P_8021AD):



>> +			skb_vlan_push(skb,



>



>Nit: skb_vlan_push() might fail.



>



>> +				      tuplehash->tuple.encap[i].proto,



>> +				      tuplehash->tuple.encap[i].id);



>> +			break;



>> +		}



>> +	}



>> +}



>



>If I understand correctly, the goal of this patchset is to move the



>existing vlan and ppp support to use the XMIT_DIRECT path?



>



>So this already works but you would prefer to not use XMIT_NEIGH?



>



>The scenarios you describe already work fine with the existing



>codebase? I am assuming 'eth' provides Internet access? You refer to



>this in the patch description:

The eth is the lower device of the bridge.
                     router
                 |------------|                           
   eth0-->br0               eth-internet

Without this patch the packet come from eth-internet will
always send through the router interface br0 with XMIT_NEIGH.

With this patch the packet come from eth-internet will send through
eth0 directly with XMIT_DIRECT(with vlan tag if need). 
So it can totally bypass the bridge process for ingress packet.


>



> br0.100-->br0(vlan filter enable)-->eth



> br0(vlan filter enable)-->eth



> br0(vlan filter disable)-->eth.100-->eth



>

Re: Re: [PATCH nf-next v2 1/3] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenxu]

>> +static void nf_flow_encap_push(struct sk_buff *skb,
>> +			       struct flow_offload_tuple_rhash *tuplehash)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < tuplehash->tuple.encap_num; i++) {
>> +		switch (tuplehash->tuple.encap[i].proto) {
>> +		case htons(ETH_P_8021Q):
>> +		case htons(ETH_P_8021AD):
>> +			skb_vlan_push(skb,
>
>Nit: skb_vlan_push() might fail.



>



The packet maybe modified.  So maybe only drop this packet if skb_vlan_push failed? 

Re: Re: [PATCH nf-next v2 2/3] nf_flow_table_offload: offload the PPPoE encap in the flowtable

[wenxu]

>> 



>> It can support all kinds of VLAN dev path:



>> pppoe-->eth



>> pppoe-->br0.100-->br0(vlan filter enable)-->eth



>> pppoe-->eth.100-->eth



>> 



>> The packet xmit and recv offload to the 'eth' in both original and



>> reply direction.



>



>This should provide a significant speed up in packet forwarding, since



>packets are not passed up to userspace anymore via ppp driver.



>



>BTW, what userspace ppp software are you using to test this?







ppp.x86_64 : The Point-to-Point Protocol daemon