| 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
| ||
|
Date: Wed, 15 Mar 2023 08:53:30 +0100
From: Sven Auhagen <sven.auhagen@...eatech.de>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
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
On Wed, Mar 15, 2023 at 08:31:48AM +0100, Maxime Chevallier wrote:
> Hello Sven,
Hello Maxime,
>
> 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.
>
In the sense of a match kind of.
There is one classifier match for no VLAN and one for any number of VLANs.
So no VLAN will match twice, correct.
This is the case right now anyway though since mvpp2_port_rss_hash_opts_set
is filtering out MVPP22_CLS_HEK_TAGGED for all rss hash options that
are set in the driver at the moment.
MVPP22_CLS_HEK_TAGGED is also not compatible with MVPP22_CLS_HEK_IP4_5T
which is probably the reason it is filtered out.
The HEK can only have a match on up to 4 fields.
MVPP22_CLS_HEK_IP4_5T already covers 4 fields.
I disabled the hash_opts line which removes the HEK_TAGGED and the entire
rule will fail out and is not added because of that.
So I am simply removing what is not working in the first place.
> Did you check that for example RSS still behaves correctly when using
> tagged and untagged traffic on the same interface ?
>
Yes all RSS work fine, I tested no VLAN, VLAN, QinQ, PPPoE and VLAN + PPPoE.
> 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.
Please see my comment above.
>
> 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 want to have RSS steering for all cases I tested above.
Do you have a different place that I do not know of where
MVPP22_CLS_HEK_TAGGED is actually loaded?
>
> I however do think that the missing frag flags are correct, and should
> be sent in a separate patch.
>
Will do that in v2.
> 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