[PATCH net 00/11] mlxsw: Various fixes

[PATCH net 00/11] mlxsw: Various fixes

[Ido Schimmel]

Patches #1-#2 from Nir fix two bugs in recent ACL work. First patch adds
a missing cleanup in error path, while second patch removes an easily
triggerable false warning.

Patch #3 from Jiri adds a missing cleanup when unlinking a port from
LAG.

Patches #4-#9 fix various bugs in recent VXLAN patches and add test
cases.

Patches #10-#11 correctly set the PVID on a bridge port during VLAN
deletion and add a test case.

Please consider patches #3 and #10 for stable.

Ido Schimmel (8):
  mlxsw: spectrum: Add VXLAN dependency for spectrum
  mlxsw: spectrum_switchdev: Avoid returning errors in commit phase
  mlxsw: spectrum_nve: Replace error code with EINVAL
  selftests: mlxsw: Add a test case for VLAN addition error flow
  net: bridge: Fix VLANs memory leak
  selftests: forwarding: Fix test for different devices
  mlxsw: spectrum_switchdev: Set PVID correctly during VLAN deletion
  selftests: forwarding: Add a test for VLAN deletion

Jiri Pirko (1):
  mlxsw: spectrum: Disable lag port TX before removing it

Nir Dotan (2):
  mlxsw: spectrum_acl: Add cleanup after C-TCAM update error condition
  mlxsw: spectrum_acl: Remove ASSERT_RTNL()s in module removal flow

 drivers/net/ethernet/mellanox/mlxsw/Kconfig   |  1 +
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  7 +++--
 .../mellanox/mlxsw/spectrum_acl_ctcam.c       | 10 ++++++-
 .../mellanox/mlxsw/spectrum_acl_erp.c         |  2 --
 .../ethernet/mellanox/mlxsw/spectrum_nve.c    |  4 +--
 .../mellanox/mlxsw/spectrum_switchdev.c       | 23 +++++++---------
 net/bridge/br_private.h                       |  1 +
 net/bridge/br_vlan.c                          | 26 +++++++++----------
 .../selftests/drivers/net/mlxsw/vxlan.sh      | 18 +++++++++++++
 .../net/forwarding/bridge_vlan_aware.sh       | 15 ++++++++++-
 .../net/forwarding/vxlan_bridge_1d.sh         |  2 +-
 11 files changed, 74 insertions(+), 35 deletions(-)

-- 
2.20.1

[PATCH net 01/11] mlxsw: spectrum_acl: Add cleanup after C-TCAM update error condition

[Ido Schimmel]

From: Nir Dotan <nird@mellanox.com>

When writing to C-TCAM, mlxsw driver uses cregion->ops->entry_insert().
In case of C-TCAM HW insertion error, the opposite action should take
place.
Add error handling case in which the C-TCAM region entry is removed, by
calling cregion->ops->entry_remove().

Fixes: a0a777b9409f ("mlxsw: spectrum_acl: Start using A-TCAM")
Signed-off-by: Nir Dotan <nird@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_acl_ctcam.c   | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_ctcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_ctcam.c
index b0f2d8e8ded0..ac222833a5cf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_ctcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_ctcam.c
@@ -72,7 +72,15 @@ mlxsw_sp_acl_ctcam_region_entry_insert(struct mlxsw_sp *mlxsw_sp,
 	act_set = mlxsw_afa_block_first_set(rulei->act_block);
 	mlxsw_reg_ptce2_flex_action_set_memcpy_to(ptce2_pl, act_set);
 
-	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ptce2), ptce2_pl);
+	err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ptce2), ptce2_pl);
+	if (err)
+		goto err_ptce2_write;
+
+	return 0;
+
+err_ptce2_write:
+	cregion->ops->entry_remove(cregion, centry);
+	return err;
 }
 
 static void
-- 
2.20.1

[PATCH net 02/11] mlxsw: spectrum_acl: Remove ASSERT_RTNL()s in module removal flow

[Ido Schimmel]

From: Nir Dotan <nird@mellanox.com>

Removal of the mlxsw driver on Spectrum-2 platforms hits an ASSERT_RTNL()
in Spectrum-2 ACL Bloom filter and in ERP removal paths. This happens
because the multicast router implementation in Spectrum-2 relies on ACLs.
Taking the RTNL lock upon driver removal is useless since the driver first
removes its ports and unregisters from notifiers so concurrent writes
cannot happen at that time. The assertions were originally put as a
reminder for future work involving ERP background optimization, but having
these assertions only during addition serves this purpose as well.

Therefore remove the ASSERT_RTNL() in both places related to ERP and Bloom
filter removal.

Fixes: cf7221a4f5a5 ("mlxsw: spectrum_router: Add Multicast routing support for Spectrum-2")
Signed-off-by: Nir Dotan <nird@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
index 1c19feefa5f2..2941967e1cc5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
@@ -1022,7 +1022,6 @@ void mlxsw_sp_acl_erp_mask_put(struct mlxsw_sp_acl_atcam_region *aregion,
 {
 	struct objagg_obj *objagg_obj = (struct objagg_obj *) erp_mask;
 
-	ASSERT_RTNL();
 	objagg_obj_put(aregion->erp_table->objagg, objagg_obj);
 }
 
@@ -1054,7 +1053,6 @@ void mlxsw_sp_acl_erp_bf_remove(struct mlxsw_sp *mlxsw_sp,
 	const struct mlxsw_sp_acl_erp *erp = objagg_obj_root_priv(objagg_obj);
 	unsigned int erp_bank;
 
-	ASSERT_RTNL();
 	if (!mlxsw_sp_acl_erp_table_is_used(erp->erp_table))
 		return;
 
-- 
2.20.1

[PATCH net 03/11] mlxsw: spectrum: Disable lag port TX before removing it

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

Make sure that lag port TX is disabled before mlxsw_sp_port_lag_leave()
is called and prevent from possible EMAD error.

Fixes: 0d65fc13042f ("mlxsw: spectrum: Implement LAG port join/leave")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index eed1045e4d96..32519c93df17 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -5005,12 +5005,15 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev,
 							   lower_dev,
 							   upper_dev);
 		} else if (netif_is_lag_master(upper_dev)) {
-			if (info->linking)
+			if (info->linking) {
 				err = mlxsw_sp_port_lag_join(mlxsw_sp_port,
 							     upper_dev);
-			else
+			} else {
+				mlxsw_sp_port_lag_tx_en_set(mlxsw_sp_port,
+							    false);
 				mlxsw_sp_port_lag_leave(mlxsw_sp_port,
 							upper_dev);
+			}
 		} else if (netif_is_ovs_master(upper_dev)) {
 			if (info->linking)
 				err = mlxsw_sp_port_ovs_join(mlxsw_sp_port);
-- 
2.20.1

[PATCH net 04/11] mlxsw: spectrum: Add VXLAN dependency for spectrum

[Ido Schimmel]

When VXLAN is a loadable module, MLXSW_SPECTRUM must not be built-in:

drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:2547: undefined
reference to `vxlan_fdb_find_uc'

Add Kconfig dependency to enforce usable configurations.

Fixes: 1231e04f5bba ("mlxsw: spectrum_switchdev: Add support for VxLAN encapsulation")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index 080ddd1942ec..b9a25aed5d11 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -78,6 +78,7 @@ config MLXSW_SPECTRUM
 	depends on IPV6 || IPV6=n
 	depends on NET_IPGRE || NET_IPGRE=n
 	depends on IPV6_GRE || IPV6_GRE=n
+	depends on VXLAN || VXLAN=n
 	select GENERIC_ALLOCATOR
 	select PARMAN
 	select OBJAGG
-- 
2.20.1

[PATCH net 05/11] mlxsw: spectrum_switchdev: Avoid returning errors in commit phase

[Ido Schimmel]

Drivers are not supposed to return errors in switchdev commit phase if
they returned OK in prepare phase. Otherwise, a WARNING is emitted.
However, when the offloading of a VXLAN tunnel is triggered by the
addition of a VLAN on a local port, it is not possible to guarantee that
the commit phase will succeed without doing a lot of work.

In these cases, the artificial division between prepare and commit phase
does not make sense, so simply do the work in the prepare phase.

Fixes: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
 .../mellanox/mlxsw/spectrum_switchdev.c       | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 1bd2c6e15f8d..e8ce2307352b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1078,8 +1078,7 @@ static int
 mlxsw_sp_bridge_port_vlan_add(struct mlxsw_sp_port *mlxsw_sp_port,
 			      struct mlxsw_sp_bridge_port *bridge_port,
 			      u16 vid, bool is_untagged, bool is_pvid,
-			      struct netlink_ext_ack *extack,
-			      struct switchdev_trans *trans)
+			      struct netlink_ext_ack *extack)
 {
 	u16 pvid = mlxsw_sp_port_pvid_determine(mlxsw_sp_port, vid, is_pvid);
 	struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan;
@@ -1095,9 +1094,6 @@ mlxsw_sp_bridge_port_vlan_add(struct mlxsw_sp_port *mlxsw_sp_port,
 	    mlxsw_sp_port_vlan->bridge_port != bridge_port)
 		return -EEXIST;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (!mlxsw_sp_port_vlan) {
 		mlxsw_sp_port_vlan = mlxsw_sp_port_vlan_create(mlxsw_sp_port,
 							       vid);
@@ -1188,6 +1184,9 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 		return err;
 	}
 
+	if (switchdev_trans_ph_commit(trans))
+		return 0;
+
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
 	if (WARN_ON(!bridge_port))
 		return -EINVAL;
@@ -1200,7 +1199,7 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 
 		err = mlxsw_sp_bridge_port_vlan_add(mlxsw_sp_port, bridge_port,
 						    vid, flag_untagged,
-						    flag_pvid, extack, trans);
+						    flag_pvid, extack);
 		if (err)
 			return err;
 	}
@@ -3207,7 +3206,6 @@ mlxsw_sp_switchdev_vxlan_vlan_add(struct mlxsw_sp *mlxsw_sp,
 				  struct mlxsw_sp_bridge_device *bridge_device,
 				  const struct net_device *vxlan_dev, u16 vid,
 				  bool flag_untagged, bool flag_pvid,
-				  struct switchdev_trans *trans,
 				  struct netlink_ext_ack *extack)
 {
 	struct vxlan_dev *vxlan = netdev_priv(vxlan_dev);
@@ -3225,9 +3223,6 @@ mlxsw_sp_switchdev_vxlan_vlan_add(struct mlxsw_sp *mlxsw_sp,
 	    mlxsw_sp_bridge_8021q_vxlan_dev_find(bridge_device->dev, vid))
 		return -EINVAL;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (!netif_running(vxlan_dev))
 		return 0;
 
@@ -3345,6 +3340,9 @@ mlxsw_sp_switchdev_vxlan_vlans_add(struct net_device *vxlan_dev,
 
 	port_obj_info->handled = true;
 
+	if (switchdev_trans_ph_commit(trans))
+		return 0;
+
 	bridge_device = mlxsw_sp_bridge_device_find(mlxsw_sp->bridge, br_dev);
 	if (!bridge_device)
 		return -EINVAL;
@@ -3358,8 +3356,7 @@ mlxsw_sp_switchdev_vxlan_vlans_add(struct net_device *vxlan_dev,
 		err = mlxsw_sp_switchdev_vxlan_vlan_add(mlxsw_sp, bridge_device,
 							vxlan_dev, vid,
 							flag_untagged,
-							flag_pvid, trans,
-							extack);
+							flag_pvid, extack);
 		if (err)
 			return err;
 	}
-- 
2.20.1

[PATCH net 06/11] mlxsw: spectrum_nve: Replace error code with EINVAL

[Ido Schimmel]

Adding a VLAN on a port can trigger the offload of a VXLAN tunnel which
is already a member in the VLAN. In case the configuration of the VXLAN
is not supported, the driver would return -EOPNOTSUPP.

This is problematic since bridge code does not interpret this as error,
but rather that it should try to setup the VLAN using the 8021q driver
instead of switchdev.

Fixes: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
index 0a31fff2516e..fb1c48c698f2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
@@ -816,14 +816,14 @@ int mlxsw_sp_nve_fid_enable(struct mlxsw_sp *mlxsw_sp, struct mlxsw_sp_fid *fid,
 	ops = nve->nve_ops_arr[params->type];
 
 	if (!ops->can_offload(nve, params->dev, extack))
-		return -EOPNOTSUPP;
+		return -EINVAL;
 
 	memset(&config, 0, sizeof(config));
 	ops->nve_config(nve, params->dev, &config);
 	if (nve->num_nve_tunnels &&
 	    memcmp(&config, &nve->config, sizeof(config))) {
 		NL_SET_ERR_MSG_MOD(extack, "Conflicting NVE tunnels configuration");
-		return -EOPNOTSUPP;
+		return -EINVAL;
 	}
 
 	err = mlxsw_sp_nve_tunnel_init(mlxsw_sp, &config);
-- 
2.20.1

[PATCH net 07/11] selftests: mlxsw: Add a test case for VLAN addition error flow

[Ido Schimmel]

Add a test case for the issue fixed by previous commit. In case the
offloading of an unsupported VxLAN tunnel was triggered by adding the
mapped VLAN to a local port, then error should be returned to the user.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/vxlan.sh       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/vxlan.sh b/tools/testing/selftests/drivers/net/mlxsw/vxlan.sh
index dcf9f4e913e0..ae6146ec5afd 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/vxlan.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/vxlan.sh
@@ -847,6 +847,24 @@ sanitization_vlan_aware_test()
 
 	log_test "vlan-aware - failed enslavement to vlan-aware bridge"
 
+	bridge vlan del vid 10 dev vxlan20
+	bridge vlan add vid 20 dev vxlan20 pvid untagged
+
+	# Test that offloading of an unsupported tunnel fails when it is
+	# triggered by addition of VLAN to a local port
+	RET=0
+
+	# TOS must be set to inherit
+	ip link set dev vxlan10 type vxlan tos 42
+
+	ip link set dev $swp1 master br0
+	bridge vlan add vid 10 dev $swp1 &> /dev/null
+	check_fail $?
+
+	log_test "vlan-aware - failed vlan addition to a local port"
+
+	ip link set dev vxlan10 type vxlan tos inherit
+
 	ip link del dev vxlan20
 	ip link del dev vxlan10
 	ip link del dev br0
-- 
2.20.1

[PATCH net 08/11] net: bridge: Fix VLANs memory leak

[Ido Schimmel]

When adding / deleting VLANs to / from a bridge port, the bridge driver
first tries to propagate the information via switchdev and falls back to
the 8021q driver in case the underlying driver does not support
switchdev. This can result in a memory leak [1] when VXLAN and mlxsw
ports are enslaved to the bridge:

$ ip link set dev vxlan0 master br0
# No mlxsw ports are enslaved to 'br0', so mlxsw ignores the switchdev
# notification and the bridge driver adds the VLAN on 'vxlan0' via the
# 8021q driver
$ bridge vlan add vid 10 dev vxlan0 pvid untagged
# mlxsw port is enslaved to the bridge
$ ip link set dev swp1 master br0
# mlxsw processes the switchdev notification and the 8021q driver is
# skipped
$ bridge vlan del vid 10 dev vxlan0

This results in 'struct vlan_info' and 'struct vlan_vid_info' being
leaked, as they were allocated by the 8021q driver during VLAN addition,
but never freed as the 8021q driver was skipped during deletion.

Fix this by introducing a new VLAN private flag that indicates whether
the VLAN was added on the port by switchdev or the 8021q driver. If the
VLAN was added by the 8021q driver, then we make sure to delete it via
the 8021q driver as well.

[1]
unreferenced object 0xffff88822d20b1e8 (size 256):
  comm "bridge", pid 2532, jiffies 4295216998 (age 1188.830s)
  hex dump (first 32 bytes):
    e0 42 97 ce 81 88 ff ff 00 00 00 00 00 00 00 00  .B..............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
    [<00000000e0178b02>] vlan_vid_add+0x661/0x920
    [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
    [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
    [<000000003535392c>] br_vlan_info+0x132/0x410
    [<00000000aedaa9dc>] br_afspec+0x75c/0x870
    [<00000000f5716133>] br_setlink+0x3dc/0x6d0
    [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
    [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
    [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
    [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
    [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
    [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
    [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
    [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
    [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270
unreferenced object 0xffff888227454308 (size 32):
  comm "bridge", pid 2532, jiffies 4295216998 (age 1188.882s)
  hex dump (first 32 bytes):
    88 b2 20 2d 82 88 ff ff 88 b2 20 2d 82 88 ff ff  .. -...... -....
    81 00 0a 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
    [<0000000018050631>] vlan_vid_add+0x3e6/0x920
    [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
    [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
    [<000000003535392c>] br_vlan_info+0x132/0x410
    [<00000000aedaa9dc>] br_afspec+0x75c/0x870
    [<00000000f5716133>] br_setlink+0x3dc/0x6d0
    [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
    [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
    [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
    [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
    [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
    [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
    [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
    [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
    [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270

Fixes: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Cc: bridge@lists.linux-foundation.org
---
 net/bridge/br_private.h |  1 +
 net/bridge/br_vlan.c    | 26 +++++++++++++-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d240b3e7919f..eabf8bf28a3f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -107,6 +107,7 @@ struct br_tunnel_info {
 /* private vlan flags */
 enum {
 	BR_VLFLAG_PER_PORT_STATS = BIT(0),
+	BR_VLFLAG_ADDED_BY_SWITCHDEV = BIT(1),
 };
 
 /**
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 4a2f31157ef5..96abf8feb9dc 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -80,16 +80,18 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
 }
 
 static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
-			  u16 vid, u16 flags, struct netlink_ext_ack *extack)
+			  struct net_bridge_vlan *v, u16 flags,
+			  struct netlink_ext_ack *extack)
 {
 	int err;
 
 	/* Try switchdev op first. In case it is not supported, fallback to
 	 * 8021q add.
 	 */
-	err = br_switchdev_port_vlan_add(dev, vid, flags, extack);
+	err = br_switchdev_port_vlan_add(dev, v->vid, flags, extack);
 	if (err == -EOPNOTSUPP)
-		return vlan_vid_add(dev, br->vlan_proto, vid);
+		return vlan_vid_add(dev, br->vlan_proto, v->vid);
+	v->priv_flags |= BR_VLFLAG_ADDED_BY_SWITCHDEV;
 	return err;
 }
 
@@ -121,19 +123,17 @@ static void __vlan_del_list(struct net_bridge_vlan *v)
 }
 
 static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
-			  u16 vid)
+			  const struct net_bridge_vlan *v)
 {
 	int err;
 
 	/* Try switchdev op first. In case it is not supported, fallback to
 	 * 8021q del.
 	 */
-	err = br_switchdev_port_vlan_del(dev, vid);
-	if (err == -EOPNOTSUPP) {
-		vlan_vid_del(dev, br->vlan_proto, vid);
-		return 0;
-	}
-	return err;
+	err = br_switchdev_port_vlan_del(dev, v->vid);
+	if (!(v->priv_flags & BR_VLFLAG_ADDED_BY_SWITCHDEV))
+		vlan_vid_del(dev, br->vlan_proto, v->vid);
+	return err == -EOPNOTSUPP ? 0 : err;
 }
 
 /* Returns a master vlan, if it didn't exist it gets created. In all cases a
@@ -242,7 +242,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
 		 * This ensures tagged traffic enters the bridge when
 		 * promiscuous mode is disabled by br_manage_promisc().
 		 */
-		err = __vlan_vid_add(dev, br, v->vid, flags, extack);
+		err = __vlan_vid_add(dev, br, v, flags, extack);
 		if (err)
 			goto out;
 
@@ -305,7 +305,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
 
 out_filt:
 	if (p) {
-		__vlan_vid_del(dev, br, v->vid);
+		__vlan_vid_del(dev, br, v);
 		if (masterv) {
 			if (v->stats && masterv->stats != v->stats)
 				free_percpu(v->stats);
@@ -338,7 +338,7 @@ static int __vlan_del(struct net_bridge_vlan *v)
 
 	__vlan_delete_pvid(vg, v->vid);
 	if (p) {
-		err = __vlan_vid_del(p->dev, p->br, v->vid);
+		err = __vlan_vid_del(p->dev, p->br, v);
 		if (err)
 			goto out;
 	} else {
-- 
2.20.1

Re: [PATCH net 08/11] net: bridge: Fix VLANs memory leak

[Nikolay Aleksandrov]

On 08/01/2019 18:48, Ido Schimmel wrote:
> When adding / deleting VLANs to / from a bridge port, the bridge driver
> first tries to propagate the information via switchdev and falls back to
> the 8021q driver in case the underlying driver does not support
> switchdev. This can result in a memory leak [1] when VXLAN and mlxsw
> ports are enslaved to the bridge:
> 
> $ ip link set dev vxlan0 master br0
> # No mlxsw ports are enslaved to 'br0', so mlxsw ignores the switchdev
> # notification and the bridge driver adds the VLAN on 'vxlan0' via the
> # 8021q driver
> $ bridge vlan add vid 10 dev vxlan0 pvid untagged
> # mlxsw port is enslaved to the bridge
> $ ip link set dev swp1 master br0
> # mlxsw processes the switchdev notification and the 8021q driver is
> # skipped
> $ bridge vlan del vid 10 dev vxlan0
> 
> This results in 'struct vlan_info' and 'struct vlan_vid_info' being
> leaked, as they were allocated by the 8021q driver during VLAN addition,
> but never freed as the 8021q driver was skipped during deletion.
> 
> Fix this by introducing a new VLAN private flag that indicates whether
> the VLAN was added on the port by switchdev or the 8021q driver. If the
> VLAN was added by the 8021q driver, then we make sure to delete it via
> the 8021q driver as well.
> 
> [1]
> unreferenced object 0xffff88822d20b1e8 (size 256):
>   comm "bridge", pid 2532, jiffies 4295216998 (age 1188.830s)
>   hex dump (first 32 bytes):
>     e0 42 97 ce 81 88 ff ff 00 00 00 00 00 00 00 00  .B..............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
>     [<00000000e0178b02>] vlan_vid_add+0x661/0x920
>     [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
>     [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
>     [<000000003535392c>] br_vlan_info+0x132/0x410
>     [<00000000aedaa9dc>] br_afspec+0x75c/0x870
>     [<00000000f5716133>] br_setlink+0x3dc/0x6d0
>     [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
>     [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
>     [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
>     [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
>     [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
>     [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
>     [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
>     [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
>     [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270
> unreferenced object 0xffff888227454308 (size 32):
>   comm "bridge", pid 2532, jiffies 4295216998 (age 1188.882s)
>   hex dump (first 32 bytes):
>     88 b2 20 2d 82 88 ff ff 88 b2 20 2d 82 88 ff ff  .. -...... -....
>     81 00 0a 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
>     [<0000000018050631>] vlan_vid_add+0x3e6/0x920
>     [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
>     [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
>     [<000000003535392c>] br_vlan_info+0x132/0x410
>     [<00000000aedaa9dc>] br_afspec+0x75c/0x870
>     [<00000000f5716133>] br_setlink+0x3dc/0x6d0
>     [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
>     [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
>     [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
>     [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
>     [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
>     [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
>     [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
>     [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
>     [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270
> 
> Fixes: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reviewed-by: Petr Machata <petrm@mellanox.com>
> Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Cc: bridge@lists.linux-foundation.org
> ---
>  net/bridge/br_private.h |  1 +
>  net/bridge/br_vlan.c    | 26 +++++++++++++-------------
>  2 files changed, 14 insertions(+), 13 deletions(-)

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

[PATCH net 09/11] selftests: forwarding: Fix test for different devices

[Ido Schimmel]

When running the test on the Spectrum ASIC the generated packets are
counted on the ingress filter and injected back to the pipeline because
of the 'pass' action. The router block then drops the packets due to
checksum error, as the test generates packets with zero checksum.

When running the test on an emulator that is not as strict about
checksum errors the test fails since packets are counted twice. Once by
the emulated ASIC on its ingress filter and again by the kernel as the
emulator does not perform checksum validation and allows the packets to
be trapped by a matching host route.

Fix this by changing the action to 'drop', which will prevent the packet
from continuing further in the pipeline to the router block.

For veth pairs this change is essentially a NOP given packets are only
processed once (by the kernel).

Fixes: a0b61f3d8ebf ("selftests: forwarding: vxlan_bridge_1d: Add an ECN decap test")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
index 56cef3b1c194..bb10e33690b2 100755
--- a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
+++ b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
@@ -629,7 +629,7 @@ __test_ecn_decap()
 	RET=0
 
 	tc filter add dev $h1 ingress pref 77 prot ip \
-		flower ip_tos $decapped_tos action pass
+		flower ip_tos $decapped_tos action drop
 	sleep 1
 	vxlan_encapped_ping_test v2 v1 192.0.2.17 \
 				 $orig_inner_tos $orig_outer_tos \
-- 
2.20.1

[PATCH net 10/11] mlxsw: spectrum_switchdev: Set PVID correctly during VLAN deletion

[Ido Schimmel]

When a VLAN is deleted from a bridge port we should not change the PVID
unless the deleted VLAN is the PVID.

Fixes: fe9ccc785de5 ("mlxsw: spectrum_switchdev: Don't batch VLAN operations")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index e8ce2307352b..0abbaa0fbf14 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1807,7 +1807,7 @@ static void
 mlxsw_sp_bridge_port_vlan_del(struct mlxsw_sp_port *mlxsw_sp_port,
 			      struct mlxsw_sp_bridge_port *bridge_port, u16 vid)
 {
-	u16 pvid = mlxsw_sp_port->pvid == vid ? 0 : vid;
+	u16 pvid = mlxsw_sp_port->pvid == vid ? 0 : mlxsw_sp_port->pvid;
 	struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan;
 
 	mlxsw_sp_port_vlan = mlxsw_sp_port_vlan_find_by_vid(mlxsw_sp_port, vid);
-- 
2.20.1

[PATCH net 11/11] selftests: forwarding: Add a test for VLAN deletion

[Ido Schimmel]

Add a VLAN on a bridge port, delete it and make sure the PVID VLAN is
not affected.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/net/forwarding/bridge_vlan_aware.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh
index d8313d0438b7..04c6431b2bd8 100755
--- a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding"
+ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding vlan_deletion"
 NUM_NETIFS=4
 CHECK_TC="yes"
 source lib.sh
@@ -96,6 +96,19 @@ flooding()
 	flood_test $swp2 $h1 $h2
 }
 
+vlan_deletion()
+{
+	# Test that the deletion of a VLAN on a bridge port does not affect
+	# the PVID VLAN
+	log_info "Add and delete a VLAN on bridge port $swp1"
+
+	bridge vlan add vid 10 dev $swp1
+	bridge vlan del vid 10 dev $swp1
+
+	ping_ipv4
+	ping_ipv6
+}
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.20.1

Re: [PATCH net 00/11] mlxsw: Various fixes

[David Miller]

From: Ido Schimmel <idosch@mellanox.com>
Date: Tue, 8 Jan 2019 16:48:02 +0000

> Patches #1-#2 from Nir fix two bugs in recent ACL work. First patch adds
> a missing cleanup in error path, while second patch removes an easily
> triggerable false warning.
> 
> Patch #3 from Jiri adds a missing cleanup when unlinking a port from
> LAG.
> 
> Patches #4-#9 fix various bugs in recent VXLAN patches and add test
> cases.
> 
> Patches #10-#11 correctly set the PVID on a bridge port during VLAN
> deletion and add a test case.

Series applied.

> Please consider patches #3 and #10 for stable.

Queued up.