* [PATCH] dsa: vsc73xx: add support for vlan filtering
@ 2021-01-20 6:30 Pawel Dembicki
2021-01-20 15:04 ` kernel test robot
2021-01-21 22:45 ` Vladimir Oltean
0 siblings, 2 replies; 9+ messages in thread
From: Pawel Dembicki @ 2021-01-20 6:30 UTC (permalink / raw)
To: netdev
Cc: Linus Wallej, Pawel Dembicki, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, linux-kernel
This patch adds support for vlan filtering in vsc73xx driver.
After vlan filtering enable, CPU_PORT is configured as trunk, without
non-tagged frames. This allows to avoid problems with transmit untagged
frames because vsc73xx is DSA_TAG_PROTO_NONE.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
drivers/net/dsa/vitesse-vsc73xx-core.c | 311 ++++++++++++++++++++++++-
1 file changed, 310 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 19ce4aa0973b..bf805eb9d3a6 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -39,6 +39,7 @@
#define VSC73XX_BLOCK_SYSTEM 0x7 /* Only subblock 0 */
#define CPU_PORT 6 /* CPU port */
+#define VLAN_TABLE_ATTEMPTS 10
/* MAC Block registers */
#define VSC73XX_MAC_CFG 0x00
@@ -62,6 +63,8 @@
#define VSC73XX_CAT_DROP 0x6e
#define VSC73XX_CAT_PR_MISC_L2 0x6f
#define VSC73XX_CAT_PR_USR_PRIO 0x75
+#define VSC73XX_CAT_VLAN_MISC 0x79
+#define VSC73XX_CAT_PORT_VLAN 0x7a
#define VSC73XX_Q_MISC_CONF 0xdf
/* MAC_CFG register bits */
@@ -122,6 +125,17 @@
#define VSC73XX_ADVPORTM_IO_LOOPBACK BIT(1)
#define VSC73XX_ADVPORTM_HOST_LOOPBACK BIT(0)
+/* TXUPDCFG transmit modify setup bits */
+#define VSC73XX_TXUPDCFG_DSCP_REWR_MODE GENMASK(20, 19)
+#define VSC73XX_TXUPDCFG_DSCP_REWR_ENA BIT(18)
+#define VSC73XX_TXUPDCFG_TX_INT_TO_USRPRIO_ENA BIT(17)
+#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID GENMASK(15, 4)
+#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA BIT(3)
+#define VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA BIT(1)
+#define VSC73XX_TXUPDCFG_TX_INSERT_TAG BIT(0)
+
+#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT 4
+
/* CAT_DROP categorizer frame dropping register bits */
#define VSC73XX_CAT_DROP_DROP_MC_SMAC_ENA BIT(6)
#define VSC73XX_CAT_DROP_FWD_CTRL_ENA BIT(4)
@@ -135,6 +149,15 @@
#define VSC73XX_Q_MISC_CONF_EARLY_TX_512 (1 << 1)
#define VSC73XX_Q_MISC_CONF_MAC_PAUSE_MODE BIT(0)
+/* CAT_VLAN_MISC categorizer VLAN miscellaneous bits*/
+#define VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA BIT(8)
+#define VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA BIT(7)
+
+/* CAT_PORT_VLAN categorizer port VLAN*/
+#define VSC73XX_CAT_PORT_VLAN_VLAN_CFI BIT(15)
+#define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12)
+#define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0)
+
/* Frame analyzer block 2 registers */
#define VSC73XX_STORMLIMIT 0x02
#define VSC73XX_ADVLEARN 0x03
@@ -185,7 +208,8 @@
#define VSC73XX_VLANACCESS_VLAN_MIRROR BIT(29)
#define VSC73XX_VLANACCESS_VLAN_SRC_CHECK BIT(28)
#define VSC73XX_VLANACCESS_VLAN_PORT_MASK GENMASK(9, 2)
-#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(2, 0)
+#define VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT 2
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(1, 0)
#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE 0
#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY 1
#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY 2
@@ -557,6 +581,287 @@ static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds,
return DSA_TAG_PROTO_NONE;
}
+static int
+vsc73xx_port_wait_for_vlan_table_cmd(struct vsc73xx *vsc, int attempts)
+{
+ u32 val;
+ int i;
+
+ for (i = 0; i <= attempts; i++) {
+ vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS,
+ &val);
+ if ((val & VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK) ==
+ VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE)
+ return 0;
+ }
+ return -EBUSY;
+}
+
+static int
+vsc73xx_port_read_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 *portmap)
+{
+ struct vsc73xx *vsc = ds->priv;
+ u32 val;
+ int ret;
+
+ if (vid > 4095)
+ return -EPERM;
+ vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid);
+ ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc, VLAN_TABLE_ATTEMPTS);
+ if (ret)
+ return ret;
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS,
+ VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK,
+ VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY);
+ ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc, VLAN_TABLE_ATTEMPTS);
+ if (ret)
+ return ret;
+ vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, &val);
+ *portmap =
+ (val & VSC73XX_VLANACCESS_VLAN_PORT_MASK) >>
+ VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT;
+ return 0;
+}
+
+static int
+vsc73xx_port_write_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 portmap)
+{
+ struct vsc73xx *vsc = ds->priv;
+ int ret;
+
+ if (vid > 4095)
+ return -EPERM;
+ vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid);
+ ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc, VLAN_TABLE_ATTEMPTS);
+ if (ret)
+ return ret;
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS,
+ VSC73XX_VLANACCESS_VLAN_SRC_CHECK |
+ VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK |
+ VSC73XX_VLANACCESS_VLAN_PORT_MASK,
+ VSC73XX_VLANACCESS_VLAN_SRC_CHECK |
+ VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY |
+ (portmap <<
+ VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT));
+ ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc, VLAN_TABLE_ATTEMPTS);
+ if (ret)
+ return ret;
+ return 0;
+}
+
+static int
+vsc73xx_port_update_vlan_table(struct dsa_switch *ds, int port, u16 vid_begin,
+ u16 vid_end, bool set)
+{
+ u8 portmap;
+ int ret;
+ u16 i;
+
+ if (vid_begin > 4095 || vid_end > 4095 || vid_begin > vid_end)
+ return -EPERM;
+
+ for (i = vid_begin; i <= vid_end; i++) {
+ ret = vsc73xx_port_read_vlan_table_entry(ds, i, &portmap);
+ if (ret)
+ return ret;
+ if (set)
+ portmap |= BIT(port);
+ else
+ portmap &= ~BIT(port);
+
+ ret = vsc73xx_port_write_vlan_table_entry(ds, i, portmap);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int vsc73xx_port_set_vlan_unaware(struct dsa_switch *ds, int port)
+{
+ struct vsc73xx *vsc = ds->priv;
+ int ret;
+
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
+ VSC73XX_MAC_CFG_VLAN_AWR,
+ ~VSC73XX_MAC_CFG_VLAN_AWR);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
+ VSC73XX_MAC_CFG_VLAN_DBLAWR,
+ ~VSC73XX_MAC_CFG_VLAN_DBLAWR);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
+ VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA,
+ VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
+ VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA,
+ VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_TAGGED_ENA,
+ ~VSC73XX_CAT_DROP_TAGGED_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA,
+ ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
+
+ ret = vsc73xx_port_update_vlan_table(ds, port, 0, 4095, 0);
+ return ret;
+}
+
+static int vsc73xx_port_set_vlan_aware(struct dsa_switch *ds, int port)
+{
+ struct vsc73xx *vsc = ds->priv;
+ int ret;
+
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
+ VSC73XX_MAC_CFG_VLAN_AWR, VSC73XX_MAC_CFG_VLAN_AWR);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
+ VSC73XX_MAC_CFG_VLAN_DBLAWR,
+ ~VSC73XX_MAC_CFG_VLAN_DBLAWR);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
+ VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA,
+ ~VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
+ VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA,
+ ~VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_TAGGED_ENA,
+ ~VSC73XX_CAT_DROP_TAGGED_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_INSERT_TAG,
+ VSC73XX_TXUPDCFG_TX_INSERT_TAG);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
+ ~VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
+
+ if (port == CPU_PORT)
+ ret = vsc73xx_port_update_vlan_table(ds, port, 0, 4095, 1);
+ else
+ ret = vsc73xx_port_update_vlan_table(ds, port, 0, 4095, 0);
+ return ret;
+}
+
+static int
+vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port,
+ bool vlan_filtering, struct switchdev_trans *trans)
+{
+ int ret;
+
+ if (switchdev_trans_ph_prepare(trans))
+ return 0;
+
+ if (vlan_filtering) {
+ ret = vsc73xx_port_set_vlan_aware(ds, port);
+ if (ret)
+ return ret;
+ ret = vsc73xx_port_set_vlan_aware(ds, CPU_PORT);
+ } else {
+ ret = vsc73xx_port_set_vlan_unaware(ds, port);
+ }
+ return ret;
+}
+
+static int vsc73xx_port_vlan_prepare(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ /* nothing needed */
+ return 0;
+}
+
+static void vsc73xx_port_vlan_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+ bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+ struct vsc73xx *vsc = ds->priv;
+ int ret;
+ u32 tmp;
+
+ if (!dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
+ return;
+
+ ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid_begin,
+ vlan->vid_end, 1);
+ if (ret)
+ return;
+
+ if (untagged && port != CPU_PORT) {
+ /* VSC73xx can have only one untagged vid per port. */
+ vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG, &tmp);
+
+ if (tmp & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)
+ dev_warn(vsc->dev,
+ "Chip support only one untagged VID per port. Overwriting...\n");
+
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID,
+ (vlan->vid_end <<
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) &
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
+ }
+ if (pvid && port != CPU_PORT) {
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA,
+ ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_CAT_PORT_VLAN,
+ VSC73XX_CAT_PORT_VLAN_VLAN_VID,
+ vlan->vid_end &
+ VSC73XX_CAT_PORT_VLAN_VLAN_VID);
+ }
+}
+
+static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+ bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+ struct vsc73xx *vsc = ds->priv;
+ u32 tmp, untagged_vid;
+ int ret;
+
+ if (!dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
+ return -EINVAL;
+
+ ret =
+ vsc73xx_port_update_vlan_table(ds, port, vlan->vid_begin,
+ vlan->vid_end, 0);
+ if (ret)
+ return ret;
+
+ /* VSC73xx can have only one untagged vid per port. Check if match. */
+ vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG, &tmp);
+ untagged_vid = (tmp & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID)
+ >> VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT;
+
+ if (untagged && untagged_vid == vlan->vid_end) {
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
+ ~VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_TXUPDCFG,
+ VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0);
+ }
+ if (pvid) {
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_CAT_DROP,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA,
+ VSC73XX_CAT_DROP_UNTAGGED_ENA);
+ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_CAT_PORT_VLAN,
+ VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0);
+ }
+ return 0;
+}
+
static int vsc73xx_setup(struct dsa_switch *ds)
{
struct vsc73xx *vsc = ds->priv;
@@ -1051,6 +1356,10 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = {
.port_disable = vsc73xx_port_disable,
.port_change_mtu = vsc73xx_change_mtu,
.port_max_mtu = vsc73xx_get_max_mtu,
+ .port_vlan_filtering = vsc73xx_port_vlan_filtering,
+ .port_vlan_prepare = vsc73xx_port_vlan_prepare,
+ .port_vlan_add = vsc73xx_port_vlan_add,
+ .port_vlan_del = vsc73xx_port_vlan_del,
};
static int vsc73xx_gpio_get(struct gpio_chip *chip, unsigned int offset)
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-20 6:30 [PATCH] dsa: vsc73xx: add support for vlan filtering Pawel Dembicki
@ 2021-01-20 15:04 ` kernel test robot
2021-01-21 22:45 ` Vladimir Oltean
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-01-20 15:04 UTC (permalink / raw)
To: Pawel Dembicki, netdev
Cc: kbuild-all, Linus Wallej, Pawel Dembicki, Andrew Lunn,
Vivien Didelot, Florian Fainelli, Vladimir Oltean,
Jakub Kicinski, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6064 bytes --]
Hi Pawel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on net-next/master ipvs/master linus/master v5.11-rc4 next-20210120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pawel-Dembicki/dsa-vsc73xx-add-support-for-vlan-filtering/20210120-161228
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 8e4052c32d6b4b39c1e13c652c7e33748d447409
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1b3778a3f0b1d668f4fa289c82997ebbd2145178
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pawel-Dembicki/dsa-vsc73xx-add-support-for-vlan-filtering/20210120-161228
git checkout 1b3778a3f0b1d668f4fa289c82997ebbd2145178
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_port_set_vlan_unaware':
>> drivers/net/dsa/vitesse-vsc73xx-core.c:686:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709535231' to '4294950911' [-Woverflow]
686 | ~VSC73XX_MAC_CFG_VLAN_AWR);
drivers/net/dsa/vitesse-vsc73xx-core.c:689:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709518847' to '4294934527' [-Woverflow]
689 | ~VSC73XX_MAC_CFG_VLAN_DBLAWR);
drivers/net/dsa/vitesse-vsc73xx-core.c:698:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551613' to '4294967293' [-Woverflow]
698 | ~VSC73XX_CAT_DROP_TAGGED_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c:701:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551611' to '4294967291' [-Woverflow]
701 | ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_port_set_vlan_aware':
drivers/net/dsa/vitesse-vsc73xx-core.c:716:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709518847' to '4294934527' [-Woverflow]
716 | ~VSC73XX_MAC_CFG_VLAN_DBLAWR);
drivers/net/dsa/vitesse-vsc73xx-core.c:719:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551359' to '4294967039' [-Woverflow]
719 | ~VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c:722:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551487' to '4294967167' [-Woverflow]
722 | ~VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c:725:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551613' to '4294967293' [-Woverflow]
725 | ~VSC73XX_CAT_DROP_TAGGED_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c:734:8: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551607' to '4294967287' [-Woverflow]
734 | ~VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_port_vlan_add':
drivers/net/dsa/vitesse-vsc73xx-core.c:811:9: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551611' to '4294967291' [-Woverflow]
811 | ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_port_vlan_del':
drivers/net/dsa/vitesse-vsc73xx-core.c:848:9: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551607' to '4294967287' [-Woverflow]
848 | ~VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
vim +686 drivers/net/dsa/vitesse-vsc73xx-core.c
678
679 static int vsc73xx_port_set_vlan_unaware(struct dsa_switch *ds, int port)
680 {
681 struct vsc73xx *vsc = ds->priv;
682 int ret;
683
684 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
685 VSC73XX_MAC_CFG_VLAN_AWR,
> 686 ~VSC73XX_MAC_CFG_VLAN_AWR);
687 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
688 VSC73XX_MAC_CFG_VLAN_DBLAWR,
689 ~VSC73XX_MAC_CFG_VLAN_DBLAWR);
690 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
691 VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA,
692 VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA);
693 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC,
694 VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA,
695 VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA);
696 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
697 VSC73XX_CAT_DROP_TAGGED_ENA,
698 ~VSC73XX_CAT_DROP_TAGGED_ENA);
699 vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP,
700 VSC73XX_CAT_DROP_UNTAGGED_ENA,
701 ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
702
703 ret = vsc73xx_port_update_vlan_table(ds, port, 0, 4095, 0);
704 return ret;
705 }
706
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72481 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-20 6:30 [PATCH] dsa: vsc73xx: add support for vlan filtering Pawel Dembicki
2021-01-20 15:04 ` kernel test robot
@ 2021-01-21 22:45 ` Vladimir Oltean
2021-01-24 23:19 ` Linus Walleij
2021-01-25 7:17 ` Paweł Dembicki
1 sibling, 2 replies; 9+ messages in thread
From: Vladimir Oltean @ 2021-01-21 22:45 UTC (permalink / raw)
To: Pawel Dembicki
Cc: netdev, Linus Wallej, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
Hi Pawel,
On Wed, Jan 20, 2021 at 07:30:18AM +0100, Pawel Dembicki wrote:
> This patch adds support for vlan filtering in vsc73xx driver.
>
> After vlan filtering enable, CPU_PORT is configured as trunk, without
> non-tagged frames. This allows to avoid problems with transmit untagged
> frames because vsc73xx is DSA_TAG_PROTO_NONE.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
What are the issues that are preventing you from getting rid of
DSA_TAG_PROTO_NONE? Not saying that making the driver VLAN aware is a
bad idea, but maybe also adding a tagging driver should really be the
path going forward. If there are hardware issues surrounding the native
tagging support, then DSA can make use of your VLAN features by
transforming them into a software-defined tagger, see
net/dsa/tag_8021q.c. But using a trunk CPU port with 8021q uppers on top
of the DSA master is a poor job of achieving that.
> ---
> +static int
> +vsc73xx_port_read_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 *portmap)
> +{
> + struct vsc73xx *vsc = ds->priv;
> + u32 val;
> + int ret;
> +
> + if (vid > 4095)
> + return -EPERM;
This is a paranoid check and should be removed (not only here but
everywhere).
> +static int vsc73xx_port_vlan_prepare(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan)
> +{
> + /* nothing needed */
> + return 0;
> +}
Can you please rebase your work on top of the net-next/master branch?
You will see that the API has changed.
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
> +
> +static void vsc73xx_port_vlan_add(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan)
> +{
> + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> + struct vsc73xx *vsc = ds->priv;
> + int ret;
> + u32 tmp;
> +
> + if (!dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
> + return;
Sorry, but no. You need to support the case where the bridge (or 8021q
module) adds a VLAN even when the port is not enforcing VLAN filtering.
See commit:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=0ee2af4ebbe3c4364429859acd571018ebfb3424
> +
> + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid_begin,
> + vlan->vid_end, 1);
> + if (ret)
> + return;
> +
> + if (untagged && port != CPU_PORT) {
> + /* VSC73xx can have only one untagged vid per port. */
> + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG, &tmp);
> +
> + if (tmp & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)
> + dev_warn(vsc->dev,
> + "Chip support only one untagged VID per port. Overwriting...\n");
Just return an error, don't overwrite, this leaves the bridge VLAN
information out of sync with the hardware otherwise, which is not a
great idea.
FWIW the drivers/net/dsa/ocelot/felix.c and drivers/net/mscc/ocelot.c
files support switching chips from the same vendor. The VSC73XX family
is much older, but some of the limitations apply to both architectures
nonetheless (like this one), you can surely borrow some ideas from
ocelot - in this case search for ocelot_vlan_prepare.
> +
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG,
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID,
> + (vlan->vid_end <<
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) &
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID);
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG,
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
> + }
> + if (pvid && port != CPU_PORT) {
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_CAT_DROP,
> + VSC73XX_CAT_DROP_UNTAGGED_ENA,
> + ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_CAT_PORT_VLAN,
> + VSC73XX_CAT_PORT_VLAN_VLAN_VID,
> + vlan->vid_end &
> + VSC73XX_CAT_PORT_VLAN_VLAN_VID);
> + }
> +}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-21 22:45 ` Vladimir Oltean
@ 2021-01-24 23:19 ` Linus Walleij
2021-01-24 23:45 ` Vladimir Oltean
2021-01-25 7:17 ` Paweł Dembicki
1 sibling, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2021-01-24 23:19 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Pawel Dembicki, netdev, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
On Thu, Jan 21, 2021 at 11:45 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Wed, Jan 20, 2021 at 07:30:18AM +0100, Pawel Dembicki wrote:
> > This patch adds support for vlan filtering in vsc73xx driver.
> >
> > After vlan filtering enable, CPU_PORT is configured as trunk, without
> > non-tagged frames. This allows to avoid problems with transmit untagged
> > frames because vsc73xx is DSA_TAG_PROTO_NONE.
> >
> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
>
> What are the issues that are preventing you from getting rid of
> DSA_TAG_PROTO_NONE? Not saying that making the driver VLAN aware is a
> bad idea, but maybe also adding a tagging driver should really be the
> path going forward.
This is due to the internal architecture of the switch, while it does
have an internal tagging format, this is stripped off before letting
it exit through the CPU port, and tagged on by the hardware
whenever the CPU transmits something. So these tags are
invisible to the CPU.
Itr would be neat if there was some bit in the switch we could
flick and then the internal tagging format would come out on
the CPU port, but sadly this does not exist.
The vendors idea is that the switch should be programmed
internally as it contains an 8051 processor that can indeed see
the internal tags. This makes a lot of sense when the chips are
used for a hardware switch, i.e. a box with several ethernet ports
on it. Sadly it is not very well adopted for the usecase of smart
operating system like linux hogging into the CPU port and
using it as a managed switch. :/
We currently have the 8051 processor in the switch disabled.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-24 23:19 ` Linus Walleij
@ 2021-01-24 23:45 ` Vladimir Oltean
2021-01-25 13:25 ` Linus Walleij
0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2021-01-24 23:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Pawel Dembicki, netdev, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
On Mon, Jan 25, 2021 at 12:19:55AM +0100, Linus Walleij wrote:
> This is due to the internal architecture of the switch, while it does
> have an internal tagging format, this is stripped off before letting
> it exit through the CPU port, and tagged on by the hardware
> whenever the CPU transmits something. So these tags are
> invisible to the CPU.
>
> Itr would be neat if there was some bit in the switch we could
> flick and then the internal tagging format would come out on
> the CPU port, but sadly this does not exist.
>
> The vendors idea is that the switch should be programmed
> internally as it contains an 8051 processor that can indeed see
> the internal tags. This makes a lot of sense when the chips are
> used for a hardware switch, i.e. a box with several ethernet ports
> on it. Sadly it is not very well adopted for the usecase of smart
> operating system like linux hogging into the CPU port and
> using it as a managed switch. :/
>
> We currently have the 8051 processor in the switch disabled.
The sad part of me not having access to any Sparx-G5e documentation
other than product briefs is that I can't actually be fully convinced
that this is true without seeing it. Other Vitesse switches support
DSA tagging towards an external CPU, so if these ones don't, the
Node Processor Interface feature must have been added later.
Anyhow, you did not approve or disprove the tag_8021q idea.
With VLAN trunking on the CPU port, how would per-port traffic be
managed? Would it be compatible with hardware-accelerated bridging
(which this driver still does not support)?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-21 22:45 ` Vladimir Oltean
2021-01-24 23:19 ` Linus Walleij
@ 2021-01-25 7:17 ` Paweł Dembicki
2021-01-28 0:37 ` Vladimir Oltean
1 sibling, 1 reply; 9+ messages in thread
From: Paweł Dembicki @ 2021-01-25 7:17 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Linus Wallej, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
czw., 21 sty 2021 o 23:45 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> Hi Pawel,
>
Hi Vladimir,
Thank You for Your answer.
> On Wed, Jan 20, 2021 at 07:30:18AM +0100, Pawel Dembicki wrote:
> > This patch adds support for vlan filtering in vsc73xx driver.
> >
> > After vlan filtering enable, CPU_PORT is configured as trunk, without
> > non-tagged frames. This allows to avoid problems with transmit untagged
> > frames because vsc73xx is DSA_TAG_PROTO_NONE.
> >
> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
>
> What are the issues that are preventing you from getting rid of
> DSA_TAG_PROTO_NONE? Not saying that making the driver VLAN aware is a
> bad idea, but maybe also adding a tagging driver should really be the
> path going forward. If there are hardware issues surrounding the native
> tagging support, then DSA can make use of your VLAN features by
> transforming them into a software-defined tagger, see
> net/dsa/tag_8021q.c. But using a trunk CPU port with 8021q uppers on top
> of the DSA master is a poor job of achieving that.
>
I was planning to prepare tagging for the next step. Without VLAN
filtering and/or tagging it is usable only as a full bridge.
Vsc73xx devices support QinQ. I can use double tagging for port
separation, but then it's impossible to filter vlans.
So, I'm planning to start working with tagging based on
net/dsa/tag_8021q.c. Should I wait with this patch and send the
corrected version with tagging support?
> > ---
> > +static int
> > +vsc73xx_port_read_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 *portmap)
> > +{
> > + struct vsc73xx *vsc = ds->priv;
> > + u32 val;
> > + int ret;
> > +
> > + if (vid > 4095)
> > + return -EPERM;
>
> This is a paranoid check and should be removed (not only here but
> everywhere).
>
> > +static int vsc73xx_port_vlan_prepare(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_port_vlan *vlan)
> > +{
> > + /* nothing needed */
> > + return 0;
> > +}
>
> Can you please rebase your work on top of the net-next/master branch?
> You will see that the API has changed.
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
>
> > +
> > +static void vsc73xx_port_vlan_add(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_port_vlan *vlan)
> > +{
> > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> > + struct vsc73xx *vsc = ds->priv;
> > + int ret;
> > + u32 tmp;
> > +
> > + if (!dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
> > + return;
>
> Sorry, but no. You need to support the case where the bridge (or 8021q
> module) adds a VLAN even when the port is not enforcing VLAN filtering.
> See commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=0ee2af4ebbe3c4364429859acd571018ebfb3424
>
> > +
> > + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid_begin,
> > + vlan->vid_end, 1);
> > + if (ret)
> > + return;
> > +
> > + if (untagged && port != CPU_PORT) {
> > + /* VSC73xx can have only one untagged vid per port. */
> > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
> > + VSC73XX_TXUPDCFG, &tmp);
> > +
> > + if (tmp & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)
> > + dev_warn(vsc->dev,
> > + "Chip support only one untagged VID per port. Overwriting...\n");
>
> Just return an error, don't overwrite, this leaves the bridge VLAN
> information out of sync with the hardware otherwise, which is not a
> great idea.
>
But it's a void return function. It always will be out of sync after
the second untagged VID attemption.
Should I give warning without changing untagged vlan?
> FWIW the drivers/net/dsa/ocelot/felix.c and drivers/net/mscc/ocelot.c
> files support switching chips from the same vendor. The VSC73XX family
> is much older, but some of the limitations apply to both architectures
> nonetheless (like this one), you can surely borrow some ideas from
> ocelot - in this case search for ocelot_vlan_prepare.
>
As Linus said, It's impossible because vsc73xx doesn't support tags
for external cpu via rgmii or regular port. This chip is very limited.
> > +
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> > + VSC73XX_TXUPDCFG,
> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID,
> > + (vlan->vid_end <<
> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) &
> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID);
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> > + VSC73XX_TXUPDCFG,
> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA);
> > + }
> > + if (pvid && port != CPU_PORT) {
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> > + VSC73XX_CAT_DROP,
> > + VSC73XX_CAT_DROP_UNTAGGED_ENA,
> > + ~VSC73XX_CAT_DROP_UNTAGGED_ENA);
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> > + VSC73XX_CAT_PORT_VLAN,
> > + VSC73XX_CAT_PORT_VLAN_VLAN_VID,
> > + vlan->vid_end &
> > + VSC73XX_CAT_PORT_VLAN_VLAN_VID);
> > + }
> > +}
Best Regards,
Pawel Dembicki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-24 23:45 ` Vladimir Oltean
@ 2021-01-25 13:25 ` Linus Walleij
0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2021-01-25 13:25 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Pawel Dembicki, netdev, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
On Mon, Jan 25, 2021 at 12:45 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> Anyhow, you did not approve or disprove the tag_8021q idea.
I just don't understand it well enough so I didn't know what to
say about that...
> With VLAN trunking on the CPU port, how would per-port traffic be
> managed? Would it be compatible with hardware-accelerated bridging
> (which this driver still does not support)?
I think in vendor code it is done like it is done on the RTL8366RB:
one VLAN per port, so the filtering inside the switch isolate the ports
by assigning them one VLAN each and the PTAG of the packets
make sure they can only reach the desired port.
(+/- my half-assed knowledge of how VLAN actually works, one
day I think I understand it, next day I don't understand it anymore)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-25 7:17 ` Paweł Dembicki
@ 2021-01-28 0:37 ` Vladimir Oltean
2021-07-27 11:57 ` Vladimir Oltean
0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2021-01-28 0:37 UTC (permalink / raw)
To: Paweł Dembicki
Cc: netdev, Linus Wallej, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
Hi Pawel,
On Mon, Jan 25, 2021 at 08:17:04AM +0100, Paweł Dembicki wrote:
> > What are the issues that are preventing you from getting rid of
> > DSA_TAG_PROTO_NONE? Not saying that making the driver VLAN aware is a
> > bad idea, but maybe also adding a tagging driver should really be the
> > path going forward. If there are hardware issues surrounding the native
> > tagging support, then DSA can make use of your VLAN features by
> > transforming them into a software-defined tagger, see
> > net/dsa/tag_8021q.c. But using a trunk CPU port with 8021q uppers on top
> > of the DSA master is a poor job of achieving that.
>
> I was planning to prepare tagging for the next step. Without VLAN
> filtering and/or tagging it is usable only as a full bridge.
> Vsc73xx devices support QinQ. I can use double tagging for port
> separation, but then it's impossible to filter vlans.
> So, I'm planning to start working with tagging based on
> net/dsa/tag_8021q.c. Should I wait with this patch and send the
> corrected version with tagging support?
I was going to send a patch some time to the DSA documentation, maybe it
clarifies some of your options.
Dealing with switches without hardware support for DSA tags
===========================================================
DSA registers a network interface for each user port, and the tagging
protocol allows the network stack to redirect Ethernet frames sent and
received by the host port into Ethernet frames sent and received by each
individual switch port. In lack of a tagging protocol, DSA will register
network interfaces that are incapable of sending or receiving traffic,
and are 'dead' from the perspective of a user space process trying to
open a socket on them. This should be avoided as much as possible for
new drivers.
Individual addressing of switch ports is a necessity for:
- supporting the default configuration where ports operate as standalone
with no bridging offload
- implementing link-layer protocols that send PDUs (STP, IEEE 1588, etc)
when the ports offload the Linux bridge
In lack of hardware support for a tagging protocol, the switch can be
configured in other ways that uniquely mark the frames sent to the CPU
with source port information (and steer them towards the correct
destination port on transmission). There is no standardized methodology
for this approach, since it depends highly on what the hardware is able
to do.
The most basic approach is to assign each port a unique pvid (port-based
VLAN, i.e. the VLAN ID that a bridge classifies untagged traffic to) and
make the CPU port a VLAN trunk. The benefit is that each port will now
be isolated from every other port in terms of forwarding, so unique
pvids per port can also be used to implement the standalone ports mode.
This is also the drawback: when offloading the Linux bridge, forwarding
will not be allowed between ports because of the VLAN restriction
imposed by the unique pvids. A workaround to this is to add all the
other ports that are members in the same bridge into the VLAN membership
of this port's pvid, but care must be taken such that the switch is
configured to operate in shared VLAN learning mode (i.e. FDB lookup is
performed ignoring the VLAN ID, just looking at the destination MAC).
There are more drawbacks to the basic approach:
(a) it would work only for VLAN-untagged traffic. VLAN tagged traffic
would be classified by the hardware to the VID derived from the VLAN
header, if that is present, unless the port is configured in
VLAN-unaware mode.
(b) if the switch port is configured in VLAN-unaware mode, it cannot
offload a Linux bridge with vlan_filtering=1, i.e. it cannot use its
hardware VLAN support for the bridge VLANs.
Further refinements to the basic approach are possible. VLAN retagging
rules can be used, which match on an ingress port equal to the front
panel port, an egress port equal to the CPU port, a VLAN ID equal to the
ingress VLAN ID, and which produce as a result a packet tagged with a
remapped VLAN ID that encodes not only the original VLAN, but also the
source port information. On transmission, VLAN-tagged traffic is
basically not a problem because most hardware is happy to transmit
double-tagged traffic. The drawbacks with VLAN retagging are usually
related to the limited number of retagging rules (which means that less
than the full 4096 VLAN ID range can typically be installed per port)
and, more importantly, the fact that encoding the source port
information in the 12-bit VLAN ID sent to the CPU will consume bits that
are used to encode the number of the original VLAN ID.
With enhancements to the Linux bridge code and to DSA, this limitation
can be somewhat alleviated by not adding a VLAN retagging rule for every
VLAN ID inserted into the hardware database, but just for those VLAN IDs
that are also present in the software VLAN database (installed with
"bridge vlan add dev br0 self"). The other VLANs will just allow
autonomous forwarding and no packet delivery to the CPU, which works
because a VLAN-aware bridge would drop VLAN-tagged packets if their VLAN
ID is not present in the software database anyway.
In short, the basic approach relies on port VLAN membership, and unless
VLAN retagging is used, means that a choice needs to be made regarding
whether the offload for a VLAN-aware bridge is to be supported, or DSA
tagging is to be supported.
With even more enhancements to the bridge and DSA data path, the source
port information might not even matter for the network stack when the
port is bridged, since the packet will end up in the data path of the
bridge anyway, regardless of which bridged port the packet came in
through (a notable exception to this is link-local traffic like bridge
PDUs - some switches treat link-local packets differently than normal
data ones). On transmission, the imprecise steering to the correct
egress port poses further complications, because of the flooding
implementation in the Linux bridge: a packet that is to be flooded
towards swp0, swp1 and swp2 will be cloned by the bridge once per egress
port, and each skb will be individually delivered to each net_device
driver for xmit. The bridge does not know that the packet transmission
through a DSA driver with no tagging protocol is imprecise, and instead
of delivering the packet just towards the requested egress port, that
the switch will likely flood the packet. So each packet will end up
flooded once by the software bridge, and twice by the hardware bridge.
This can be modeled as a TX-side offload for packet flooding, and could
be used to prevent the bridge from cloning the packets in the first
place, and just deliver them once to a randomly chosen port which is
bridged.
The enhancements discussed earlier make one assumption: that link-local
traffic is to be treated separately, and that the hardware has
provisioning for decoding the source port at least for those. In fact,
link-local traffic is an issue also on transmission, since BPDUs need to
be transmitted by the network stack even towards ports that are in the
BLOCKING STP state. A tagging protocol implementation based purely on
VLAN port membership will not help with that.
A radically different implementation approach based still on VLAN IDs is
to make use of packet processing pipelines in the switch that are not
coupled with the bridging service, but work independently on a port's
ingress and egress pipeline. Typically, these are configured by the user
as an offload to tc-flower filters.
Identifying the ingress switch port can be achieved by pushing the
desired VLAN as a second, outer tag, on egress towards the CPU port.
In tc-flower pseudocode it would look like this:
$ tc qdisc add dev <cpu-port> clsact
$ for eth in swp0 swp1 swp2 swp3; do \
tc filter add dev <cpu-port> egress flower indev ${eth} \
action vlan push id <rxvlan> protocol 802.1ad; \
done
Other ways of pushing an outer VLAN, such as based on 802.1ad bridging,
are useless because they basically require that the port is VLAN-unaware,
which lands us in the same spot as the basic approach with port-based
VLAN membership, being unable to offload a VLAN-aware bridge.
As for steering traffic injected into the switch from the network stack,
it can be done using a combination of a redirect rule that matches on
VLAN ID, plus another VLAN pop action chained to it.
$ tc qdisc add dev <cpu-port> clsact
$ for eth in swp0 swp1 swp2 swp3; do \
tc filter add dev <cpu-port> ingress protocol 802.1Q flower
vlan_id <txvlan> action vlan pop \
action mirred egress redirect dev ${eth}; \
done
But since DSA does not register network interfaces for the CPU port,
this configuration would be impossible for the user to do. So this
configuration, if supported by the hardware, should be done privately
by the driver.
The advantage is that since the packet processing pipeline is not
coupled with the bridging service, then a VLAN-aware Linux bridge can
still be offloaded. The drawback is that chances are low for a switch to
not have hardware support for a tagging protocol, but to support a
programmable packet pipeline, which may make the idea impractical.
----------------------------
So there you have it, this is my trainwreck of thoughts, you should
think about the use cases you need and choose appropriately. It is
unfortunately not easy and you will need to find compromises.
Finally, it may be that the switch is not really a DSA switch (you can
be certain that this is the case if you can't even decode link-local
traffic, or inject link-local traffic into a BLOCKING port) and you are
doing it a disservice by treating it like one.
Non-DSA switches are devices that have non-Ethernet based methods of
sending and receiving packets.
> > Can you please rebase your work on top of the net-next/master branch?
> > You will see that the API has changed.
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
> >
> > > +
> > > +static void vsc73xx_port_vlan_add(struct dsa_switch *ds, int port,
> > > + const struct switchdev_obj_port_vlan *vlan)
> > > +{
> > > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> > > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> > > + struct vsc73xx *vsc = ds->priv;
> > > + int ret;
> > > + u32 tmp;
> > > +
> > > + if (!dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
> > > + return;
> >
> > Sorry, but no. You need to support the case where the bridge (or 8021q
> > module) adds a VLAN even when the port is not enforcing VLAN filtering.
> > See commit:
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=0ee2af4ebbe3c4364429859acd571018ebfb3424
> >
> > > +
> > > + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid_begin,
> > > + vlan->vid_end, 1);
> > > + if (ret)
> > > + return;
> > > +
> > > + if (untagged && port != CPU_PORT) {
> > > + /* VSC73xx can have only one untagged vid per port. */
> > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
> > > + VSC73XX_TXUPDCFG, &tmp);
> > > +
> > > + if (tmp & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)
> > > + dev_warn(vsc->dev,
> > > + "Chip support only one untagged VID per port. Overwriting...\n");
> >
> > Just return an error, don't overwrite, this leaves the bridge VLAN
> > information out of sync with the hardware otherwise, which is not a
> > great idea.
> >
>
> But it's a void return function. It always will be out of sync after
> the second untagged VID attemption.
> Should I give warning without changing untagged vlan?
If you had checked the updated API, you'd have noticed that the function
no longer returns void.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] dsa: vsc73xx: add support for vlan filtering
2021-01-28 0:37 ` Vladimir Oltean
@ 2021-07-27 11:57 ` Vladimir Oltean
0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Oltean @ 2021-07-27 11:57 UTC (permalink / raw)
To: Paweł Dembicki
Cc: netdev, Linus Wallej, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Jakub Kicinski, linux-kernel
Hi Pawel,
On Thu, Jan 28, 2021 at 02:37:55AM +0200, Vladimir Oltean wrote:
> With even more enhancements to the bridge and DSA data path, the source
> port information might not even matter for the network stack when the
> port is bridged, since the packet will end up in the data path of the
> bridge anyway, regardless of which bridged port the packet came in
> through (a notable exception to this is link-local traffic like bridge
> PDUs - some switches treat link-local packets differently than normal
> data ones). On transmission, the imprecise steering to the correct
> egress port poses further complications, because of the flooding
> implementation in the Linux bridge: a packet that is to be flooded
> towards swp0, swp1 and swp2 will be cloned by the bridge once per egress
> port, and each skb will be individually delivered to each net_device
> driver for xmit. The bridge does not know that the packet transmission
> through a DSA driver with no tagging protocol is imprecise, and instead
> of delivering the packet just towards the requested egress port, that
> the switch will likely flood the packet. So each packet will end up
> flooded once by the software bridge, and twice by the hardware bridge.
> This can be modeled as a TX-side offload for packet flooding, and could
> be used to prevent the bridge from cloning the packets in the first
> place, and just deliver them once to a randomly chosen port which is
> bridged.
Did you make any progress with getting rid of DSA_TAG_PROTO_NONE for
vsc73xx?
Just FYI, the bridge and DSA enhancement that I was talking about above
got accepted and you should be able to make use of it.
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=356ae88f8322066a2cd1aee831b7fb768ff2905c
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=beeee08ca1d432891f9e1f6188eea85ffac68527
Assuming you haven't solved your issue with link-local packets, you
should at least be able to configure the switch as follows:
- in standalone mode and under a VLAN-unaware bridge: enable Shared VLAN
Learning, set VLAN_TCI_IGNORE_ENA to always classify packets to the
port-based VLAN and not look at the VLAN headers, reserve the
1024-3071 VID range for tag_8021q, and call dsa_tag_8021q_register().
- under a VLAN-aware bridge: same as the above except enable Independent
VLAN Learning and disable VLAN_TCI_IGNORE_ENA. Packets from ports
under a VLAN-aware bridge will come tagged with the bridge VLAN, so
you can draw inspiration from net/dsa/tag_sja1105.c to see how to
perform reception for those (dsa_find_designated_bridge_port_by_vid).
This should give you the ability to expose the switch as an STP-incapable
bridge accelerator with port isolation and VLAN support, which frankly
seems about all that the hardware can offer.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-27 11:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 6:30 [PATCH] dsa: vsc73xx: add support for vlan filtering Pawel Dembicki
2021-01-20 15:04 ` kernel test robot
2021-01-21 22:45 ` Vladimir Oltean
2021-01-24 23:19 ` Linus Walleij
2021-01-24 23:45 ` Vladimir Oltean
2021-01-25 13:25 ` Linus Walleij
2021-01-25 7:17 ` Paweł Dembicki
2021-01-28 0:37 ` Vladimir Oltean
2021-07-27 11:57 ` Vladimir Oltean
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).