Discussion:
[PATCH 0/2] DSA tagging mismatches
Andrew Lunn
2014-10-22 23:35:16 UTC
Permalink
The second patch is a fix, which should be applied to -rc. It is
possible to get a DSA configuration which does not work. The patch
stops this happening.

The first patch detects this situation, and errors out the probe of
DSA, making it more obvious something is wrong. It is not required to
apply it -rc.

Andrew Lunn (2):
net: dsa: Error out on tagging protocol mismatches
dsa: mv88e6171: Fix tagging protocol/Kconfig

drivers/net/dsa/mv88e6171.c | 2 +-
net/dsa/dsa.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
--
2.1.1
Andrew Lunn
2014-10-22 23:35:18 UTC
Permalink
The mv88e6171 can support two different tagging protocols, DSA and
EDSA. The switch driver structure only allows one protocol to be
enumerated, and DSA was chosen. However the Kconfig entry ensures the
EDSA tagging code is built. With a minimal configuration, we then end
up with a mismatch. The probe is successful, EDSA tagging is used, but
the switch is configured for DSA, resulting in mangled packets.

Change the switch driver structure to enumerate EDSA, fixing the
mismatch.

Signed-off-by: Andrew Lunn <***@lunn.ch>
Fixes: 42f272539487 ("net: DSA: Marvell mv88e6171 switch driver")
---
drivers/net/dsa/mv88e6171.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 1020a7af67cf..78d8e876f3aa 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -395,7 +395,7 @@ static int mv88e6171_get_sset_count(struct dsa_switch *ds)
}

struct dsa_switch_driver mv88e6171_switch_driver = {
- .tag_protocol = DSA_TAG_PROTO_DSA,
+ .tag_protocol = DSA_TAG_PROTO_EDSA,
.priv_size = sizeof(struct mv88e6xxx_priv_state),
.probe = mv88e6171_probe,
.setup = mv88e6171_setup,
--
2.1.1
Andrew Lunn
2014-10-22 23:35:17 UTC
Permalink
If there is a mismatch between enabled tagging protocols and the
protocol the switch supports, error out, rather than continue with a
situation which is unlikely to work.

Signed-off-by: Andrew Lunn <***@lunn.ch>
cc: ***@intel.com
---
net/dsa/dsa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 22f34cf4cb27..8a31bd81a315 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -175,7 +175,8 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
break;
#endif
default:
- break;
+ ret = -ENOPROTOOPT;
+ goto out;
}

dst->tag_protocol = drv->tag_protocol;
--
2.1.1
Florian Fainelli
2014-10-22 23:46:52 UTC
Permalink
Post by Andrew Lunn
If there is a mismatch between enabled tagging protocols and the
protocol the switch supports, error out, rather than continue with a
situation which is unlikely to work.
---
net/dsa/dsa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 22f34cf4cb27..8a31bd81a315 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -175,7 +175,8 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
break;
#endif
- break;
+ ret = -ENOPROTOOPT;
+ goto out;
}
This prevents using a switch driver without tagging, which is something
that you might want to do (link setup, ethtool stats, EEE etc...).
--
Florian
Andrew Lunn
2014-10-23 13:32:03 UTC
Permalink
Post by Florian Fainelli
Post by Andrew Lunn
If there is a mismatch between enabled tagging protocols and the
protocol the switch supports, error out, rather than continue with a
situation which is unlikely to work.
---
net/dsa/dsa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 22f34cf4cb27..8a31bd81a315 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -175,7 +175,8 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
break;
#endif
- break;
+ ret = -ENOPROTOOPT;
+ goto out;
}
This prevents using a switch driver without tagging, which is something
that you might want to do (link setup, ethtool stats, EEE etc...).
Hi Florian

I didn't know that was a use case.

So i assume such a driver would use DSA_TAG_PROTO_NONE? So all i need
to do is add that as a case value to the switch statement, and we
should cover that use case, and still be able to detect a mismatch.

v2 patch soon.

Andrew

Loading...