lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 15 Mar 2023 08:31:48 +0100
From:   Maxime Chevallier <maxime.chevallier@...tlin.com>
To:     Sven Auhagen <Sven.Auhagen@...eatech.de>
Cc:     netdev@...r.kernel.org, mw@...ihalf.com, linux@...linux.org.uk,
        kuba@...nel.org, davem@...emloft.net
Subject: Re: [PATCH 1/3] net: mvpp2: classifier flow remove tagged

Hello Sven,

On Sat, 11 Mar 2023 08:09:48 +0100
Sven Auhagen <Sven.Auhagen@...eatech.de> wrote:

> The classifier attribute MVPP22_CLS_HEK_TAGGED
> has no effect in the definition and is filtered out by default.

It's been a while since I've worked in this, but it looks like the
non-tagged and tagged flows will start behaving the same way with this
patch, which isn't what we want. Offsets to extract the various fields
change based on the presence or not of a vlan tag, hence why we have
all these flow definitions.

Did you check that for example RSS still behaves correctly when using
tagged and untagged traffic on the same interface ?

I didn't test the QinQ at the time, so it's indeed likely that it
doesn't behave as expected, but removing the MVPP22_CLS_HEK_TAGGED
will cause issues if you start hashing inbound traffic based on the
vlan tag.

the MVPP22_CLS_HEK_TAGGED does have an effect, as it's defined a
(VLAN id | VLAN_PRI) hashing capability. Removing it will also break
the per-vlan-prio rxnfc steering.

Do you have more details on the use-case ? Do you wan to do RSS,
steering ?

I however do think that the missing frag flags are correct, and should
be sent in a separate patch.

Thanks,

Maxime

> Even if it is applied to the classifier, it would discard double
> or tripple tagged vlans.
> 
> Also add missing IP Fragmentation Flag.
> 
> Signed-off-by: Sven Auhagen <sven.auhagen@...eatech.de>
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c index
> 41d935d1aaf6..efdf8d30f438 100644 ---
> a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c +++
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c @@ -44,17 +44,17 @@
> static const struct mvpp2_cls_flow cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* TCP over IPv4 flows, Not fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4 | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4_OPT | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_L4_TCP, MVPP2_PRS_IP_MASK),
>  
> @@ -62,35 +62,38 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4,
> MVPP2_FL_IP4_TCP_FRAG_UNTAG, MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE | MVPP2_PRS_RI_L3_IP4 |
> -		       MVPP2_PRS_RI_L4_TCP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_TCP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_FRAG_UNTAG,
>  		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE |
> MVPP2_PRS_RI_L3_IP4_OPT |
> -		       MVPP2_PRS_RI_L4_TCP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_TCP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_FRAG_UNTAG,
>  		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE |
> MVPP2_PRS_RI_L3_IP4_OTHER |
> -		       MVPP2_PRS_RI_L4_TCP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_TCP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	/* TCP over IPv4 flows, fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4 | MVPP2_PRS_RI_L4_TCP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4 |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4_OPT | MVPP2_PRS_RI_L4_TCP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4_OPT |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP4, MVPP2_FL_IP4_TCP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_L4_TCP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	/* UDP over IPv4 flows, Not fragmented, no vlan tag */
> @@ -114,17 +117,17 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* UDP over IPv4 flows, Not fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4 | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4_OPT | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP4_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_5T,
>  		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_L4_UDP, MVPP2_PRS_IP_MASK),
>  
> @@ -132,35 +135,38 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4,
> MVPP2_FL_IP4_UDP_FRAG_UNTAG, MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE | MVPP2_PRS_RI_L3_IP4 |
> -		       MVPP2_PRS_RI_L4_UDP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_UDP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_FRAG_UNTAG,
>  		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE |
> MVPP2_PRS_RI_L3_IP4_OPT |
> -		       MVPP2_PRS_RI_L4_UDP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_UDP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_FRAG_UNTAG,
>  		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_VLAN_NONE |
> MVPP2_PRS_RI_L3_IP4_OTHER |
> -		       MVPP2_PRS_RI_L4_UDP,
> +		       MVPP2_PRS_RI_IP_FRAG_TRUE |
> MVPP2_PRS_RI_L4_UDP, MVPP2_PRS_IP_MASK | MVPP2_PRS_RI_VLAN_MASK),
>  
>  	/* UDP over IPv4 flows, fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4 | MVPP2_PRS_RI_L4_UDP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4 |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4_OPT | MVPP2_PRS_RI_L4_UDP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4_OPT |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP4, MVPP2_FL_IP4_UDP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> -		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_L4_UDP,
> +		       MVPP22_CLS_HEK_IP4_2T,
> +		       MVPP2_PRS_RI_L3_IP4_OTHER |
> MVPP2_PRS_RI_IP_FRAG_TRUE |
> +			   MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	/* TCP over IPv6 flows, not fragmented, no vlan tag */
> @@ -178,12 +184,12 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* TCP over IPv6 flows, not fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP6, MVPP2_FL_IP6_TCP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP6_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_5T,
>  		       MVPP2_PRS_RI_L3_IP6 | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP6, MVPP2_FL_IP6_TCP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP6_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_5T,
>  		       MVPP2_PRS_RI_L3_IP6_EXT | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
> @@ -202,13 +208,13 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* TCP over IPv6 flows, fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP6, MVPP2_FL_IP6_TCP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6 |
> MVPP2_PRS_RI_IP_FRAG_TRUE | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_TCP6, MVPP2_FL_IP6_TCP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6_EXT |
> MVPP2_PRS_RI_IP_FRAG_TRUE | MVPP2_PRS_RI_L4_TCP,
>  		       MVPP2_PRS_IP_MASK),
> @@ -228,12 +234,12 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* UDP over IPv6 flows, not fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP6, MVPP2_FL_IP6_UDP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP6_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_5T,
>  		       MVPP2_PRS_RI_L3_IP6 | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP6, MVPP2_FL_IP6_UDP_NF_TAG,
> -		       MVPP22_CLS_HEK_IP6_5T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_5T,
>  		       MVPP2_PRS_RI_L3_IP6_EXT | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
> @@ -252,13 +258,13 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* UDP over IPv6 flows, fragmented, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP6, MVPP2_FL_IP6_UDP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6 |
> MVPP2_PRS_RI_IP_FRAG_TRUE | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
>  
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_UDP6, MVPP2_FL_IP6_UDP_FRAG_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6_EXT |
> MVPP2_PRS_RI_IP_FRAG_TRUE | MVPP2_PRS_RI_L4_UDP,
>  		       MVPP2_PRS_IP_MASK),
> @@ -279,15 +285,15 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* IPv4 flows, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_IP4, MVPP2_FL_IP4_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_L3_IP4,
>  		       MVPP2_PRS_RI_L3_PROTO_MASK),
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_IP4, MVPP2_FL_IP4_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_L3_IP4_OPT,
>  		       MVPP2_PRS_RI_L3_PROTO_MASK),
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_IP4, MVPP2_FL_IP4_TAG,
> -		       MVPP22_CLS_HEK_IP4_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP4_2T,
>  		       MVPP2_PRS_RI_L3_IP4_OTHER,
>  		       MVPP2_PRS_RI_L3_PROTO_MASK),
>  
> @@ -303,11 +309,11 @@ static const struct mvpp2_cls_flow
> cls_flows[MVPP2_N_PRS_FLOWS] = { 
>  	/* IPv6 flows, with vlan tag */
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_IP6, MVPP2_FL_IP6_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6,
>  		       MVPP2_PRS_RI_L3_PROTO_MASK),
>  	MVPP2_DEF_FLOW(MVPP22_FLOW_IP6, MVPP2_FL_IP6_TAG,
> -		       MVPP22_CLS_HEK_IP6_2T | MVPP22_CLS_HEK_TAGGED,
> +		       MVPP22_CLS_HEK_IP6_2T,
>  		       MVPP2_PRS_RI_L3_IP6,
>  		       MVPP2_PRS_RI_L3_PROTO_MASK),
>  

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ