diff mbox series

[net-next,v1,9/9] net: dsa: qca: ar9331: add vlan support

Message ID 20210403114848.30528-10-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series ar9331: mainline some parts of switch functionality | expand

Commit Message

Oleksij Rempel April 3, 2021, 11:48 a.m. UTC 
This switch provides simple VLAN resolution database for 16 entries (VLANs).
With this database we can cover typical functionalities as port based
VLANs, untagged and tagged egress. Port based ingress filtering.

The VLAN database is working on top of forwarding database. So,
potentially, we can have multiple VLANs on top of multiple bridges.
Hawing one VLAN on top of multiple bridges will fail on different
levels, most probably DSA framework should warn if some one wont to make
something likes this.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/qca/ar9331.c | 255 +++++++++++++++++++++++++++++++++++
 1 file changed, 255 insertions(+)

Comments

Vladimir Oltean April 4, 2021, 12:36 a.m. UTC | #1 
On Sat, Apr 03, 2021 at 01:48:48PM +0200, Oleksij Rempel wrote:
> This switch provides simple VLAN resolution database for 16 entries (VLANs).
> With this database we can cover typical functionalities as port based
> VLANs, untagged and tagged egress. Port based ingress filtering.
> 
> The VLAN database is working on top of forwarding database. So,

Define 'on top'.

> potentially, we can have multiple VLANs on top of multiple bridges.
> Hawing one VLAN on top of multiple bridges will fail on different

s/Hawing/Having/

> levels, most probably DSA framework should warn if some one wont to make

s/wont/wants/
s/some one/someone/

> something likes this.

Finally, why should the DSA framework warn?
Even in the default configuration of two bridges, the default_pvid (1)
will be the same. What problems do you have with that?

In commit 0ee2af4ebbe3 ("net: dsa: set configure_vlan_while_not_filtering
to true by default"), I did not notice that ar9331 does not have VLAN
operations, and I mistakenly set ds->configure_vlan_while_not_filtering
= false for your driver. Could you please delete that line and ensure the
following works?

ip link add br0 type bridge
ip link set lan0 master br0
bridge vlan add dev lan0 vid 100
ip link set br0 type bridge vlan_filtering 1
# make sure you can receive traffic with VLAN 100

> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/qca/ar9331.c | 255 +++++++++++++++++++++++++++++++++++
>  1 file changed, 255 insertions(+)
> 
> +static int ar9331_sw_vt_wait(struct ar9331_sw_priv *priv, u32 *f0)
> +{
> +	struct regmap *regmap = priv->regmap;
> +
> +	return regmap_read_poll_timeout(regmap,
> +				        AR9331_SW_REG_VLAN_TABLE_FUNCTION0,
> +				        *f0, !(*f0 & AR9331_SW_VT0_BUSY),
> +				        100, 2000);
> +}
> +
> +static int ar9331_sw_port_vt_rmw(struct ar9331_sw_priv *priv, u16 vid,
> +				 u8 port_mask_set, u8 port_mask_clr)
> +{
> +	struct regmap *regmap = priv->regmap;
> +	u32 f0, f1, port_mask = 0, port_mask_new, func;
> +	struct ar9331_sw_vlan_db *vdb = NULL;
> +	int ret, i;
> +
> +	if (!vid)
> +		return 0;
> +
> +	ret = ar9331_sw_vt_wait(priv, &f0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, 0);
> +	if (ret)
> +		goto error;
> +
> +	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, 0);
> +	if (ret)
> +		goto error;
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->vdb); i++) {
> +		if (priv->vdb[i].vid == vid) {
> +			vdb = &priv->vdb[i];
> +			break;
> +		}
> +	}
> +
> +	ret = regmap_read(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, &f1);
> +	if (ret)
> +		return ret;
> +
> +	if (vdb) {
> +		port_mask = vdb->port_mask;
> +	}
> +
> +	port_mask_new = port_mask & ~port_mask_clr;
> +	port_mask_new |= port_mask_set;
> +
> +	if (port_mask_new && port_mask_new == port_mask) {
> +		dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
> +				    __func__, port_mask_new);

With VLANs, the bridge is indeed much less strict compared to FDBs, due
to the old API having ranges baked in (which were never used).

That being said, is there actually any value in this message? Would you
mind deleting it (I see how it could annoy a user)?

You might want to look at devlink regions if you want to debug the VLAN
table of the hardware.

> +		return 0;
> +	}
> +
> +	if (port_mask_new) {
> +		func = AR9331_SW_VT0_FUNC_LOAD_ENTRY;
> +	} else {
> +		func = AR9331_SW_VT0_FUNC_PURGE_ENTRY;
> +		port_mask_new = port_mask;
> +	}
> +
> +	if (vdb) {
> +		vdb->port_mask = port_mask_new;
> +
> +		if (!port_mask_new)
> +			vdb->vid = 0;
> +	} else {
> +		for (i = 0; i < ARRAY_SIZE(priv->vdb); i++) {
> +			if (!priv->vdb[i].vid) {
> +				vdb = &priv->vdb[i];
> +				break;
> +			}
> +		}
> +
> +		if (!vdb) {
> +			dev_err_ratelimited(priv->dev, "Local VDB is full\n");

You have a netlink extack at your disposal, use it.

> +			return -ENOMEM;
> +		}
> +		vdb->vid = vid;
> +		vdb->port_mask = port_mask_new;
> +	}
> +
> +	f0 = FIELD_PREP(AR9331_SW_VT0_VID, vid) |
> +	     FIELD_PREP(AR9331_SW_VT0_FUNC, func) |
> +	     AR9331_SW_VT0_BUSY;
> +	f1 = FIELD_PREP(AR9331_SW_VT1_VID_MEM, port_mask_new) |
> +		AR9331_SW_VT1_VALID;
> +
> +	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, f1);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, f0);
> +	if (ret)
> +		return ret;
> +
> +	ret = ar9331_sw_vt_wait(priv, &f0);
> +	if (ret)
> +		return ret;
> +
> +	if (f0 & AR9331_SW_VT0_FULL_VIO) {
> +		/* cleanup error status */
> +		regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, 0);
> +		dev_err_ratelimited(priv->dev, "%s: can't add new entry, VT is full\n", __func__);

Similar comment as above.

> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +
> +error:
> +	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
> +			    ERR_PTR(ret));
> +
> +	return ret;
> +}
> +
> +static int ar9331_port_vlan_set_pvid(struct ar9331_sw_priv *priv, int port,
> +				     u16 pvid)
> +{
> +	struct regmap *regmap = priv->regmap;
> +	int ret;
> +	u32 mask, val;
> +
> +	mask = AR9331_SW_PORT_VLAN_8021Q_MODE |
> +		AR9331_SW_PORT_VLAN_FORCE_DEFALUT_VID_EN |
> +		AR9331_SW_PORT_VLAN_FORCE_PORT_VLAN_EN;
> +	val = AR9331_SW_PORT_VLAN_FORCE_DEFALUT_VID_EN |
> +		AR9331_SW_PORT_VLAN_FORCE_PORT_VLAN_EN |
> +		FIELD_PREP(AR9331_SW_PORT_VLAN_8021Q_MODE,
> +			   AR9331_SW_8021Q_MODE_FALLBACK);
> +
> +	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
> +				 mask, val);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
> +				  AR9331_SW_PORT_VLAN_PORT_VID,
> +				  FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID,
> +					     pvid));
> +}
> +
> +static int ar9331_port_vlan_add(struct dsa_switch *ds, int port,
> +				const struct switchdev_obj_port_vlan *vlan,
> +				struct netlink_ext_ack *extack)
> +{
> +	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;

You don't need to cast a void pointer in the C language.

> +	struct regmap *regmap = priv->regmap;
> +	int ret, mode;
> +
> +	ret = ar9331_sw_port_vt_rmw(priv, vlan->vid, BIT(port), 0);
> +	if (ret)
> +		goto error;
> +
> +	if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
> +		ret = ar9331_port_vlan_set_pvid(priv, port, vlan->vid);
> +
> +	if (ret)
> +		goto error;
> +
> +	if (vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		mode = AR9331_SW_PORT_CTRL_EG_VLAN_MODE_STRIP;
> +	else
> +		mode = AR9331_SW_PORT_CTRL_EG_VLAN_MODE_ADD;
> +
> +	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_CTRL(port),
> +				 AR9331_SW_PORT_CTRL_EG_VLAN_MODE, mode);
> +	if (ret)
> +		goto error;
> +
> +	return 0;
> +error:
> +	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
> +			    ERR_PTR(ret));
> +
> +	return ret;
> +}
> +
> +static int ar9331_port_vlan_del(struct dsa_switch *ds, int port,
> +				const struct switchdev_obj_port_vlan *vlan)
> +{
> +	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> +	int ret;
> +
> +	ret = ar9331_sw_port_vt_rmw(priv, vlan->vid, 0, BIT(port));
> +	if (ret)
> +		goto error;
> +
> +	return 0;
> +
> +error:
> +	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
> +			    ERR_PTR(ret));
> +
> +	return ret;
> +}

This looks like a whole lot of boilerplate compared to:

	ret = vlan_table_read_modify_write
	if (ret)
		print_error

	return ret

> +
>  static const struct dsa_switch_ops ar9331_sw_ops = {
>  	.get_tag_protocol	= ar9331_sw_get_tag_protocol,
>  	.setup			= ar9331_sw_setup,
> @@ -1292,6 +1544,9 @@ static const struct dsa_switch_ops ar9331_sw_ops = {
>  	.port_bridge_join	= ar9331_sw_port_bridge_join,
>  	.port_bridge_leave	= ar9331_sw_port_bridge_leave,
>  	.port_stp_state_set	= ar9331_sw_port_stp_state_set,
> +	.port_vlan_filtering	= ar9331_port_vlan_filtering,
> +	.port_vlan_add		= ar9331_port_vlan_add,
> +	.port_vlan_del		= ar9331_port_vlan_del,
>  };
>  
>  static irqreturn_t ar9331_sw_irq(int irq, void *data)
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index 83b59e771a5f..40062388d4a7 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -67,6 +67,27 @@ 
 #define AR9331_SW_REG_GLOBAL_CTRL		0x30
 #define AR9331_SW_GLOBAL_CTRL_MFS_M		GENMASK(13, 0)
 
+#define AR9331_SW_NUM_VLAN_RECORDS		16
+
+#define AR9331_SW_REG_VLAN_TABLE_FUNCTION0	0x40
+#define AR9331_SW_VT0_PRI_EN			BIT(31)
+#define AR9331_SW_VT0_PRI			GENMASK(30, 28)
+#define AR9331_SW_VT0_VID			GENMASK(27, 16)
+#define AR9331_SW_VT0_PORT_NUM			GENMASK(11, 8)
+#define AR9331_SW_VT0_FULL_VIO			BIT(4)
+#define AR9331_SW_VT0_BUSY			BIT(3)
+#define AR9331_SW_VT0_FUNC			GENMASK(2, 0)
+#define AR9331_SW_VT0_FUNC_NOP			0
+#define AR9331_SW_VT0_FUNC_FLUSH_ALL		1
+#define AR9331_SW_VT0_FUNC_LOAD_ENTRY		2
+#define AR9331_SW_VT0_FUNC_PURGE_ENTRY		3
+#define AR9331_SW_VT0_FUNC_DEL_PORT		4
+#define AR9331_SW_VT0_FUNC_GET_NEXT		5
+
+#define AR9331_SW_REG_VLAN_TABLE_FUNCTION1	0x44
+#define AR9331_SW_VT1_VALID			BIT(11)
+#define AR9331_SW_VT1_VID_MEM			GENMASK(9, 0)
+
 /* Size of the address resolution table (ARL) */
 #define AR9331_SW_NUM_ARL_RECORDS		1024
 
@@ -308,6 +329,11 @@  struct ar9331_sw_port {
 	struct spinlock stats_lock;
 };
 
+struct ar9331_sw_vlan_db {
+	u16 vid;
+	u8 port_mask;
+};
+
 struct ar9331_sw_fdb {
 	u8 port_mask;
 	u8 aging;
@@ -328,6 +354,7 @@  struct ar9331_sw_priv {
 	struct ar9331_sw_port port[AR9331_SW_PORTS];
 	int cpu_port;
 	u32 isolated_ports;
+	struct ar9331_sw_vlan_db vdb[AR9331_SW_NUM_VLAN_RECORDS];
 };
 
 static struct ar9331_sw_priv *ar9331_sw_port_to_priv(struct ar9331_sw_port *port)
@@ -1273,6 +1300,231 @@  static void ar9331_sw_port_stp_state_set(struct dsa_switch *ds, int port,
 	dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
 }
 
+static int ar9331_port_vlan_filtering(struct dsa_switch *ds, int port,
+				      bool vlan_filtering,
+				      struct netlink_ext_ack *extack)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	struct regmap *regmap = priv->regmap;
+	u32 mode;
+	int ret;
+
+	if (vlan_filtering)
+		mode = AR9331_SW_8021Q_MODE_SECURE;
+	else
+		mode = AR9331_SW_8021Q_MODE_NONE;
+
+	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+				 AR9331_SW_PORT_VLAN_8021Q_MODE,
+				 FIELD_PREP(AR9331_SW_PORT_VLAN_8021Q_MODE,
+					    mode));
+	if (ret)
+		dev_err_ratelimited(priv->dev, "%s: error: %pe\n",
+				    __func__, ERR_PTR(ret));
+
+	return ret;
+}
+
+static int ar9331_sw_vt_wait(struct ar9331_sw_priv *priv, u32 *f0)
+{
+	struct regmap *regmap = priv->regmap;
+
+	return regmap_read_poll_timeout(regmap,
+				        AR9331_SW_REG_VLAN_TABLE_FUNCTION0,
+				        *f0, !(*f0 & AR9331_SW_VT0_BUSY),
+				        100, 2000);
+}
+
+static int ar9331_sw_port_vt_rmw(struct ar9331_sw_priv *priv, u16 vid,
+				 u8 port_mask_set, u8 port_mask_clr)
+{
+	struct regmap *regmap = priv->regmap;
+	u32 f0, f1, port_mask = 0, port_mask_new, func;
+	struct ar9331_sw_vlan_db *vdb = NULL;
+	int ret, i;
+
+	if (!vid)
+		return 0;
+
+	ret = ar9331_sw_vt_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, 0);
+	if (ret)
+		goto error;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, 0);
+	if (ret)
+		goto error;
+
+	for (i = 0; i < ARRAY_SIZE(priv->vdb); i++) {
+		if (priv->vdb[i].vid == vid) {
+			vdb = &priv->vdb[i];
+			break;
+		}
+	}
+
+	ret = regmap_read(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, &f1);
+	if (ret)
+		return ret;
+
+	if (vdb) {
+		port_mask = vdb->port_mask;
+	}
+
+	port_mask_new = port_mask & ~port_mask_clr;
+	port_mask_new |= port_mask_set;
+
+	if (port_mask_new && port_mask_new == port_mask) {
+		dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
+				    __func__, port_mask_new);
+		return 0;
+	}
+
+	if (port_mask_new) {
+		func = AR9331_SW_VT0_FUNC_LOAD_ENTRY;
+	} else {
+		func = AR9331_SW_VT0_FUNC_PURGE_ENTRY;
+		port_mask_new = port_mask;
+	}
+
+	if (vdb) {
+		vdb->port_mask = port_mask_new;
+
+		if (!port_mask_new)
+			vdb->vid = 0;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(priv->vdb); i++) {
+			if (!priv->vdb[i].vid) {
+				vdb = &priv->vdb[i];
+				break;
+			}
+		}
+
+		if (!vdb) {
+			dev_err_ratelimited(priv->dev, "Local VDB is full\n");
+			return -ENOMEM;
+		}
+		vdb->vid = vid;
+		vdb->port_mask = port_mask_new;
+	}
+
+	f0 = FIELD_PREP(AR9331_SW_VT0_VID, vid) |
+	     FIELD_PREP(AR9331_SW_VT0_FUNC, func) |
+	     AR9331_SW_VT0_BUSY;
+	f1 = FIELD_PREP(AR9331_SW_VT1_VID_MEM, port_mask_new) |
+		AR9331_SW_VT1_VALID;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION1, f1);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, f0);
+	if (ret)
+		return ret;
+
+	ret = ar9331_sw_vt_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	if (f0 & AR9331_SW_VT0_FULL_VIO) {
+		/* cleanup error status */
+		regmap_write(regmap, AR9331_SW_REG_VLAN_TABLE_FUNCTION0, 0);
+		dev_err_ratelimited(priv->dev, "%s: can't add new entry, VT is full\n", __func__);
+		return -ENOMEM;
+	}
+
+	return 0;
+
+error:
+	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
+			    ERR_PTR(ret));
+
+	return ret;
+}
+
+static int ar9331_port_vlan_set_pvid(struct ar9331_sw_priv *priv, int port,
+				     u16 pvid)
+{
+	struct regmap *regmap = priv->regmap;
+	int ret;
+	u32 mask, val;
+
+	mask = AR9331_SW_PORT_VLAN_8021Q_MODE |
+		AR9331_SW_PORT_VLAN_FORCE_DEFALUT_VID_EN |
+		AR9331_SW_PORT_VLAN_FORCE_PORT_VLAN_EN;
+	val = AR9331_SW_PORT_VLAN_FORCE_DEFALUT_VID_EN |
+		AR9331_SW_PORT_VLAN_FORCE_PORT_VLAN_EN |
+		FIELD_PREP(AR9331_SW_PORT_VLAN_8021Q_MODE,
+			   AR9331_SW_8021Q_MODE_FALLBACK);
+
+	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+				 mask, val);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+				  AR9331_SW_PORT_VLAN_PORT_VID,
+				  FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID,
+					     pvid));
+}
+
+static int ar9331_port_vlan_add(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_vlan *vlan,
+				struct netlink_ext_ack *extack)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	struct regmap *regmap = priv->regmap;
+	int ret, mode;
+
+	ret = ar9331_sw_port_vt_rmw(priv, vlan->vid, BIT(port), 0);
+	if (ret)
+		goto error;
+
+	if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
+		ret = ar9331_port_vlan_set_pvid(priv, port, vlan->vid);
+
+	if (ret)
+		goto error;
+
+	if (vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED)
+		mode = AR9331_SW_PORT_CTRL_EG_VLAN_MODE_STRIP;
+	else
+		mode = AR9331_SW_PORT_CTRL_EG_VLAN_MODE_ADD;
+
+	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_CTRL(port),
+				 AR9331_SW_PORT_CTRL_EG_VLAN_MODE, mode);
+	if (ret)
+		goto error;
+
+	return 0;
+error:
+	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
+			    ERR_PTR(ret));
+
+	return ret;
+}
+
+static int ar9331_port_vlan_del(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_vlan *vlan)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	int ret;
+
+	ret = ar9331_sw_port_vt_rmw(priv, vlan->vid, 0, BIT(port));
+	if (ret)
+		goto error;
+
+	return 0;
+
+error:
+	dev_err_ratelimited(priv->dev, "%s: error: %pe\n", __func__,
+			    ERR_PTR(ret));
+
+	return ret;
+}
+
 static const struct dsa_switch_ops ar9331_sw_ops = {
 	.get_tag_protocol	= ar9331_sw_get_tag_protocol,
 	.setup			= ar9331_sw_setup,
@@ -1292,6 +1544,9 @@  static const struct dsa_switch_ops ar9331_sw_ops = {
 	.port_bridge_join	= ar9331_sw_port_bridge_join,
 	.port_bridge_leave	= ar9331_sw_port_bridge_leave,
 	.port_stp_state_set	= ar9331_sw_port_stp_state_set,
+	.port_vlan_filtering	= ar9331_port_vlan_filtering,
+	.port_vlan_add		= ar9331_port_vlan_add,
+	.port_vlan_del		= ar9331_port_vlan_del,
 };
 
 static irqreturn_t ar9331_sw_irq(int irq, void *data)