* [PATCH net-next 0/2] net: dsa: b53: Remove unconditional CPU VLAN tagging @ 2021-06-07 22:08 Florian Fainelli 2021-06-07 22:08 ` [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs Florian Fainelli 2021-06-07 22:08 ` [PATCH net-next 2/2] net: dsa: Remove bridge PVID untagging Florian Fainelli 0 siblings, 2 replies; 6+ messages in thread From: Florian Fainelli @ 2021-06-07 22:08 UTC (permalink / raw) To: netdev Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, open list, Matthew Hagan This patch series removes the unconditional egress tagging of VLANs added to a b53 switch's CPU port. This had led to adding the untag_bridge_pvid attribute in the DSA framework and is causing problems with stacking of switches such as the configuration being used by Matthew. Since we use Broadcom tags for all of the supported switches nowadays the original situation that led to doing this unconditional egress tagging is no longer warranted. Florian Fainelli (2): net: dsa: b53: Do not force tagging on CPU port VLANs net: dsa: Remove bridge PVID untagging drivers/net/dsa/b53/b53_common.c | 5 ++- include/net/dsa.h | 8 ----- net/dsa/dsa.c | 9 ----- net/dsa/dsa_priv.h | 59 -------------------------------- 4 files changed, 2 insertions(+), 79 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs 2021-06-07 22:08 [PATCH net-next 0/2] net: dsa: b53: Remove unconditional CPU VLAN tagging Florian Fainelli @ 2021-06-07 22:08 ` Florian Fainelli 2021-06-07 22:22 ` Vladimir Oltean 2021-06-07 22:08 ` [PATCH net-next 2/2] net: dsa: Remove bridge PVID untagging Florian Fainelli 1 sibling, 1 reply; 6+ messages in thread From: Florian Fainelli @ 2021-06-07 22:08 UTC (permalink / raw) To: netdev Cc: Florian Fainelli, Matthew Hagan, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, open list 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs 2021-06-07 22:08 ` [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs Florian Fainelli @ 2021-06-07 22:22 ` Vladimir Oltean 2021-06-07 22:31 ` Florian Fainelli 0 siblings, 1 reply; 6+ messages in thread From: Vladimir Oltean @ 2021-06-07 22:22 UTC (permalink / raw) To: Florian Fainelli Cc: netdev, Matthew Hagan, Andrew Lunn, Vivien Didelot, David S. Miller, Jakub Kicinski, open list 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs 2021-06-07 22:22 ` Vladimir Oltean @ 2021-06-07 22:31 ` Florian Fainelli 2021-06-07 22:36 ` Florian Fainelli 0 siblings, 1 reply; 6+ messages in thread From: Florian Fainelli @ 2021-06-07 22:31 UTC (permalink / raw) To: Vladimir Oltean Cc: netdev, Matthew Hagan, Andrew Lunn, Vivien Didelot, David S. Miller, Jakub Kicinski, open list 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. -- Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs 2021-06-07 22:31 ` Florian Fainelli @ 2021-06-07 22:36 ` Florian Fainelli 0 siblings, 0 replies; 6+ messages in thread From: Florian Fainelli @ 2021-06-07 22:36 UTC (permalink / raw) To: Vladimir Oltean Cc: netdev, Matthew Hagan, Andrew Lunn, Vivien Didelot, David S. Miller, Jakub Kicinski, open list 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. -- Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] net: dsa: Remove bridge PVID untagging 2021-06-07 22:08 [PATCH net-next 0/2] net: dsa: b53: Remove unconditional CPU VLAN tagging Florian Fainelli 2021-06-07 22:08 ` [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs Florian Fainelli @ 2021-06-07 22:08 ` Florian Fainelli 1 sibling, 0 replies; 6+ messages in thread From: Florian Fainelli @ 2021-06-07 22:08 UTC (permalink / raw) To: netdev Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, open list, Matthew Hagan Now that the only user of the bridge PVID untagging code is no longer forcing the PVID untagged VLAN configuration to be PVID egress tagged for the CPU, we no longer need this support code to pop the VLAN tag automatically. Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/net/dsa/b53/b53_common.c | 1 - include/net/dsa.h | 8 ----- net/dsa/dsa.c | 9 ----- net/dsa/dsa_priv.h | 59 -------------------------------- 4 files changed, 77 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 56e3b42ec28c..dc43dadd6d31 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2660,7 +2660,6 @@ struct b53_device *b53_switch_alloc(struct device *base, dev->priv = priv; dev->ops = ops; ds->ops = &b53_switch_ops; - ds->untag_bridge_pvid = true; dev->vlan_enabled = true; /* Let DSA handle the case were multiple bridges span the same switch * device and different VLAN awareness settings are requested, which diff --git a/include/net/dsa.h b/include/net/dsa.h index e1a2610a0e06..216443820a7e 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -355,14 +355,6 @@ struct dsa_switch { */ bool configure_vlan_while_not_filtering; - /* If the switch driver always programs the CPU port as egress tagged - * despite the VLAN configuration indicating otherwise, then setting - * @untag_bridge_pvid will force the DSA receive path to pop the bridge's - * default_pvid VLAN tagged frames to offer a consistent behavior - * between a vlan_filtering=0 and vlan_filtering=1 bridge device. - */ - bool untag_bridge_pvid; - /* Let DSA manage the FDB entries towards the CPU, based on the * software bridge database. */ diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 84cad1be9ce4..daac329b6e93 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -260,15 +260,6 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev, p = netdev_priv(skb->dev); - if (unlikely(cpu_dp->ds->untag_bridge_pvid)) { - nskb = dsa_untag_bridge_pvid(skb); - if (!nskb) { - kfree_skb(skb); - return 0; - } - skb = nskb; - } - dev_sw_netstats_rx_add(skb->dev, skb->len); if (dsa_skb_defer_rx_timestamp(p, skb)) diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 92282de54230..08d915d951b0 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -290,65 +290,6 @@ dsa_slave_to_master(const struct net_device *dev) return dp->cpu_dp->master; } -/* If under a bridge with vlan_filtering=0, make sure to send pvid-tagged - * frames as untagged, since the bridge will not untag them. - */ -static inline struct sk_buff *dsa_untag_bridge_pvid(struct sk_buff *skb) -{ - struct dsa_port *dp = dsa_slave_to_port(skb->dev); - struct net_device *br = dp->bridge_dev; - struct net_device *dev = skb->dev; - struct net_device *upper_dev; - u16 vid, pvid, proto; - int err; - - if (!br || br_vlan_enabled(br)) - return skb; - - err = br_vlan_get_proto(br, &proto); - if (err) - return skb; - - /* Move VLAN tag from data to hwaccel */ - if (!skb_vlan_tag_present(skb) && skb->protocol == htons(proto)) { - skb = skb_vlan_untag(skb); - if (!skb) - return NULL; - } - - if (!skb_vlan_tag_present(skb)) - return skb; - - vid = skb_vlan_tag_get_id(skb); - - /* We already run under an RCU read-side critical section since - * we are called from netif_receive_skb_list_internal(). - */ - err = br_vlan_get_pvid_rcu(dev, &pvid); - if (err) - return skb; - - if (vid != pvid) - return skb; - - /* The sad part about attempting to untag from DSA is that we - * don't know, unless we check, if the skb will end up in - * the bridge's data path - br_allowed_ingress() - or not. - * For example, there might be an 8021q upper for the - * default_pvid of the bridge, which will steal VLAN-tagged traffic - * from the bridge's data path. This is a configuration that DSA - * supports because vlan_filtering is 0. In that case, we should - * definitely keep the tag, to make sure it keeps working. - */ - upper_dev = __vlan_find_dev_deep_rcu(br, htons(proto), vid); - if (upper_dev) - return skb; - - __vlan_hwaccel_clear_tag(skb); - - return skb; -} - /* switch.c */ int dsa_switch_register_notifier(struct dsa_switch *ds); void dsa_switch_unregister_notifier(struct dsa_switch *ds); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-07 22:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-07 22:08 [PATCH net-next 0/2] net: dsa: b53: Remove unconditional CPU VLAN tagging Florian Fainelli 2021-06-07 22:08 ` [PATCH net-next 1/2] net: dsa: b53: Do not force tagging on CPU port VLANs Florian Fainelli 2021-06-07 22:22 ` Vladimir Oltean 2021-06-07 22:31 ` Florian Fainelli 2021-06-07 22:36 ` Florian Fainelli 2021-06-07 22:08 ` [PATCH net-next 2/2] net: dsa: Remove bridge PVID untagging Florian Fainelli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).