From f7cdb3ecc9b7f609082fc89e5b79d66858504899 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:28 +0300 Subject: [PATCH 1/9] net: bridge: update BROPT_VLAN_ENABLED before notifying switchdev in br_vlan_filter_toggle SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING is notified by the bridge from two places: - nbp_vlan_init(), during bridge port creation - br_vlan_filter_toggle(), during a netlink/sysfs/ioctl change requested by user space If a switchdev driver uses br_vlan_enabled(br_dev) inside its handler for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING attribute notifier, different things will be seen depending on whether the bridge calls from the first path or the second: - in nbp_vlan_init(), br_vlan_enabled() reflects the current state of the bridge - in br_vlan_filter_toggle(), br_vlan_enabled() reflects the past state of the bridge This can lead in some cases to complications in driver implementation, which can be avoided if these could reliably use br_vlan_enabled(). Nothing seems to depend on this behavior, and it seems overall more straightforward for br_vlan_enabled() to return the proper value even during the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING notifier, so temporarily enable the bridge option, then revert it if the switchdev notifier failed. Cc: Roopa Prabhu Cc: Nikolay Aleksandrov Cc: Ido Schimmel Cc: Jiri Pirko Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/bridge/br_vlan.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 325600361487..805206f31795 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -840,11 +840,14 @@ int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val, if (br_opt_get(br, BROPT_VLAN_ENABLED) == !!val) return 0; - err = switchdev_port_attr_set(br->dev, &attr, extack); - if (err && err != -EOPNOTSUPP) - return err; - br_opt_toggle(br, BROPT_VLAN_ENABLED, !!val); + + err = switchdev_port_attr_set(br->dev, &attr, extack); + if (err && err != -EOPNOTSUPP) { + br_opt_toggle(br, BROPT_VLAN_ENABLED, !val); + return err; + } + br_manage_promisc(br); recalculate_group_addr(br); br_recalculate_fwd_mask(br); From ee80dd2e89ecce9c5dd6f556b8f581c9e1cbb605 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:29 +0300 Subject: [PATCH 2/9] net: bridge: add a helper for retrieving port VLANs from the data path Introduce a brother of br_vlan_get_info() which is protected by the RCU mechanism, as opposed to br_vlan_get_info() which relies on taking the write-side rtnl_mutex. This is needed for drivers which need to find out whether a bridge port has a VLAN configured or not. For example, certain DSA switches might not offer complete source port identification to the CPU on RX, just the VLAN in which the packet was received. Based on this VLAN, we cannot set an accurate skb->dev ingress port, but at least we can configure one that behaves the same as the correct one would (this is possible because DSA sets skb->offload_fwd_mark = 1). When we look at the bridge RX handler (br_handle_frame), we see that what matters regarding skb->dev is the VLAN ID and the port STP state. So we need to select an skb->dev that has the same bridge VLAN as the packet we're receiving, and is in the LEARNING or FORWARDING STP state. The latter is easy, but for the former, we should somehow keep a shadow list of the bridge VLANs on each port, and a lookup table between VLAN ID and the 'designated port for imprecise RX'. That is rather complicated to keep in sync properly (the designated port per VLAN needs to be updated on the addition and removal of a VLAN, as well as on the join/leave events of the bridge on that port). So, to avoid all that complexity, let's just iterate through our finite number of ports and ask the bridge, for each packet: "do you have this VLAN configured on this port?". Cc: Roopa Prabhu Cc: Nikolay Aleksandrov Cc: Ido Schimmel Cc: Jiri Pirko Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- include/linux/if_bridge.h | 8 ++++++++ net/bridge/br_vlan.c | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h index f0b4ffbd8582..b73b4ff749e1 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -111,6 +111,8 @@ int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid); int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto); int br_vlan_get_info(const struct net_device *dev, u16 vid, struct bridge_vlan_info *p_vinfo); +int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, + struct bridge_vlan_info *p_vinfo); #else static inline bool br_vlan_enabled(const struct net_device *dev) { @@ -137,6 +139,12 @@ static inline int br_vlan_get_info(const struct net_device *dev, u16 vid, { return -EINVAL; } + +static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, + struct bridge_vlan_info *p_vinfo) +{ + return -EINVAL; +} #endif #if IS_ENABLED(CONFIG_BRIDGE) diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 805206f31795..8cfd035bbaf9 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -1449,6 +1449,33 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid, } EXPORT_SYMBOL_GPL(br_vlan_get_info); +int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, + struct bridge_vlan_info *p_vinfo) +{ + struct net_bridge_vlan_group *vg; + struct net_bridge_vlan *v; + struct net_bridge_port *p; + + p = br_port_get_check_rcu(dev); + if (p) + vg = nbp_vlan_group_rcu(p); + else if (netif_is_bridge_master(dev)) + vg = br_vlan_group_rcu(netdev_priv(dev)); + else + return -EINVAL; + + v = br_vlan_find(vg, vid); + if (!v) + return -ENOENT; + + p_vinfo->vid = vid; + p_vinfo->flags = v->flags; + if (vid == br_get_pvid(vg)) + p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID; + return 0; +} +EXPORT_SYMBOL_GPL(br_vlan_get_info_rcu); + static int br_vlan_is_bind_vlan_dev(const struct net_device *dev) { return is_vlan_dev(dev) && From d63f8877c48c30f8c3cd263e30658e947fc90f64 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 26 Jul 2021 19:55:30 +0300 Subject: [PATCH 3/9] net: dsa: sja1105: remove redundant re-assignment of pointer table The pointer table is being re-assigned with a value that is never read. The assignment is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index 07bb65a36083..4f1331ff5053 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -2163,8 +2163,6 @@ static int sja1105_build_vlan_table(struct sja1105_private *priv) if (!new_vlan) return -ENOMEM; - table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP]; - for (i = 0; i < VLAN_N_VID; i++) new_vlan[i].vlanid = VLAN_N_VID; From 6dfd23d35e75098ac61a605f6c591ce42e95cdcb Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:31 +0300 Subject: [PATCH 4/9] net: dsa: sja1105: delete vlan delta save/restore logic With the best_effort_vlan_filtering mode now gone, the driver does not have 3 operating modes anymore (VLAN-unaware, VLAN-aware and best effort), but only 2. The idea is that we will gain support for network stack I/O through a VLAN-aware bridge, using the data plane offload framework (imprecise RX, imprecise TX). So the VLAN-aware use case will be more functional. But standalone ports that are part of the same switch when some other ports are under a VLAN-aware bridge should work too. Termination on those should work through the tag_8021q RX VLAN and TX VLAN. This was not possible using the old logic, because: - in VLAN-unaware mode, only the tag_8021q VLANs were committed to hw - in VLAN-aware mode, only the bridge VLANs were committed to hw - in best-effort VLAN mode, both the tag_8021q and bridge VLANs were committed to hw The strategy for the new VLAN-aware mode is to allow the bridge and the tag_8021q VLANs to coexist in the VLAN table at the same time. [ yes, we need to make sure that the bridge cannot install a tag_8021q VLAN, but ] This means that the save/restore logic introduced by commit ec5ae61076d0 ("net: dsa: sja1105: save/restore VLANs using a delta commit method") does not serve a purpose any longer. We can delete it and restore the old code that simply adds a VLAN to the VLAN table and calls it a day. Note that we keep the sja1105_commit_pvid() function from those days, but adapt it slightly. Ports that are under a VLAN-aware bridge use the bridge's pvid, ports that are standalone or under a VLAN-unaware bridge use the tag_8021q pvid, for local termination or VLAN-unaware forwarding. Now, when the vlan_filtering property is toggled for the bridge, the pvid of the ports beneath it is the only thing that's changing, we no longer delete some VLANs and restore others. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105.h | 12 +- drivers/net/dsa/sja1105/sja1105_main.c | 396 +++++++------------------ 2 files changed, 111 insertions(+), 297 deletions(-) diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h index 068be8afd322..9cd7dbdd7db9 100644 --- a/drivers/net/dsa/sja1105/sja1105.h +++ b/drivers/net/dsa/sja1105/sja1105.h @@ -226,14 +226,6 @@ struct sja1105_flow_block { int num_virtual_links; }; -struct sja1105_bridge_vlan { - struct list_head list; - int port; - u16 vid; - bool pvid; - bool untagged; -}; - struct sja1105_private { struct sja1105_static_config static_config; bool rgmii_rx_delay[SJA1105_MAX_NUM_PORTS]; @@ -249,8 +241,8 @@ struct sja1105_private { struct gpio_desc *reset_gpio; struct spi_device *spidev; struct dsa_switch *ds; - struct list_head dsa_8021q_vlans; - struct list_head bridge_vlans; + u16 bridge_pvid[SJA1105_MAX_NUM_PORTS]; + u16 tag_8021q_pvid[SJA1105_MAX_NUM_PORTS]; struct sja1105_flow_block flow_block; struct sja1105_port ports[SJA1105_MAX_NUM_PORTS]; /* Serializes transmission of management frames so that diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index 4f1331ff5053..309e6a933df7 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -378,8 +378,6 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv) table->entry_count = 1; for (port = 0; port < ds->num_ports; port++) { - struct sja1105_bridge_vlan *v; - if (dsa_is_unused_port(ds, port)) continue; @@ -387,22 +385,10 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv) pvid.vlan_bc |= BIT(port); pvid.tag_port &= ~BIT(port); - v = kzalloc(sizeof(*v), GFP_KERNEL); - if (!v) - return -ENOMEM; - - v->port = port; - v->vid = SJA1105_DEFAULT_VLAN; - v->untagged = true; - if (dsa_is_cpu_port(ds, port)) - v->pvid = true; - list_add(&v->list, &priv->dsa_8021q_vlans); - - v = kmemdup(v, sizeof(*v), GFP_KERNEL); - if (!v) - return -ENOMEM; - - list_add(&v->list, &priv->bridge_vlans); + if (dsa_is_cpu_port(ds, port)) { + priv->tag_8021q_pvid[port] = SJA1105_DEFAULT_VLAN; + priv->bridge_pvid[port] = SJA1105_DEFAULT_VLAN; + } } ((struct sja1105_vlan_lookup_entry *)table->entries)[0] = pvid; @@ -1990,12 +1976,29 @@ static int sja1105_pvid_apply(struct sja1105_private *priv, int port, u16 pvid) mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries; + if (mac[port].vlanid == pvid) + return 0; + mac[port].vlanid = pvid; return sja1105_dynamic_config_write(priv, BLK_IDX_MAC_CONFIG, port, &mac[port], true); } +static int sja1105_commit_pvid(struct dsa_switch *ds, int port) +{ + struct dsa_port *dp = dsa_to_port(ds, port); + struct sja1105_private *priv = ds->priv; + u16 pvid; + + if (dp->bridge_dev && br_vlan_enabled(dp->bridge_dev)) + pvid = priv->bridge_pvid[port]; + else + pvid = priv->tag_8021q_pvid[port]; + + return sja1105_pvid_apply(priv, port, pvid); +} + static enum dsa_tag_protocol sja1105_get_tag_protocol(struct dsa_switch *ds, int port, enum dsa_tag_protocol mp) @@ -2021,179 +2024,6 @@ static int sja1105_is_vlan_configured(struct sja1105_private *priv, u16 vid) return -1; } -static int sja1105_commit_vlans(struct sja1105_private *priv, - struct sja1105_vlan_lookup_entry *new_vlan) -{ - struct sja1105_vlan_lookup_entry *vlan; - struct sja1105_table *table; - int num_vlans = 0; - int rc, i, k = 0; - - /* VLAN table */ - table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP]; - vlan = table->entries; - - for (i = 0; i < VLAN_N_VID; i++) { - int match = sja1105_is_vlan_configured(priv, i); - - if (new_vlan[i].vlanid != VLAN_N_VID) - num_vlans++; - - if (new_vlan[i].vlanid == VLAN_N_VID && match >= 0) { - /* Was there before, no longer is. Delete */ - dev_dbg(priv->ds->dev, "Deleting VLAN %d\n", i); - rc = sja1105_dynamic_config_write(priv, - BLK_IDX_VLAN_LOOKUP, - i, &vlan[match], false); - if (rc < 0) - return rc; - } else if (new_vlan[i].vlanid != VLAN_N_VID) { - /* Nothing changed, don't do anything */ - if (match >= 0 && - vlan[match].vlanid == new_vlan[i].vlanid && - vlan[match].tag_port == new_vlan[i].tag_port && - vlan[match].vlan_bc == new_vlan[i].vlan_bc && - vlan[match].vmemb_port == new_vlan[i].vmemb_port) - continue; - /* Update entry */ - dev_dbg(priv->ds->dev, "Updating VLAN %d\n", i); - rc = sja1105_dynamic_config_write(priv, - BLK_IDX_VLAN_LOOKUP, - i, &new_vlan[i], - true); - if (rc < 0) - return rc; - } - } - - if (table->entry_count) - kfree(table->entries); - - table->entries = kcalloc(num_vlans, table->ops->unpacked_entry_size, - GFP_KERNEL); - if (!table->entries) - return -ENOMEM; - - table->entry_count = num_vlans; - vlan = table->entries; - - for (i = 0; i < VLAN_N_VID; i++) { - if (new_vlan[i].vlanid == VLAN_N_VID) - continue; - vlan[k++] = new_vlan[i]; - } - - return 0; -} - -static int sja1105_commit_pvid(struct sja1105_private *priv) -{ - struct sja1105_bridge_vlan *v; - struct list_head *vlan_list; - int rc = 0; - - if (priv->vlan_aware) - vlan_list = &priv->bridge_vlans; - else - vlan_list = &priv->dsa_8021q_vlans; - - list_for_each_entry(v, vlan_list, list) { - if (v->pvid) { - rc = sja1105_pvid_apply(priv, v->port, v->vid); - if (rc) - break; - } - } - - return rc; -} - -static int -sja1105_build_bridge_vlans(struct sja1105_private *priv, - struct sja1105_vlan_lookup_entry *new_vlan) -{ - struct sja1105_bridge_vlan *v; - - if (!priv->vlan_aware) - return 0; - - list_for_each_entry(v, &priv->bridge_vlans, list) { - int match = v->vid; - - new_vlan[match].vlanid = v->vid; - new_vlan[match].vmemb_port |= BIT(v->port); - new_vlan[match].vlan_bc |= BIT(v->port); - if (!v->untagged) - new_vlan[match].tag_port |= BIT(v->port); - new_vlan[match].type_entry = SJA1110_VLAN_D_TAG; - } - - return 0; -} - -static int -sja1105_build_dsa_8021q_vlans(struct sja1105_private *priv, - struct sja1105_vlan_lookup_entry *new_vlan) -{ - struct sja1105_bridge_vlan *v; - - list_for_each_entry(v, &priv->dsa_8021q_vlans, list) { - int match = v->vid; - - new_vlan[match].vlanid = v->vid; - new_vlan[match].vmemb_port |= BIT(v->port); - new_vlan[match].vlan_bc |= BIT(v->port); - if (!v->untagged) - new_vlan[match].tag_port |= BIT(v->port); - new_vlan[match].type_entry = SJA1110_VLAN_D_TAG; - } - - return 0; -} - -static int sja1105_build_vlan_table(struct sja1105_private *priv) -{ - struct sja1105_vlan_lookup_entry *new_vlan; - struct sja1105_table *table; - int rc, i; - - table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP]; - new_vlan = kcalloc(VLAN_N_VID, - table->ops->unpacked_entry_size, GFP_KERNEL); - if (!new_vlan) - return -ENOMEM; - - for (i = 0; i < VLAN_N_VID; i++) - new_vlan[i].vlanid = VLAN_N_VID; - - /* Bridge VLANs */ - rc = sja1105_build_bridge_vlans(priv, new_vlan); - if (rc) - goto out; - - /* VLANs necessary for dsa_8021q operation, given to us by tag_8021q.c: - * - RX VLANs - * - TX VLANs - * - Crosschip links - */ - rc = sja1105_build_dsa_8021q_vlans(priv, new_vlan); - if (rc) - goto out; - - rc = sja1105_commit_vlans(priv, new_vlan); - if (rc) - goto out; - - rc = sja1105_commit_pvid(priv); - if (rc) - goto out; - -out: - kfree(new_vlan); - - return rc; -} - /* The TPID setting belongs to the General Parameters table, * which can only be partially reconfigured at runtime (and not the TPID). * So a switch reset is required. @@ -2275,9 +2105,14 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled, l2_lookup_params = table->entries; l2_lookup_params->shared_learn = !priv->vlan_aware; - rc = sja1105_build_vlan_table(priv); - if (rc) - return rc; + for (port = 0; port < ds->num_ports; port++) { + if (dsa_is_unused_port(ds, port)) + continue; + + rc = sja1105_commit_pvid(ds, port); + if (rc) + return rc; + } rc = sja1105_static_config_reload(priv, SJA1105_VLAN_FILTERING); if (rc) @@ -2286,71 +2121,86 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled, return rc; } -/* Returns number of VLANs added (0 or 1) on success, - * or a negative error code. - */ -static int sja1105_vlan_add_one(struct dsa_switch *ds, int port, u16 vid, - u16 flags, struct list_head *vlan_list) +static int sja1105_vlan_add(struct sja1105_private *priv, int port, u16 vid, + u16 flags) { - bool untagged = flags & BRIDGE_VLAN_INFO_UNTAGGED; - bool pvid = flags & BRIDGE_VLAN_INFO_PVID; - struct sja1105_bridge_vlan *v; + struct sja1105_vlan_lookup_entry *vlan; + struct sja1105_table *table; + int match, rc; - list_for_each_entry(v, vlan_list, list) { - if (v->port == port && v->vid == vid) { - /* Already added */ - if (v->untagged == untagged && v->pvid == pvid) - /* Nothing changed */ - return 0; + table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP]; - /* It's the same VLAN, but some of the flags changed - * and the user did not bother to delete it first. - * Update it and trigger sja1105_build_vlan_table. - */ - v->untagged = untagged; - v->pvid = pvid; - return 1; - } + match = sja1105_is_vlan_configured(priv, vid); + if (match < 0) { + rc = sja1105_table_resize(table, table->entry_count + 1); + if (rc) + return rc; + match = table->entry_count - 1; } - v = kzalloc(sizeof(*v), GFP_KERNEL); - if (!v) { - dev_err(ds->dev, "Out of memory while storing VLAN\n"); - return -ENOMEM; - } + /* Assign pointer after the resize (it's new memory) */ + vlan = table->entries; - v->port = port; - v->vid = vid; - v->untagged = untagged; - v->pvid = pvid; - list_add(&v->list, vlan_list); + vlan[match].type_entry = SJA1110_VLAN_D_TAG; + vlan[match].vlanid = vid; + vlan[match].vlan_bc |= BIT(port); + vlan[match].vmemb_port |= BIT(port); + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) + vlan[match].tag_port &= ~BIT(port); + else + vlan[match].tag_port |= BIT(port); - return 1; + return sja1105_dynamic_config_write(priv, BLK_IDX_VLAN_LOOKUP, vid, + &vlan[match], true); } -/* Returns number of VLANs deleted (0 or 1) */ -static int sja1105_vlan_del_one(struct dsa_switch *ds, int port, u16 vid, - struct list_head *vlan_list) +static int sja1105_vlan_del(struct sja1105_private *priv, int port, u16 vid) { - struct sja1105_bridge_vlan *v, *n; + struct sja1105_vlan_lookup_entry *vlan; + struct sja1105_table *table; + bool keep = true; + int match, rc; - list_for_each_entry_safe(v, n, vlan_list, list) { - if (v->port == port && v->vid == vid) { - list_del(&v->list); - kfree(v); - return 1; - } - } + table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP]; + + match = sja1105_is_vlan_configured(priv, vid); + /* Can't delete a missing entry. */ + if (match < 0) + return 0; + + /* Assign pointer after the resize (it's new memory) */ + vlan = table->entries; + + vlan[match].vlanid = vid; + vlan[match].vlan_bc &= ~BIT(port); + vlan[match].vmemb_port &= ~BIT(port); + /* Also unset tag_port, just so we don't have a confusing bitmap + * (no practical purpose). + */ + vlan[match].tag_port &= ~BIT(port); + + /* If there's no port left as member of this VLAN, + * it's time for it to go. + */ + if (!vlan[match].vmemb_port) + keep = false; + + rc = sja1105_dynamic_config_write(priv, BLK_IDX_VLAN_LOOKUP, vid, + &vlan[match], keep); + if (rc < 0) + return rc; + + if (!keep) + return sja1105_table_delete_entry(table, match); return 0; } -static int sja1105_vlan_add(struct dsa_switch *ds, int port, - const struct switchdev_obj_port_vlan *vlan, - struct netlink_ext_ack *extack) +static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan, + struct netlink_ext_ack *extack) { struct sja1105_private *priv = ds->priv; - bool vlan_table_changed = false; int rc; /* Be sure to deny alterations to the configuration done by tag_8021q. @@ -2361,34 +2211,22 @@ static int sja1105_vlan_add(struct dsa_switch *ds, int port, return -EBUSY; } - rc = sja1105_vlan_add_one(ds, port, vlan->vid, vlan->flags, - &priv->bridge_vlans); - if (rc < 0) + rc = sja1105_vlan_add(priv, port, vlan->vid, vlan->flags); + if (rc) return rc; - if (rc > 0) - vlan_table_changed = true; - if (!vlan_table_changed) - return 0; + if (vlan->flags & BRIDGE_VLAN_INFO_PVID) + priv->bridge_pvid[port] = vlan->vid; - return sja1105_build_vlan_table(priv); + return sja1105_commit_pvid(ds, port); } -static int sja1105_vlan_del(struct dsa_switch *ds, int port, - const struct switchdev_obj_port_vlan *vlan) +static int sja1105_bridge_vlan_del(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan) { struct sja1105_private *priv = ds->priv; - bool vlan_table_changed = false; - int rc; - rc = sja1105_vlan_del_one(ds, port, vlan->vid, &priv->bridge_vlans); - if (rc > 0) - vlan_table_changed = true; - - if (!vlan_table_changed) - return 0; - - return sja1105_build_vlan_table(priv); + return sja1105_vlan_del(priv, port, vlan->vid); } static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid, @@ -2397,23 +2235,21 @@ static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid, struct sja1105_private *priv = ds->priv; int rc; - rc = sja1105_vlan_add_one(ds, port, vid, flags, &priv->dsa_8021q_vlans); - if (rc <= 0) + rc = sja1105_vlan_add(priv, port, vid, flags); + if (rc) return rc; - return sja1105_build_vlan_table(priv); + if (flags & BRIDGE_VLAN_INFO_PVID) + priv->tag_8021q_pvid[port] = vid; + + return sja1105_commit_pvid(ds, port); } static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid) { struct sja1105_private *priv = ds->priv; - int rc; - rc = sja1105_vlan_del_one(ds, port, vid, &priv->dsa_8021q_vlans); - if (!rc) - return 0; - - return sja1105_build_vlan_table(priv); + return sja1105_vlan_del(priv, port, vid); } /* The programming model for the SJA1105 switch is "all-at-once" via static @@ -2531,7 +2367,6 @@ static int sja1105_setup(struct dsa_switch *ds) static void sja1105_teardown(struct dsa_switch *ds) { struct sja1105_private *priv = ds->priv; - struct sja1105_bridge_vlan *v, *n; int port; rtnl_lock(); @@ -2553,16 +2388,6 @@ static void sja1105_teardown(struct dsa_switch *ds) sja1105_tas_teardown(ds); sja1105_ptp_clock_unregister(ds); sja1105_static_config_free(&priv->static_config); - - list_for_each_entry_safe(v, n, &priv->dsa_8021q_vlans, list) { - list_del(&v->list); - kfree(v); - } - - list_for_each_entry_safe(v, n, &priv->bridge_vlans, list) { - list_del(&v->list); - kfree(v); - } } static void sja1105_port_disable(struct dsa_switch *ds, int port) @@ -3002,8 +2827,8 @@ static const struct dsa_switch_ops sja1105_switch_ops = { .port_bridge_flags = sja1105_port_bridge_flags, .port_stp_state_set = sja1105_bridge_stp_state_set, .port_vlan_filtering = sja1105_vlan_filtering, - .port_vlan_add = sja1105_vlan_add, - .port_vlan_del = sja1105_vlan_del, + .port_vlan_add = sja1105_bridge_vlan_add, + .port_vlan_del = sja1105_bridge_vlan_del, .port_mdb_add = sja1105_mdb_add, .port_mdb_del = sja1105_mdb_del, .port_hwtstamp_get = sja1105_hwtstamp_get, @@ -3164,9 +2989,6 @@ static int sja1105_probe(struct spi_device *spi) mutex_init(&priv->ptp_data.lock); mutex_init(&priv->mgmt_lock); - INIT_LIST_HEAD(&priv->bridge_vlans); - INIT_LIST_HEAD(&priv->dsa_8021q_vlans); - sja1105_tas_setup(ds); sja1105_flower_setup(ds); From 4fbc08bd366595dab4bedfec08021bad8609ba77 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:32 +0300 Subject: [PATCH 5/9] net: dsa: sja1105: deny 8021q uppers on ports Now that best-effort VLAN filtering is gone and we are left with the imprecise RX and imprecise TX based in VLAN-aware mode, where the tagger just guesses the source port based on plausibility of the VLAN ID, 8021q uppers installed on top of a standalone port, while other ports of that switch are under a VLAN-aware bridge don't quite "just work". In fact it could be possible to restrict the VLAN IDs used by the 8021q uppers to not be shared with VLAN IDs used by that VLAN-aware bridge, but then the tagger needs to be patched to search for 8021q uppers too, not just for the "designated bridge port" which will be introduced in a later patch. I haven't given a possible implementation full thought, it seems maybe possible but not worth the effort right now. The only certain thing is that currently the tagger won't be able to figure out the source port for these packets because they will come with the VLAN ID of the 8021q upper and are no longer retagged to a tag_8021q sub-VLAN like the best effort VLAN filtering code used to do. So just deny these for the moment. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105_main.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index 309e6a933df7..a380f37fd22d 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -2252,6 +2252,20 @@ static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid) return sja1105_vlan_del(priv, port, vid); } +static int sja1105_prechangeupper(struct dsa_switch *ds, int port, + struct netdev_notifier_changeupper_info *info) +{ + struct netlink_ext_ack *extack = info->info.extack; + struct net_device *upper = info->upper_dev; + + if (is_vlan_dev(upper)) { + NL_SET_ERR_MSG_MOD(extack, "8021q uppers are not supported"); + return -EBUSY; + } + + return 0; +} + /* The programming model for the SJA1105 switch is "all-at-once" via static * configuration tables. Some of these can be dynamically modified at runtime, * but not the xMII mode parameters table. @@ -2846,6 +2860,7 @@ static const struct dsa_switch_ops sja1105_switch_ops = { .devlink_info_get = sja1105_devlink_info_get, .tag_8021q_vlan_add = sja1105_dsa_8021q_vlan_add, .tag_8021q_vlan_del = sja1105_dsa_8021q_vlan_del, + .port_prechangeupper = sja1105_prechangeupper, }; static const struct of_device_id sja1105_dt_ids[]; From 19fa937a391e58f4bb74ea52a5cdb4f259e67db2 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:33 +0300 Subject: [PATCH 6/9] net: dsa: sja1105: deny more than one VLAN-aware bridge With tag_sja1105.c's only ability being to perform an imprecise RX procedure and identify whether a packet comes from a VLAN-aware bridge or not, we have no way to determine whether a packet with VLAN ID 5 comes from, say, br0 or br1. Actually we could, but it would mean that we need to restrict all VLANs from br0 to be different from all VLANs from br1, and this includes the default_pvid, which makes a setup with 2 VLAN-aware bridges highly imprectical. The fact of the matter is that this isn't even that big of a practical limitation, since even with a single VLAN-aware bridge we can pretty much enforce forwarding isolation based on the VLAN port membership. So in the end, tell the user that they need to model their setup using a single VLAN-aware bridge. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105_main.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index a380f37fd22d..ef63226fed2b 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -2257,12 +2257,25 @@ static int sja1105_prechangeupper(struct dsa_switch *ds, int port, { struct netlink_ext_ack *extack = info->info.extack; struct net_device *upper = info->upper_dev; + struct dsa_switch_tree *dst = ds->dst; + struct dsa_port *dp; if (is_vlan_dev(upper)) { NL_SET_ERR_MSG_MOD(extack, "8021q uppers are not supported"); return -EBUSY; } + if (netif_is_bridge_master(upper)) { + list_for_each_entry(dp, &dst->ports, list) { + if (dp->bridge_dev && dp->bridge_dev != upper && + br_vlan_enabled(dp->bridge_dev)) { + NL_SET_ERR_MSG_MOD(extack, + "Only one VLAN-aware bridge is supported"); + return -EBUSY; + } + } + } + return 0; } From 884be12f85666c6e9ff1cf3ead06a7371f6863dc Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:34 +0300 Subject: [PATCH 7/9] net: dsa: sja1105: add support for imprecise RX This is already common knowledge by now, but the sja1105 does not have hardware support for DSA tagging for data plane packets, and tag_8021q sets up a unique pvid per port, transmitted as VLAN-tagged towards the CPU, for the source port to be decoded nonetheless. When the port is part of a VLAN-aware bridge, the pvid committed to hardware is taken from the bridge and not from tag_8021q, so we need to work with that the best we can. Configure the switches to send all packets to the CPU as VLAN-tagged (even ones that were originally untagged on the wire) and make use of dsa_untag_bridge_pvid() to get rid of it before we send those packets up the network stack. With the classified VLAN used by hardware known to the tagger, we first peek at the VID in an attempt to figure out if the packet was received from a VLAN-unaware port (standalone or under a VLAN-unaware bridge), case in which we can continue to call dsa_8021q_rcv(). If that is not the case, the packet probably came from a VLAN-aware bridge. So we call the DSA helper that finds for us a "designated bridge port" - one that is a member of the VLAN ID from the packet, and is in the proper STP state - basically these are all checks performed by br_handle_frame() in the software RX data path. The bridge will accept the packet as valid even if the source port was maybe wrong. So it will maybe learn the MAC SA of the packet on the wrong port, and its software FDB will be out of sync with the hardware FDB. So replies towards this same MAC DA will not work, because the bridge will send towards a different netdev. This is where the bridge data plane offload ("imprecise TX") added by the next patch comes in handy. The software FDB is wrong, true, but the hardware FDB isn't, and by offloading the bridge forwarding plane we have a chance to right a wrong, and have the hardware look up the FDB for us for the reply packet. So it all cancels out. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105_main.c | 8 ++- net/dsa/dsa_priv.h | 43 +++++++++++++ net/dsa/tag_sja1105.c | 87 +++++++++++++------------- 3 files changed, 93 insertions(+), 45 deletions(-) diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index ef63226fed2b..a6a671f0fca5 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -2201,6 +2201,7 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port, struct netlink_ext_ack *extack) { struct sja1105_private *priv = ds->priv; + u16 flags = vlan->flags; int rc; /* Be sure to deny alterations to the configuration done by tag_8021q. @@ -2211,7 +2212,11 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port, return -EBUSY; } - rc = sja1105_vlan_add(priv, port, vlan->vid, vlan->flags); + /* Always install bridge VLANs as egress-tagged on the CPU port. */ + if (dsa_is_cpu_port(ds, port)) + flags = 0; + + rc = sja1105_vlan_add(priv, port, vlan->vid, flags); if (rc) return rc; @@ -2361,6 +2366,7 @@ static int sja1105_setup(struct dsa_switch *ds) * TPID is ETH_P_SJA1105, and the VLAN ID is the port pvid. */ ds->vlan_filtering_is_global = true; + ds->untag_bridge_pvid = true; /* Advertise the 8 egress queues */ ds->num_tx_queues = SJA1105_NUM_TC; diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index b1d9aa4d313c..da3ad02d6ceb 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -397,6 +397,49 @@ static inline struct sk_buff *dsa_untag_bridge_pvid(struct sk_buff *skb) return skb; } +/* For switches without hardware support for DSA tagging to be able + * to support termination through the bridge. + */ +static inline struct net_device * +dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid) +{ + struct dsa_port *cpu_dp = master->dsa_ptr; + struct dsa_switch_tree *dst = cpu_dp->dst; + struct bridge_vlan_info vinfo; + struct net_device *slave; + struct dsa_port *dp; + int err; + + list_for_each_entry(dp, &dst->ports, list) { + if (dp->type != DSA_PORT_TYPE_USER) + continue; + + if (!dp->bridge_dev) + continue; + + if (dp->stp_state != BR_STATE_LEARNING && + dp->stp_state != BR_STATE_FORWARDING) + continue; + + /* Since the bridge might learn this packet, keep the CPU port + * affinity with the port that will be used for the reply on + * xmit. + */ + if (dp->cpu_dp != cpu_dp) + continue; + + slave = dp->slave; + + err = br_vlan_get_info_rcu(slave, vid, &vinfo); + if (err) + continue; + + return slave; + } + + return NULL; +} + /* 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/tag_sja1105.c b/net/dsa/tag_sja1105.c index 7c92c329a092..f142a933c5e2 100644 --- a/net/dsa/tag_sja1105.c +++ b/net/dsa/tag_sja1105.c @@ -115,40 +115,6 @@ static inline bool sja1105_is_meta_frame(const struct sk_buff *skb) return true; } -static bool sja1105_can_use_vlan_as_tags(const struct sk_buff *skb) -{ - struct vlan_ethhdr *hdr = vlan_eth_hdr(skb); - u16 vlan_tci; - - if (hdr->h_vlan_proto == htons(ETH_P_SJA1105)) - return true; - - if (hdr->h_vlan_proto != htons(ETH_P_8021Q) && - !skb_vlan_tag_present(skb)) - return false; - - if (skb_vlan_tag_present(skb)) - vlan_tci = skb_vlan_tag_get(skb); - else - vlan_tci = ntohs(hdr->h_vlan_TCI); - - return vid_is_dsa_8021q(vlan_tci & VLAN_VID_MASK); -} - -/* This is the first time the tagger sees the frame on RX. - * Figure out if we can decode it. - */ -static bool sja1105_filter(const struct sk_buff *skb, struct net_device *dev) -{ - if (sja1105_can_use_vlan_as_tags(skb)) - return true; - if (sja1105_is_link_local(skb)) - return true; - if (sja1105_is_meta_frame(skb)) - return true; - return false; -} - /* Calls sja1105_port_deferred_xmit in sja1105_main.c */ static struct sk_buff *sja1105_defer_xmit(struct sja1105_port *sp, struct sk_buff *skb) @@ -371,15 +337,42 @@ static bool sja1110_skb_has_inband_control_extension(const struct sk_buff *skb) return ntohs(eth_hdr(skb)->h_proto) == ETH_P_SJA1110; } +/* Returns true for imprecise RX and sets the @vid. + * Returns false for precise RX and sets @source_port and @switch_id. + */ +static bool sja1105_vlan_rcv(struct sk_buff *skb, int *source_port, + int *switch_id, u16 *vid) +{ + struct vlan_ethhdr *hdr = (struct vlan_ethhdr *)skb_mac_header(skb); + u16 vlan_tci; + + if (skb_vlan_tag_present(skb)) + vlan_tci = skb_vlan_tag_get(skb); + else + vlan_tci = ntohs(hdr->h_vlan_TCI); + + if (vid_is_dsa_8021q_rxvlan(vlan_tci & VLAN_VID_MASK)) { + dsa_8021q_rcv(skb, source_port, switch_id); + return false; + } + + /* Try our best with imprecise RX */ + *vid = vlan_tci & VLAN_VID_MASK; + + return true; +} + static struct sk_buff *sja1105_rcv(struct sk_buff *skb, struct net_device *netdev, struct packet_type *pt) { + int source_port = -1, switch_id = -1; struct sja1105_meta meta = {0}; - int source_port, switch_id; + bool imprecise_rx = false; struct ethhdr *hdr; bool is_link_local; bool is_meta; + u16 vid; hdr = eth_hdr(skb); is_link_local = sja1105_is_link_local(skb); @@ -389,7 +382,8 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb, if (sja1105_skb_has_tag_8021q(skb)) { /* Normal traffic path. */ - dsa_8021q_rcv(skb, &source_port, &switch_id); + imprecise_rx = sja1105_vlan_rcv(skb, &source_port, &switch_id, + &vid); } else if (is_link_local) { /* Management traffic path. Switch embeds the switch ID and * port ID into bytes of the destination MAC, courtesy of @@ -408,7 +402,10 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb, return NULL; } - skb->dev = dsa_master_find_slave(netdev, switch_id, source_port); + if (imprecise_rx) + skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid); + else + skb->dev = dsa_master_find_slave(netdev, switch_id, source_port); if (!skb->dev) { netdev_warn(netdev, "Couldn't decode source port\n"); return NULL; @@ -522,6 +519,8 @@ static struct sk_buff *sja1110_rcv(struct sk_buff *skb, struct packet_type *pt) { int source_port = -1, switch_id = -1; + bool imprecise_rx = false; + u16 vid; skb->offload_fwd_mark = 1; @@ -534,13 +533,15 @@ static struct sk_buff *sja1110_rcv(struct sk_buff *skb, /* Packets with in-band control extensions might still have RX VLANs */ if (likely(sja1105_skb_has_tag_8021q(skb))) - dsa_8021q_rcv(skb, &source_port, &switch_id); + imprecise_rx = sja1105_vlan_rcv(skb, &source_port, &switch_id, + &vid); - skb->dev = dsa_master_find_slave(netdev, switch_id, source_port); + if (imprecise_rx) + skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid); + else + skb->dev = dsa_master_find_slave(netdev, switch_id, source_port); if (!skb->dev) { - netdev_warn(netdev, - "Couldn't decode source port %d and switch id %d\n", - source_port, switch_id); + netdev_warn(netdev, "Couldn't decode source port\n"); return NULL; } @@ -576,7 +577,6 @@ static const struct dsa_device_ops sja1105_netdev_ops = { .proto = DSA_TAG_PROTO_SJA1105, .xmit = sja1105_xmit, .rcv = sja1105_rcv, - .filter = sja1105_filter, .needed_headroom = VLAN_HLEN, .flow_dissect = sja1105_flow_dissect, .promisc_on_master = true, @@ -590,7 +590,6 @@ static const struct dsa_device_ops sja1110_netdev_ops = { .proto = DSA_TAG_PROTO_SJA1110, .xmit = sja1110_xmit, .rcv = sja1110_rcv, - .filter = sja1105_filter, .flow_dissect = sja1110_flow_dissect, .needed_headroom = SJA1110_HEADER_LEN + VLAN_HLEN, .needed_tailroom = SJA1110_RX_TRAILER_LEN + SJA1110_MAX_PADDING_LEN, From b6ad86e6ad6c46e52cac218e62613c6c47cf7fa0 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:35 +0300 Subject: [PATCH 8/9] net: dsa: sja1105: add bridge TX data plane offload based on tag_8021q The main desire for having this feature in sja1105 is to support network stack termination for traffic coming from a VLAN-aware bridge. For sja1105, offloading the bridge data plane means sending packets as-is, with the proper VLAN tag, to the chip. The chip will look up its FDB and forward them to the correct destination port. But we support bridge data plane offload even for VLAN-unaware bridges, and the implementation there is different. In fact, VLAN-unaware bridging is governed by tag_8021q, so it makes sense to have the .bridge_fwd_offload_add() implementation fully within tag_8021q. The key difference is that we only support 1 VLAN-aware bridge, but we support multiple VLAN-unaware bridges. So we need to make sure that the forwarding domain is not crossed by packets injected from the stack. For this, we introduce the concept of a tag_8021q TX VLAN for bridge forwarding offload. As opposed to the regular TX VLANs which contain only 2 ports (the user port and the CPU port), a bridge data plane TX VLAN is "multicast" (or "imprecise"): it contains all the ports that are part of a certain bridge, and the hardware will select where the packet goes within this "imprecise" forwarding domain. Each VLAN-unaware bridge has its own "imprecise" TX VLAN, so we make use of the unique "bridge_num" provided by DSA for the data plane offload. We use the same 3 bits from the tag_8021q VLAN ID format to encode this bridge number. Note that these 3 bit positions have been used before for sub-VLANs in best-effort VLAN filtering mode. The difference is that for best-effort, the sub-VLANs were only valid on RX (and it was documented that the sub-VLAN field needed to be transmitted as zero). Whereas for the bridge data plane offload, these 3 bits are only valid on TX. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- drivers/net/dsa/sja1105/sja1105_main.c | 4 +++ include/linux/dsa/8021q.h | 10 ++++++ net/dsa/tag_8021q.c | 48 +++++++++++++++++++++++--- net/dsa/tag_sja1105.c | 31 +++++++++++++++++ 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index a6a671f0fca5..da042e211dda 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -2367,6 +2367,8 @@ static int sja1105_setup(struct dsa_switch *ds) */ ds->vlan_filtering_is_global = true; ds->untag_bridge_pvid = true; + /* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */ + ds->num_fwd_offloading_bridges = 7; /* Advertise the 8 egress queues */ ds->num_tx_queues = SJA1105_NUM_TC; @@ -2880,6 +2882,8 @@ static const struct dsa_switch_ops sja1105_switch_ops = { .tag_8021q_vlan_add = sja1105_dsa_8021q_vlan_add, .tag_8021q_vlan_del = sja1105_dsa_8021q_vlan_del, .port_prechangeupper = sja1105_prechangeupper, + .port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload, + .port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload, }; static const struct of_device_id sja1105_dt_ids[]; diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h index ec5abfcdefd1..c7fa4a3498fe 100644 --- a/include/linux/dsa/8021q.h +++ b/include/linux/dsa/8021q.h @@ -35,6 +35,16 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev, void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id); +int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port, + struct net_device *br, + int bridge_num); + +void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port, + struct net_device *br, + int bridge_num); + +u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num); + u16 dsa_8021q_tx_vid(struct dsa_switch *ds, int port); u16 dsa_8021q_rx_vid(struct dsa_switch *ds, int port); diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c index 51dcde7db26b..654697ebb6f3 100644 --- a/net/dsa/tag_8021q.c +++ b/net/dsa/tag_8021q.c @@ -17,7 +17,7 @@ * * | 11 | 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | * +-----------+-----+-----------------+-----------+-----------------------+ - * | DIR | RSV | SWITCH_ID | RSV | PORT | + * | DIR | VBID| SWITCH_ID | VBID | PORT | * +-----------+-----+-----------------+-----------+-----------------------+ * * DIR - VID[11:10]: @@ -30,9 +30,10 @@ * SWITCH_ID - VID[8:6]: * Index of switch within DSA tree. Must be between 0 and 7. * - * RSV - VID[5:4]: - * To be used for further expansion of PORT or for other purposes. - * Must be transmitted as zero and ignored on receive. + * VBID - { VID[9], VID[5:4] }: + * Virtual bridge ID. If between 1 and 7, packet targets the broadcast + * domain of a bridge. If transmitted as zero, packet targets a single + * port. Field only valid on transmit, must be ignored on receive. * * PORT - VID[3:0]: * Index of switch port. Must be between 0 and 15. @@ -50,11 +51,30 @@ #define DSA_8021Q_SWITCH_ID(x) (((x) << DSA_8021Q_SWITCH_ID_SHIFT) & \ DSA_8021Q_SWITCH_ID_MASK) +#define DSA_8021Q_VBID_HI_SHIFT 9 +#define DSA_8021Q_VBID_HI_MASK GENMASK(9, 9) +#define DSA_8021Q_VBID_LO_SHIFT 4 +#define DSA_8021Q_VBID_LO_MASK GENMASK(5, 4) +#define DSA_8021Q_VBID_HI(x) (((x) & GENMASK(2, 2)) >> 2) +#define DSA_8021Q_VBID_LO(x) ((x) & GENMASK(1, 0)) +#define DSA_8021Q_VBID(x) \ + (((DSA_8021Q_VBID_LO(x) << DSA_8021Q_VBID_LO_SHIFT) & \ + DSA_8021Q_VBID_LO_MASK) | \ + ((DSA_8021Q_VBID_HI(x) << DSA_8021Q_VBID_HI_SHIFT) & \ + DSA_8021Q_VBID_HI_MASK)) + #define DSA_8021Q_PORT_SHIFT 0 #define DSA_8021Q_PORT_MASK GENMASK(3, 0) #define DSA_8021Q_PORT(x) (((x) << DSA_8021Q_PORT_SHIFT) & \ DSA_8021Q_PORT_MASK) +u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num) +{ + /* The VBID value of 0 is reserved for precise TX */ + return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1); +} +EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid); + /* Returns the VID to be inserted into the frame from xmit for switch steering * instructions on egress. Encodes switch ID and port ID. */ @@ -387,6 +407,26 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, return 0; } +int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port, + struct net_device *br, + int bridge_num) +{ + u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num); + + return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid); +} +EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload); + +void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port, + struct net_device *br, + int bridge_num) +{ + u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num); + + dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid); +} +EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_unoffload); + /* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port) { diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c index f142a933c5e2..cddee4b499d8 100644 --- a/net/dsa/tag_sja1105.c +++ b/net/dsa/tag_sja1105.c @@ -133,6 +133,31 @@ static u16 sja1105_xmit_tpid(struct sja1105_port *sp) return sp->xmit_tpid; } +static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb, + struct net_device *netdev) +{ + struct dsa_port *dp = dsa_slave_to_port(netdev); + struct net_device *br = dp->bridge_dev; + u16 tx_vid; + + /* If the port is under a VLAN-aware bridge, just slide the + * VLAN-tagged packet into the FDB and hope for the best. + * This works because we support a single VLAN-aware bridge + * across the entire dst, and its VLANs cannot be shared with + * any standalone port. + */ + if (br_vlan_enabled(br)) + return skb; + + /* If the port is under a VLAN-unaware bridge, use an imprecise + * TX VLAN that targets the bridge's entire broadcast domain, + * instead of just the specific port. + */ + tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(dp->bridge_num); + + return dsa_8021q_xmit(skb, netdev, sja1105_xmit_tpid(dp->priv), tx_vid); +} + static struct sk_buff *sja1105_xmit(struct sk_buff *skb, struct net_device *netdev) { @@ -141,6 +166,9 @@ static struct sk_buff *sja1105_xmit(struct sk_buff *skb, u16 queue_mapping = skb_get_queue_mapping(skb); u8 pcp = netdev_txq_to_tc(netdev, queue_mapping); + if (skb->offload_fwd_mark) + return sja1105_imprecise_xmit(skb, netdev); + /* Transmitting management traffic does not rely upon switch tagging, * but instead SPI-installed management routes. Part 2 of this * is the .port_deferred_xmit driver callback. @@ -165,6 +193,9 @@ static struct sk_buff *sja1110_xmit(struct sk_buff *skb, __be16 *tx_header; int trailer_pos; + if (skb->offload_fwd_mark) + return sja1105_imprecise_xmit(skb, netdev); + /* Transmitting control packets is done using in-band control * extensions, while data packets are transmitted using * tag_8021q TX VLANs. From edac6f6332d96aab59af5f27a195f55cd080f034 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 26 Jul 2021 19:55:36 +0300 Subject: [PATCH 9/9] Revert "net: dsa: Allow drivers to filter packets they can decode source port from" This reverts commit cc1939e4b3aaf534fb2f3706820012036825731c. Currently 2 classes of DSA drivers are able to send/receive packets directly through the DSA master: - drivers with DSA_TAG_PROTO_NONE - sja1105 Now that sja1105 has gained the ability to perform traffic termination even under the tricky case (VLAN-aware bridge), and that is much more functional (we can perform VLAN-aware bridging with foreign interfaces), there is no reason to keep this code in the receive path of the network core. So delete it. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- include/net/dsa.h | 15 --------------- net/dsa/port.c | 1 - net/ethernet/eth.c | 6 +----- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index f8eb2dc3fbef..55fcac854058 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -88,11 +88,6 @@ struct dsa_device_ops { struct packet_type *pt); void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto, int *offset); - /* Used to determine which traffic should match the DSA filter in - * eth_type_trans, and which, if any, should bypass it and be processed - * as regular on the master net device. - */ - bool (*filter)(const struct sk_buff *skb, struct net_device *dev); unsigned int needed_headroom; unsigned int needed_tailroom; const char *name; @@ -246,7 +241,6 @@ struct dsa_port { struct dsa_switch_tree *dst; struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt); - bool (*filter)(const struct sk_buff *skb, struct net_device *dev); enum { DSA_PORT_TYPE_UNUSED = 0, @@ -985,15 +979,6 @@ static inline bool netdev_uses_dsa(const struct net_device *dev) return false; } -static inline bool dsa_can_decode(const struct sk_buff *skb, - struct net_device *dev) -{ -#if IS_ENABLED(CONFIG_NET_DSA) - return !dev->dsa_ptr->filter || dev->dsa_ptr->filter(skb, dev); -#endif - return false; -} - /* All DSA tags that push the EtherType to the right (basically all except tail * tags, which don't break dissection) can be treated the same from the * perspective of the flow dissector. diff --git a/net/dsa/port.c b/net/dsa/port.c index 7b9bf45a76b6..b927d94b6934 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -888,7 +888,6 @@ int dsa_port_mrp_del_ring_role(const struct dsa_port *dp, void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, const struct dsa_device_ops *tag_ops) { - cpu_dp->filter = tag_ops->filter; cpu_dp->rcv = tag_ops->rcv; cpu_dp->tag_ops = tag_ops; } diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 9cce612e8976..171ba75b74c9 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -182,12 +182,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) * at all, so we check here whether one of those tagging * variants has been configured on the receiving interface, * and if so, set skb->protocol without looking at the packet. - * The DSA tagging protocol may be able to decode some but not all - * traffic (for example only for management). In that case give it the - * option to filter the packets from which it can decode source port - * information. */ - if (unlikely(netdev_uses_dsa(dev)) && dsa_can_decode(skb, dev)) + if (unlikely(netdev_uses_dsa(dev))) return htons(ETH_P_XDSA); if (likely(eth_proto_is_802_3(eth->h_proto)))