[PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenx05124561]

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 |  7 +++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 32c0eb1..99ae2550 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -282,6 +282,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)
 {
@@ -403,6 +420,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);
@@ -659,6 +677,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 900d48c..f9837c9 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -119,12 +119,15 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
 				info->indev = NULL;
 				break;
 			}
-			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) {
+				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

Re: [PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[Pablo Neira Ayuso]

On Tue, Apr 05, 2022 at 10:38:35AM -0400, wenx05124561@163.com wrote:
> 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

I assume this eth is a bridge port.

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

This is an enhancement or fix?

Is this going to work for VLAN + PPP?

Would you update tools/testing/selftests/netfilter/nft_flowtable.sh to
cover bridge filtering usecase? It could be done in a follow up patch.

> Signed-off-by: wenxu <wenxu@chinatelecom.cn>
> ---
>  net/netfilter/nf_flow_table_ip.c | 19 +++++++++++++++++++
>  net/netfilter/nft_flow_offload.c |  7 +++++--
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index 32c0eb1..99ae2550 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -282,6 +282,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)
>  {
> @@ -403,6 +420,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);
> @@ -659,6 +677,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 900d48c..f9837c9 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -119,12 +119,15 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
>  				info->indev = NULL;
>  				break;
>  			}
> -			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) {
> +				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
> 

Re: [PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenxu]

On 2022/4/27 22:10, Pablo Neira Ayuso wrote:
> On Tue, Apr 05, 2022 at 10:38:35AM -0400, wenx05124561@163.com wrote:
>> 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
> I assume this eth is a bridge port.

Yes it is. And it also can support the case without bridge as following.

eth.100-->eth.

>
>> The packet xmit and recv offload to the 'eth' in both original and
>> reply direction.
> This is an enhancement or fix?
It's an enhancement and  it make the vlan packet can offload through the real dev.
>
> Is this going to work for VLAN + PPP?
>
> Would you update tools/testing/selftests/netfilter/nft_flowtable.sh to
> cover bridge filtering usecase? It could be done in a follow up patch.
I will do for both  if this patch reivew ok .
>
>> Signed-off-by: wenxu <wenxu@chinatelecom.cn>
>> ---
>>  net/netfilter/nf_flow_table_ip.c | 19 +++++++++++++++++++
>>  net/netfilter/nft_flow_offload.c |  7 +++++--
>>  2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
>> index 32c0eb1..99ae2550 100644
>> --- a/net/netfilter/nf_flow_table_ip.c
>> +++ b/net/netfilter/nf_flow_table_ip.c
>> @@ -282,6 +282,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)
>>  {
>> @@ -403,6 +420,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);
>> @@ -659,6 +677,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 900d48c..f9837c9 100644
>> --- a/net/netfilter/nft_flow_offload.c
>> +++ b/net/netfilter/nft_flow_offload.c
>> @@ -119,12 +119,15 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
>>  				info->indev = NULL;
>>  				break;
>>  			}
>> -			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) {
>> +				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
>>

Re: [PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[Pablo Neira Ayuso]

On Wed, Apr 27, 2022 at 11:28:16PM +0800, wenxu wrote:
> 
> On 2022/4/27 22:10, Pablo Neira Ayuso wrote:
> > On Tue, Apr 05, 2022 at 10:38:35AM -0400, wenx05124561@163.com wrote:
> >> 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
> > I assume this eth is a bridge port.
> 
> Yes it is. And it also can support the case without bridge as following.
> 
> eth.100-->eth.
> 
> >
> >> The packet xmit and recv offload to the 'eth' in both original and
> >> reply direction.
> > This is an enhancement or fix?
>
> It's an enhancement and  it make the vlan packet can offload through the real dev.

What's the benefit from the existing approach?

> > Is this going to work for VLAN + PPP?
> >
> > Would you update tools/testing/selftests/netfilter/nft_flowtable.sh to
> > cover bridge filtering usecase? It could be done in a follow up patch.
>
> I will do for both  if this patch reivew ok .

OK.

Re: [PATCH nf-next] nf_flow_table_offload: offload the vlan encap in the flowtable

[wenxu]

On 2022/4/27 23:55, Pablo Neira Ayuso wrote:
> On Wed, Apr 27, 2022 at 11:28:16PM +0800, wenxu wrote:
>> On 2022/4/27 22:10, Pablo Neira Ayuso wrote:
>>> On Tue, Apr 05, 2022 at 10:38:35AM -0400, wenx05124561@163.com wrote:
>>>> 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
>>> I assume this eth is a bridge port.
>> Yes it is. And it also can support the case without bridge as following.
>>
>> eth.100-->eth.
>>
>>>> The packet xmit and recv offload to the 'eth' in both original and
>>>> reply direction.
>>> This is an enhancement or fix?
>> It's an enhancement and  it make the vlan packet can offload through the real dev.
> What's the benefit from the existing approach?

For the simplest case

eth.100 base on eth


eth.100  and ethx are route interface.

Without this patch.

The packet outgoing path from eth   --- >  ethx

The packet incoming path from ethx ---->  eth.100---> eth

 With this patch

The packet incoming path from ethx   (direct to)---> eth, it is the same with outgoing.

>
>>> Is this going to work for VLAN + PPP?
>>>
>>> Would you update tools/testing/selftests/netfilter/nft_flowtable.sh to
>>> cover bridge filtering usecase? It could be done in a follow up patch.
>> I will do for both  if this patch reivew ok .
> OK.