[PATCH net-next 0/2] Simplify DSA handling of VLAN subinterface offload

[PATCH net-next 0/2] Simplify DSA handling of VLAN subinterface offload

[Vladimir Oltean]

Depends on Vivien Didelot's patchset:
https://patchwork.ozlabs.org/project/netdev/list/?series=127197&state=*

This patchset removes a few strange-looking guards for -EOPNOTSUPP in
dsa_slave_vlan_rx_add_vid and dsa_slave_vlan_rx_kill_vid, making that
code path no longer possible.

It also disables the code path for the sja1105 driver, which does
support editing the VLAN table, but not hardware-accelerated VLAN
sub-interfaces, therefore the check in the DSA core would be wrong.
There was no better DSA callback to do this than .port_enable, i.e.
at ndo_open time.

Vladimir Oltean (2):
  net: dsa: Advertise the VLAN offload netdev ability only if switch
    supports it
  net: dsa: sja1105: Clear VLAN filtering offload netdev feature

 drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
 net/dsa/slave.c                        | 15 ++++++---------
 2 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH net-next 1/2] net: dsa: Advertise the VLAN offload netdev ability only if switch supports it

[Vladimir Oltean]

When adding a VLAN sub-interface on a DSA slave port, the 8021q core
checks NETIF_F_HW_VLAN_CTAG_FILTER and, if the netdev is capable of
filtering, calls .ndo_vlan_rx_add_vid or .ndo_vlan_rx_kill_vid to
configure the VLAN offloading.

DSA sets this up counter-intuitively: it always advertises this netdev
feature, but the underlying driver may not actually support VLAN table
manipulation. In that case, the DSA core is forced to ignore the error,
because not being able to offload the VLAN is still fine - and should
result in the creation of a non-accelerated VLAN sub-interface.

Change this so that the netdev feature is only advertised for switch
drivers that support VLAN manipulation, instead of checking for
-EOPNOTSUPP at runtime.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 net/dsa/slave.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d84225125099..9a88035517a6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1131,11 +1131,11 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 	}
 
 	ret = dsa_port_vid_add(dp, vid, 0);
-	if (ret && ret != -EOPNOTSUPP)
+	if (ret)
 		return ret;
 
 	ret = dsa_port_vid_add(dp->cpu_dp, vid, 0);
-	if (ret && ret != -EOPNOTSUPP)
+	if (ret)
 		return ret;
 
 	return 0;
@@ -1164,14 +1164,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 			return -EBUSY;
 	}
 
-	ret = dsa_port_vid_del(dp, vid);
-	if (ret == -EOPNOTSUPP)
-		ret = 0;
-
 	/* Do not deprogram the CPU port as it may be shared with other user
 	 * ports which can be members of this VLAN as well.
 	 */
-	return ret;
+	return dsa_port_vid_del(dp, vid);
 }
 
 static const struct ethtool_ops dsa_slave_ethtool_ops = {
@@ -1418,8 +1414,9 @@ int dsa_slave_create(struct dsa_port *port)
 	if (slave_dev == NULL)
 		return -ENOMEM;
 
-	slave_dev->features = master->vlan_features | NETIF_F_HW_TC |
-				NETIF_F_HW_VLAN_CTAG_FILTER;
+	slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
+	if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
+		slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
 	slave_dev->hw_features |= NETIF_F_HW_TC;
 	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
 	if (!IS_ERR_OR_NULL(port->mac))
-- 
2.17.1

[PATCH net-next 2/2] net: dsa: sja1105: Clear VLAN filtering offload netdev feature

[Vladimir Oltean]

The switch barely supports traffic I/O, and it does that by repurposing
VLANs when there is no bridge that is taking control of them.

Letting DSA declare this netdev feature as supported (see
dsa_slave_create) would mean that VLAN sub-interfaces created on sja1105
switch ports will be hardware offloaded. That means that
net/8021q/vlan_core.c would install the VLAN into the filter tables of
the switch, potentially interfering with the tag_8021q VLANs.

We need to prevent that from happening and not let the 8021q core
offload VLANs to the switch hardware tables. In vlan_filtering=0 modes
of operation, the switch ports can pass through VLAN-tagged frames with
no problem.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index df976b259e43..d8cff0107ec4 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1728,6 +1728,21 @@ static void sja1105_teardown(struct dsa_switch *ds)
 	sja1105_static_config_free(&priv->static_config);
 }
 
+static int sja1105_port_enable(struct dsa_switch *ds, int port,
+			       struct phy_device *phy)
+{
+	struct net_device *slave;
+
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
+	slave = ds->ports[port].slave;
+
+	slave->features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;
+
+	return 0;
+}
+
 static int sja1105_mgmt_xmit(struct dsa_switch *ds, int port, int slot,
 			     struct sk_buff *skb, bool takets)
 {
@@ -2049,6 +2064,7 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.get_ethtool_stats	= sja1105_get_ethtool_stats,
 	.get_sset_count		= sja1105_get_sset_count,
 	.get_ts_info		= sja1105_get_ts_info,
+	.port_enable		= sja1105_port_enable,
 	.port_fdb_dump		= sja1105_fdb_dump,
 	.port_fdb_add		= sja1105_fdb_add,
 	.port_fdb_del		= sja1105_fdb_del,
-- 
2.17.1

Re: [PATCH net-next 0/2] Simplify DSA handling of VLAN subinterface offload

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Sun, 25 Aug 2019 22:46:28 +0300

> Depends on Vivien Didelot's patchset:
> https://patchwork.ozlabs.org/project/netdev/list/?series=127197&state=*
> 
> This patchset removes a few strange-looking guards for -EOPNOTSUPP in
> dsa_slave_vlan_rx_add_vid and dsa_slave_vlan_rx_kill_vid, making that
> code path no longer possible.
> 
> It also disables the code path for the sja1105 driver, which does
> support editing the VLAN table, but not hardware-accelerated VLAN
> sub-interfaces, therefore the check in the DSA core would be wrong.
> There was no better DSA callback to do this than .port_enable, i.e.
> at ndo_open time.

Series applied.