On Fri, Sep 11, 2020 at 12:48:37PM -0700, Florian Fainelli wrote:
> > > I'm conflicted. So you prefer having the CPU port as egress-tagged?
> >
> > I do, because I realized that some of the switches we support are still
> > configured in DSA_TAG_NONE mode because the CPU port they chose is not
> > Broadcom tag capable and there is an user out there who cares a lot
> > about that case not to "break".
> >
Ok.
> > > Also, I think I'll also experiment with a version of this patch that is
> > > local to the DSA RX path. The bridge people may not like it, and as far
> > > as I understand, only DSA has this situation where pvid-tagged traffic
> > > may end up with a vlan tag on ingress.
> >
> > OK so something along the lines of: port is bridged, and bridge has
> > vlan_filtering=0 and switch does egress tagging and VID is bridge's
> > default_pvid then pop the tag?
> >
> > Should this be a helper function that is utilized by the relevant tagger
> > drivers or do you want this in dsa_switch_rcv()?
>
> The two drivers that appear to be untagging the CPU port unconditionally are
> b53 and kzs9477.
So, a helper in DSA would look something like this:
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 75c8fac82017..c0bb978c6ff7 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -204,6 +204,7 @@ struct dsa_port {
const char *mac;
struct device_node *dn;
unsigned int ageing_time;
+ int pvid;
bool vlan_filtering;
u8 stp_state;
struct net_device *bridge_dev;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4a5e2832009b..84d47f838b4e 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -273,6 +273,8 @@ static int dsa_port_setup(struct dsa_port *dp)
if (dp->setup)
return 0;
+ dp->pvid = -1;
+
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
dsa_port_disable(dp);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2da656d984ef..d1dec232fc45 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -7,6 +7,7 @@
#ifndef __DSA_PRIV_H
#define __DSA_PRIV_H
+#include <linux/if_bridge.h>
#include <linux/phy.h>
#include <linux/netdevice.h>
#include <linux/netpoll.h>
@@ -194,6 +195,40 @@ 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 vlan_ethhdr *hdr = vlan_eth_hdr(skb);
+ struct net_device *br = dp->bridge_dev;
+ u16 proto;
+ int err;
+
+ if (!br || br_vlan_enabled(br))
+ return skb;
+
+ err = br_vlan_get_proto(br, &proto);
+ if (err)
+ return skb;
+
+ if (!skb_vlan_tag_present(skb) && hdr->h_vlan_proto == htons(proto)) {
+ skb = skb_vlan_untag(skb);
+ if (!skb)
+ return NULL;
+ }
+
+ if (!skb_vlan_tag_present(skb))
+ return skb;
+
+ /* Cannot use br_vlan_get_pvid here as that requires RTNL */
+ if (skb_vlan_tag_get_id(skb) == dp->pvid)
+ __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);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 86c8dc5c32a0..9167cc678f41 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -315,21 +315,45 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
if (!ds->ops->port_vlan_add)
return 0;
- for (port = 0; port < ds->num_ports; port++)
- if (dsa_switch_vlan_match(ds, port, info))
+ for (port = 0; port < ds->num_ports; port++) {
+ if (dsa_switch_vlan_match(ds, port, info)) {
ds->ops->port_vlan_add(ds, port, info->vlan);
+ if (info->vlan->flags & BRIDGE_VLAN_INFO_PVID) {
+ struct dsa_port *dp = dsa_to_port(ds, port);
+
+ dp->pvid = info->vlan->vid_end;
+ }
+ }
+ }
+
return 0;
}
static int dsa_switch_vlan_del(struct dsa_switch *ds,
struct dsa_notifier_vlan_info *info)
{
+ int err;
+
if (!ds->ops->port_vlan_del)
return -EOPNOTSUPP;
- if (ds->index == info->sw_index)
- return ds->ops->port_vlan_del(ds, info->port, info->vlan);
+ if (ds->index == info->sw_index) {
+ const struct switchdev_obj_port_vlan *vlan = info->vlan;
+ struct dsa_port *dp = dsa_to_port(ds, info->port);
+ int vid;
+
+ err = ds->ops->port_vlan_del(ds, info->port, info->vlan);
+ if (err)
+ return err;
+
+ for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
+ if (vid == dp->pvid) {
+ dp->pvid = -1;
+ break;
+ }
+ }
+ }
/* Do not deprogram the DSA links as they may be used as conduit
* for other VLAN members in the fabric.
It's quite a bit more complex, I don't like it.
Thanks,
-Vladimir
