netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue
@ 2021-04-09  5:03 wenxu
  2021-04-09  8:27 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: wenxu @ 2021-04-09  5:03 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

From: wenxu <wenxu@ucloud.cn>

For the vlan packet the h_vlan_encapsulated_proto should be set
on the flow_dissector_key_basic->n_porto flow_dissector.

Fixes: a82055af5959 ("netfilter: nft_payload: add VLAN offload support")
Fixes: 89d8fd44abfb ("netfilter: nft_payload: add C-VLAN offload support")
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 net/netfilter/nft_payload.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index cb1c8c2..84c5ecc 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -233,8 +233,8 @@ static int nft_payload_offload_ll(struct nft_offload_ctx *ctx,
 		if (!nft_payload_offload_mask(reg, priv->len, sizeof(__be16)))
 			return -EOPNOTSUPP;
 
-		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_VLAN, vlan,
-				  vlan_tpid, sizeof(__be16), reg);
+		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_BASIC, basic,
+				  n_proto, sizeof(__be16), reg);
 		nft_offload_set_dependency(ctx, NFT_OFFLOAD_DEP_NETWORK);
 		break;
 	case offsetof(struct vlan_ethhdr, h_vlan_TCI) + sizeof(struct vlan_hdr):
@@ -249,8 +249,8 @@ static int nft_payload_offload_ll(struct nft_offload_ctx *ctx,
 		if (!nft_payload_offload_mask(reg, priv->len, sizeof(__be16)))
 			return -EOPNOTSUPP;
 
-		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_CVLAN, vlan,
-				  vlan_tpid, sizeof(__be16), reg);
+		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_BASIC, basic,
+				  n_proto, sizeof(__be16), reg);
 		break;
 	default:
 		return -EOPNOTSUPP;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue
  2021-04-09  5:03 [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue wenxu
@ 2021-04-09  8:27 ` Pablo Neira Ayuso
  2021-04-09  9:31   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2021-04-09  8:27 UTC (permalink / raw)
  To: wenxu; +Cc: netfilter-devel

On Fri, Apr 09, 2021 at 01:03:49PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> For the vlan packet the h_vlan_encapsulated_proto should be set
> on the flow_dissector_key_basic->n_porto flow_dissector.
> 
> Fixes: a82055af5959 ("netfilter: nft_payload: add VLAN offload support")
> Fixes: 89d8fd44abfb ("netfilter: nft_payload: add C-VLAN offload support")
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  net/netfilter/nft_payload.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> index cb1c8c2..84c5ecc 100644
> --- a/net/netfilter/nft_payload.c
> +++ b/net/netfilter/nft_payload.c
> @@ -233,8 +233,8 @@ static int nft_payload_offload_ll(struct nft_offload_ctx *ctx,
>  		if (!nft_payload_offload_mask(reg, priv->len, sizeof(__be16)))
>  			return -EOPNOTSUPP;
>  
> -		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_VLAN, vlan,
> -				  vlan_tpid, sizeof(__be16), reg);
> +		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_BASIC, basic,
> +				  n_proto, sizeof(__be16), reg);

nftables already sets KEY_BASIC accordingly to 0x8100.

# nft --debug=netlink add rule netdev x y vlan id 100
netdev
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 2b @ link header + 12 => reg 1 ]
  [ cmp eq reg 1 0x00000081 ] <----------------------------- HERE
  [ payload load 2b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
  [ cmp eq reg 1 0x00006400 ]

What are you trying to fix?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue
  2021-04-09  8:27 ` Pablo Neira Ayuso
@ 2021-04-09  9:31   ` Pablo Neira Ayuso
  2021-04-09 14:28     ` wenxu
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2021-04-09  9:31 UTC (permalink / raw)
  To: wenxu; +Cc: netfilter-devel

On Fri, Apr 09, 2021 at 10:27:17AM +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 09, 2021 at 01:03:49PM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> > 
> > For the vlan packet the h_vlan_encapsulated_proto should be set
> > on the flow_dissector_key_basic->n_porto flow_dissector.
> > 
> > Fixes: a82055af5959 ("netfilter: nft_payload: add VLAN offload support")
> > Fixes: 89d8fd44abfb ("netfilter: nft_payload: add C-VLAN offload support")
> > Signed-off-by: wenxu <wenxu@ucloud.cn>
> > ---
> >  net/netfilter/nft_payload.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> > index cb1c8c2..84c5ecc 100644
> > --- a/net/netfilter/nft_payload.c
> > +++ b/net/netfilter/nft_payload.c
> > @@ -233,8 +233,8 @@ static int nft_payload_offload_ll(struct nft_offload_ctx *ctx,
> >  		if (!nft_payload_offload_mask(reg, priv->len, sizeof(__be16)))
> >  			return -EOPNOTSUPP;
> >  
> > -		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_VLAN, vlan,
> > -				  vlan_tpid, sizeof(__be16), reg);
> > +		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_BASIC, basic,
> > +				  n_proto, sizeof(__be16), reg);
> 
> nftables already sets KEY_BASIC accordingly to 0x8100.
> 
> # nft --debug=netlink add rule netdev x y vlan id 100
> netdev
>   [ meta load iiftype => reg 1 ]
>   [ cmp eq reg 1 0x00000001 ]
>   [ payload load 2b @ link header + 12 => reg 1 ]
>   [ cmp eq reg 1 0x00000081 ] <----------------------------- HERE
>   [ payload load 2b @ link header + 14 => reg 1 ]
>   [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
>   [ cmp eq reg 1 0x00006400 ]
> 
> What are you trying to fix?

Could you provide a rule that works for tc offload with vlan? I'd like
to check what internal representation is triggering in the kernel.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue
  2021-04-09  9:31   ` Pablo Neira Ayuso
@ 2021-04-09 14:28     ` wenxu
  0 siblings, 0 replies; 4+ messages in thread
From: wenxu @ 2021-04-09 14:28 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel


在 2021/4/9 17:31, Pablo Neira Ayuso 写道:
> On Fri, Apr 09, 2021 at 10:27:17AM +0200, Pablo Neira Ayuso wrote:
>> On Fri, Apr 09, 2021 at 01:03:49PM +0800, wenxu@ucloud.cn wrote:
>>> From: wenxu <wenxu@ucloud.cn>
>>>
>>> For the vlan packet the h_vlan_encapsulated_proto should be set
>>> on the flow_dissector_key_basic->n_porto flow_dissector.
>>>
>>> Fixes: a82055af5959 ("netfilter: nft_payload: add VLAN offload support")
>>> Fixes: 89d8fd44abfb ("netfilter: nft_payload: add C-VLAN offload support")
>>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>>> ---
>>>  net/netfilter/nft_payload.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
>>> index cb1c8c2..84c5ecc 100644
>>> --- a/net/netfilter/nft_payload.c
>>> +++ b/net/netfilter/nft_payload.c
>>> @@ -233,8 +233,8 @@ static int nft_payload_offload_ll(struct nft_offload_ctx *ctx,
>>>  		if (!nft_payload_offload_mask(reg, priv->len, sizeof(__be16)))
>>>  			return -EOPNOTSUPP;
>>>  
>>> -		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_VLAN, vlan,
>>> -				  vlan_tpid, sizeof(__be16), reg);
>>> +		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_BASIC, basic,
>>> +				  n_proto, sizeof(__be16), reg);
>> nftables already sets KEY_BASIC accordingly to 0x8100.
>>
>> # nft --debug=netlink add rule netdev x y vlan id 100
>> netdev
>>   [ meta load iiftype => reg 1 ]
>>   [ cmp eq reg 1 0x00000001 ]
>>   [ payload load 2b @ link header + 12 => reg 1 ]
>>   [ cmp eq reg 1 0x00000081 ] <----------------------------- HERE
>>   [ payload load 2b @ link header + 14 => reg 1 ]
>>   [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
>>   [ cmp eq reg 1 0x00006400 ]
>>
>> What are you trying to fix?

First the vlan_tpid of KEY_VLAN is not the representation h_vlan_encapsulated_proto, So this

need be fixed. Just see the fl_set_key in the cls_flower.c

fl_set_key-->fl_set_key_vlan(pass the ethernet type of vlan to the vlan_tpid which normal is 0x8100)


Then if the rule match the h_vlan_encapsulated_proto(normally ipv4/6), The h_vlan_encapsulated_proto

will be set to the n_proto of BASIC_KEY. Also see the fl_set_key in the cls_flower.c


     if (tb[TCA_FLOWER_KEY_ETH_TYPE]) {
                ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);

                if (eth_type_vlan(ethertype)) {
                        fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID,
                                        TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan,
                                        &mask->vlan);

                        if (tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]) {  <----------------------------- HERE

                                ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]);
                                if (eth_type_vlan(ethertype)) {
                                        fl_set_key_vlan(tb, ethertype,
                                                        TCA_FLOWER_KEY_CVLAN_ID,
                                                        TCA_FLOWER_KEY_CVLAN_PRIO,
                                                        &key->cvlan, &mask->cvlan);
                                        fl_set_key_val(tb, &key->basic.n_proto,
                                                       TCA_FLOWER_KEY_CVLAN_ETH_TYPE, <----------------------------- HERE
                                                       &mask->basic.n_proto,
                                                       TCA_FLOWER_UNSPEC,
                                                       sizeof(key->basic.n_proto));
                                } else {
                                        key->basic.n_proto = ethertype;  <----------------------------- HERE

                                        mask->basic.n_proto = cpu_to_be16(~0);
                                }   
                        }   
                } else {
                        key->basic.n_proto = ethertype;
                        mask->basic.n_proto = cpu_to_be16(~0);
                }   
        }   


BR

wenxu

> Could you provide a rule that works for tc offload with vlan? I'd like
> to check what internal representation is triggering in the kernel.
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-09 14:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-09  5:03 [PATCH nf v2] netfilter: nft_payload: fix the h_vlan_encapsulated_proto flow_dissector vlaue wenxu
2021-04-09  8:27 ` Pablo Neira Ayuso
2021-04-09  9:31   ` Pablo Neira Ayuso
2021-04-09 14:28     ` wenxu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).