| Message ID | 20210607220843.3799414-2-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: b53: Remove unconditional CPU VLAN tagging | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 16 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Jun 07, 2021 at 03:08:42PM -0700, Florian Fainelli wrote: > Commit ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all > VLANs") forced the CPU port to be always tagged in any VLAN membership. > This was necessary back then because we did not support Broadcom tags > for all configurations so the only way to differentiate tagged and > untagged traffic while DSA_TAG_PROTO_NONE was used was to force the CPU > port into being always tagged. > > This is not necessary anymore since 8fab459e69ab ("net: dsa: b53: Enable > Broadcom tags for 531x5/539x families") and we can simply program what > we are being told now, regardless of the port being CPU or user-facing. > > Reported-by: Matthew Hagan <mnhagan88@gmail.com> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/net/dsa/b53/b53_common.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c > index 3ca6b394dd5f..56e3b42ec28c 100644 > --- a/drivers/net/dsa/b53/b53_common.c > +++ b/drivers/net/dsa/b53/b53_common.c > @@ -1477,7 +1477,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port, > untagged = true; > > vl->members |= BIT(port); > - if (untagged && !dsa_is_cpu_port(ds, port)) > + if (untagged) > vl->untag |= BIT(port); > else > vl->untag &= ~BIT(port); > @@ -1514,7 +1514,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port, > if (pvid == vlan->vid) > pvid = b53_default_pvid(dev); > > - if (untagged && !dsa_is_cpu_port(ds, port)) > + if (untagged) > vl->untag &= ~(BIT(port)); > > b53_set_vlan_entry(dev, vlan->vid, vl); > -- > 2.25.1 > Don't you want to keep this functionality for BCM5325 / BCM5365 and such, which still use DSA_TAG_PROTO_NONE?
On 6/7/2021 3:22 PM, Vladimir Oltean wrote: > On Mon, Jun 07, 2021 at 03:08:42PM -0700, Florian Fainelli wrote: >> Commit ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all >> VLANs") forced the CPU port to be always tagged in any VLAN membership. >> This was necessary back then because we did not support Broadcom tags >> for all configurations so the only way to differentiate tagged and >> untagged traffic while DSA_TAG_PROTO_NONE was used was to force the CPU >> port into being always tagged. >> >> This is not necessary anymore since 8fab459e69ab ("net: dsa: b53: Enable >> Broadcom tags for 531x5/539x families") and we can simply program what >> we are being told now, regardless of the port being CPU or user-facing. >> >> Reported-by: Matthew Hagan <mnhagan88@gmail.com> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> drivers/net/dsa/b53/b53_common.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c >> index 3ca6b394dd5f..56e3b42ec28c 100644 >> --- a/drivers/net/dsa/b53/b53_common.c >> +++ b/drivers/net/dsa/b53/b53_common.c >> @@ -1477,7 +1477,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port, >> untagged = true; >> >> vl->members |= BIT(port); >> - if (untagged && !dsa_is_cpu_port(ds, port)) >> + if (untagged) >> vl->untag |= BIT(port); >> else >> vl->untag &= ~BIT(port); >> @@ -1514,7 +1514,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port, >> if (pvid == vlan->vid) >> pvid = b53_default_pvid(dev); >> >> - if (untagged && !dsa_is_cpu_port(ds, port)) >> + if (untagged) >> vl->untag &= ~(BIT(port)); >> >> b53_set_vlan_entry(dev, vlan->vid, vl); >> -- >> 2.25.1 >> > > Don't you want to keep this functionality for BCM5325 / BCM5365 and > such, which still use DSA_TAG_PROTO_NONE? Humm, in premise yes, however I am debating removing support for 5325/5365 entirely, nobody that I know of has even been trying to get those devices to work with that driver.
On 6/7/2021 3:31 PM, Florian Fainelli wrote: > > > On 6/7/2021 3:22 PM, Vladimir Oltean wrote: >> On Mon, Jun 07, 2021 at 03:08:42PM -0700, Florian Fainelli wrote: >>> Commit ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all >>> VLANs") forced the CPU port to be always tagged in any VLAN membership. >>> This was necessary back then because we did not support Broadcom tags >>> for all configurations so the only way to differentiate tagged and >>> untagged traffic while DSA_TAG_PROTO_NONE was used was to force the CPU >>> port into being always tagged. >>> >>> This is not necessary anymore since 8fab459e69ab ("net: dsa: b53: Enable >>> Broadcom tags for 531x5/539x families") and we can simply program what >>> we are being told now, regardless of the port being CPU or user-facing. >>> >>> Reported-by: Matthew Hagan <mnhagan88@gmail.com> >>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >>> --- >>> drivers/net/dsa/b53/b53_common.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c >>> index 3ca6b394dd5f..56e3b42ec28c 100644 >>> --- a/drivers/net/dsa/b53/b53_common.c >>> +++ b/drivers/net/dsa/b53/b53_common.c >>> @@ -1477,7 +1477,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port, >>> untagged = true; >>> >>> vl->members |= BIT(port); >>> - if (untagged && !dsa_is_cpu_port(ds, port)) >>> + if (untagged) >>> vl->untag |= BIT(port); >>> else >>> vl->untag &= ~BIT(port); >>> @@ -1514,7 +1514,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port, >>> if (pvid == vlan->vid) >>> pvid = b53_default_pvid(dev); >>> >>> - if (untagged && !dsa_is_cpu_port(ds, port)) >>> + if (untagged) >>> vl->untag &= ~(BIT(port)); >>> >>> b53_set_vlan_entry(dev, vlan->vid, vl); >>> -- >>> 2.25.1 >>> >> >> Don't you want to keep this functionality for BCM5325 / BCM5365 and >> such, which still use DSA_TAG_PROTO_NONE? > > Humm, in premise yes, however I am debating removing support for > 5325/5365 entirely, nobody that I know of has even been trying to get > those devices to work with that driver. On second thought, we just need to have those devices return DSA_TAG_PROTO_BRCM_LEGACY which is what they use, there does appear to be a couple of users, including myself, I had not realized that the device I was using used a 5325 (thought it was a 53125). V2 coming.
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 3ca6b394dd5f..56e3b42ec28c 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1477,7 +1477,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port, untagged = true; vl->members |= BIT(port); - if (untagged && !dsa_is_cpu_port(ds, port)) + if (untagged) vl->untag |= BIT(port); else vl->untag &= ~BIT(port); @@ -1514,7 +1514,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port, if (pvid == vlan->vid) pvid = b53_default_pvid(dev); - if (untagged && !dsa_is_cpu_port(ds, port)) + if (untagged) vl->untag &= ~(BIT(port)); b53_set_vlan_entry(dev, vlan->vid, vl);
Commit ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all VLANs") forced the CPU port to be always tagged in any VLAN membership. This was necessary back then because we did not support Broadcom tags for all configurations so the only way to differentiate tagged and untagged traffic while DSA_TAG_PROTO_NONE was used was to force the CPU port into being always tagged. This is not necessary anymore since 8fab459e69ab ("net: dsa: b53: Enable Broadcom tags for 531x5/539x families") and we can simply program what we are being told now, regardless of the port being CPU or user-facing. Reported-by: Matthew Hagan <mnhagan88@gmail.com> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/net/dsa/b53/b53_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)