| Message ID | 20210611035733.400713-1-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: b53: Create default VLAN entry explicitly | 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, 21 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Thu, Jun 10, 2021 at 08:57:32PM -0700, Florian Fainelli wrote: > In case CONFIG_VLAN_8021Q is not set, there will be no call down to the > b53 driver to ensure that the default PVID VLAN entry will be configured > with the appropriate untagged attribute towards the CPU port. We were > implicitly relying on dsa_slave_vlan_rx_add_vid() to do that for us, > instead make it explicit. > > Reported-by: Vladimir Oltean <olteanv@gmail.com> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/net/dsa/b53/b53_common.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c > index 6e199454e41d..5fd9ed327c1b 100644 > --- a/drivers/net/dsa/b53/b53_common.c > +++ b/drivers/net/dsa/b53/b53_common.c > @@ -748,9 +748,20 @@ int b53_configure_vlan(struct dsa_switch *ds) > > b53_enable_vlan(dev, -1, dev->vlan_enabled, ds->vlan_filtering); > > - b53_for_each_port(dev, i) > + /* Create an untagged VLAN entry for the default PVID in case > + * CONFIG_VLAN_8021Q is disabled and there are no calls to > + * dsa_slave_vlan_rx_add_vid() to create the default VLAN > + * entry. Do this only when the tagging protocol is not > + * DSA_TAG_PROTO_NONE > + */ > + b53_for_each_port(dev, i) { > + v = &dev->vlans[def_vid]; > + v->members |= BIT(i); > + if (dev->tag_protocol != DSA_TAG_PROTO_NONE) > + v->untag = v->members; > b53_write16(dev, B53_VLAN_PAGE, > B53_VLAN_PORT_DEF_TAG(i), def_vid); > + } > > /* Upon initial call we have not set-up any VLANs, but upon > * system resume, we need to restore all VLAN entries. > -- > 2.25.1 > So VLAN 0 is by default egress-tagged? This means that for tag_proto == DSA_TAG_PROTO_NONE, you are reintroducing the problem fixed by commit d965a5432d4c ("net: dsa: b53: Ensure the default VID is untagged"), aka untagged packets sent from the DSA master will land as VID-0-tagged on the wire? I would expect something like this would yield consistent results between the b53_configure_vlan and the b53_vlan_add code path, is that true? -----------------------------[ cut here ]----------------------------- diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 6e199454e41d..03456e019406 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -728,6 +728,13 @@ static u16 b53_default_pvid(struct b53_device *dev) return 0; } +static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port) +{ + struct b53_device *dev = ds->priv; + + return dev->tag_protocol == DSA_TAG_PROTO_NONE && dsa_is_cpu_port(ds, port); +} + int b53_configure_vlan(struct dsa_switch *ds) { struct b53_device *dev = ds->priv; @@ -748,6 +755,21 @@ int b53_configure_vlan(struct dsa_switch *ds) b53_enable_vlan(dev, -1, dev->vlan_enabled, ds->vlan_filtering); + /* Create an untagged VLAN entry for the default PVID in case + * CONFIG_VLAN_8021Q is disabled and there are no calls to + * dsa_slave_vlan_rx_add_vid() to create the default VLAN + * entry. Do this only when the tagging protocol is not + * DSA_TAG_PROTO_NONE + */ + v = &dev->vlans[def_vid]; + + b53_for_each_port(dev, i) { + v->members |= BIT(i); + + if (!b53_vlan_port_needs_forced_tagged(ds, port)) + v->untag |= BIT(i); + } + b53_for_each_port(dev, i) b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), def_vid); @@ -1460,13 +1482,6 @@ static int b53_vlan_prepare(struct dsa_switch *ds, int port, return 0; } -static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port) -{ - struct b53_device *dev = ds->priv; - - return dev->tag_protocol == DSA_TAG_PROTO_NONE && dsa_is_cpu_port(ds, port); -} - int b53_vlan_add(struct dsa_switch *ds, int port, const struct switchdev_obj_port_vlan *vlan, struct netlink_ext_ack *extack) -----------------------------[ cut here ]-----------------------------
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 6e199454e41d..5fd9ed327c1b 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -748,9 +748,20 @@ int b53_configure_vlan(struct dsa_switch *ds) b53_enable_vlan(dev, -1, dev->vlan_enabled, ds->vlan_filtering); - b53_for_each_port(dev, i) + /* Create an untagged VLAN entry for the default PVID in case + * CONFIG_VLAN_8021Q is disabled and there are no calls to + * dsa_slave_vlan_rx_add_vid() to create the default VLAN + * entry. Do this only when the tagging protocol is not + * DSA_TAG_PROTO_NONE + */ + b53_for_each_port(dev, i) { + v = &dev->vlans[def_vid]; + v->members |= BIT(i); + if (dev->tag_protocol != DSA_TAG_PROTO_NONE) + v->untag = v->members; b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), def_vid); + } /* Upon initial call we have not set-up any VLANs, but upon * system resume, we need to restore all VLAN entries.
In case CONFIG_VLAN_8021Q is not set, there will be no call down to the b53 driver to ensure that the default PVID VLAN entry will be configured with the appropriate untagged attribute towards the CPU port. We were implicitly relying on dsa_slave_vlan_rx_add_vid() to do that for us, instead make it explicit. Reported-by: Vladimir Oltean <olteanv@gmail.com> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/net/dsa/b53/b53_common.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)