netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices
@ 2022-09-07  7:56 Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 1/4] net: bonding: Share lacpdu_mcast_addr definition Benjamin Poirier
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Benjamin Poirier @ 2022-09-07  7:56 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

This series fixes similar problems in the bonding and team drivers.

Because of missing dev_{uc,mc}_unsync() calls, addresses added to
underlying devices may be leftover after the aggregated device is deleted.
Add the missing calls and a few related tests.

v2:
* fix selftest installation, see patch 3

v3:
* Split lacpdu_multicast changes to their own patch, #1
* In ndo_{add,del}_slave methods, only perform address list changes when
  the aggregated device is up (patches 2 & 3)
* Add selftest function related to the above change (patch 4)

Benjamin Poirier (4):
  net: bonding: Share lacpdu_mcast_addr definition
  net: bonding: Unsync device addresses on ndo_stop
  net: team: Unsync device addresses on ndo_stop
  net: Add tests for bonding and team address list management

 MAINTAINERS                                   |   1 +
 drivers/net/bonding/bond_3ad.c                |   5 +-
 drivers/net/bonding/bond_main.c               |  57 +++++----
 drivers/net/team/team.c                       |  24 +++-
 include/net/bond_3ad.h                        |   2 -
 include/net/bonding.h                         |   3 +
 tools/testing/selftests/Makefile              |   1 +
 .../selftests/drivers/net/bonding/Makefile    |   5 +-
 .../selftests/drivers/net/bonding/config      |   1 +
 .../drivers/net/bonding/dev_addr_lists.sh     | 109 ++++++++++++++++++
 .../selftests/drivers/net/bonding/lag_lib.sh  |  61 ++++++++++
 .../selftests/drivers/net/team/Makefile       |   6 +
 .../testing/selftests/drivers/net/team/config |   3 +
 .../drivers/net/team/dev_addr_lists.sh        |  51 ++++++++
 14 files changed, 297 insertions(+), 32 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
 create mode 100644 tools/testing/selftests/drivers/net/bonding/lag_lib.sh
 create mode 100644 tools/testing/selftests/drivers/net/team/Makefile
 create mode 100644 tools/testing/selftests/drivers/net/team/config
 create mode 100755 tools/testing/selftests/drivers/net/team/dev_addr_lists.sh

-- 
2.37.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net v3 1/4] net: bonding: Share lacpdu_mcast_addr definition
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
@ 2022-09-07  7:56 ` Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 2/4] net: bonding: Unsync device addresses on ndo_stop Benjamin Poirier
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benjamin Poirier @ 2022-09-07  7:56 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

There are already a few definitions of arrays containing
MULTICAST_LACPDU_ADDR and the next patch will add one more use. These all
contain the same constant data so define one common instance for all
bonding code.

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 drivers/net/bonding/bond_3ad.c  |  5 +++--
 drivers/net/bonding/bond_main.c | 16 ++++------------
 include/net/bond_3ad.h          |  2 --
 include/net/bonding.h           |  3 +++
 4 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 184608bd8999..e58a1e0cadd2 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -88,8 +88,9 @@ static const u8 null_mac_addr[ETH_ALEN + 2] __long_aligned = {
 static const u16 ad_ticks_per_sec = 1000 / AD_TIMER_INTERVAL;
 static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
-static const u8 lacpdu_mcast_addr[ETH_ALEN + 2] __long_aligned =
-	MULTICAST_LACPDU_ADDR;
+const u8 lacpdu_mcast_addr[ETH_ALEN + 2] __long_aligned = {
+	0x01, 0x80, 0xC2, 0x00, 0x00, 0x02
+};
 
 /* ================= main 802.3ad protocol functions ================== */
 static int ad_lacpdu_send(struct port *port);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5c2febe94428..faced8ae4edd 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -865,12 +865,8 @@ static void bond_hw_addr_flush(struct net_device *bond_dev,
 	dev_uc_unsync(slave_dev, bond_dev);
 	dev_mc_unsync(slave_dev, bond_dev);
 
-	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
-		/* del lacpdu mc addr from mc list */
-		u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
-
-		dev_mc_del(slave_dev, lacpdu_multicast);
-	}
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		dev_mc_del(slave_dev, lacpdu_mcast_addr);
 }
 
 /*--------------------------- Active slave change ---------------------------*/
@@ -2171,12 +2167,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		dev_uc_sync_multiple(slave_dev, bond_dev);
 		netif_addr_unlock_bh(bond_dev);
 
-		if (BOND_MODE(bond) == BOND_MODE_8023AD) {
-			/* add lacpdu mc addr to mc list */
-			u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
-
-			dev_mc_add(slave_dev, lacpdu_multicast);
-		}
+		if (BOND_MODE(bond) == BOND_MODE_8023AD)
+			dev_mc_add(slave_dev, lacpdu_mcast_addr);
 	}
 
 	bond->slave_cnt++;
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index be2992e6de5d..a016f275cb01 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -15,8 +15,6 @@
 #define PKT_TYPE_LACPDU         cpu_to_be16(ETH_P_SLOW)
 #define AD_TIMER_INTERVAL       100 /*msec*/
 
-#define MULTICAST_LACPDU_ADDR    {0x01, 0x80, 0xC2, 0x00, 0x00, 0x02}
-
 #define AD_LACP_SLOW 0
 #define AD_LACP_FAST 1
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index afd606df149a..e999f851738b 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -786,6 +786,9 @@ extern struct rtnl_link_ops bond_link_ops;
 /* exported from bond_sysfs_slave.c */
 extern const struct sysfs_ops slave_sysfs_ops;
 
+/* exported from bond_3ad.c */
+extern const u8 lacpdu_mcast_addr[];
+
 static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *skb)
 {
 	dev_core_stats_tx_dropped_inc(dev);
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net v3 2/4] net: bonding: Unsync device addresses on ndo_stop
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 1/4] net: bonding: Share lacpdu_mcast_addr definition Benjamin Poirier
@ 2022-09-07  7:56 ` Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 3/4] net: team: " Benjamin Poirier
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benjamin Poirier @ 2022-09-07  7:56 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

Netdev drivers are expected to call dev_{uc,mc}_sync() in their
ndo_set_rx_mode method and dev_{uc,mc}_unsync() in their ndo_stop method.
This is mentioned in the kerneldoc for those dev_* functions.

The bonding driver calls dev_{uc,mc}_unsync() during ndo_uninit instead of
ndo_stop. This is ineffective because address lists (dev->{uc,mc}) have
already been emptied in unregister_netdevice_many() before ndo_uninit is
called. This mistake can result in addresses being leftover on former bond
slaves after a bond has been deleted; see test_LAG_cleanup() in the last
patch in this series.

Add unsync calls, via bond_hw_addr_flush(), at their expected location,
bond_close().
Add dev_mc_add() call to bond_open() to match the above change.

v3:
* When adding or deleting a slave, only sync/unsync, add/del addresses if
  the bond is up. In other cases, it is taken care of at the right time by
  ndo_open/ndo_set_rx_mode/ndo_stop.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 drivers/net/bonding/bond_main.c | 47 ++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index faced8ae4edd..bc6d8b0aa6fb 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -886,7 +886,8 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active,
 		if (bond->dev->flags & IFF_ALLMULTI)
 			dev_set_allmulti(old_active->dev, -1);
 
-		bond_hw_addr_flush(bond->dev, old_active->dev);
+		if (bond->dev->flags & IFF_UP)
+			bond_hw_addr_flush(bond->dev, old_active->dev);
 	}
 
 	if (new_active) {
@@ -897,10 +898,12 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active,
 		if (bond->dev->flags & IFF_ALLMULTI)
 			dev_set_allmulti(new_active->dev, 1);
 
-		netif_addr_lock_bh(bond->dev);
-		dev_uc_sync(new_active->dev, bond->dev);
-		dev_mc_sync(new_active->dev, bond->dev);
-		netif_addr_unlock_bh(bond->dev);
+		if (bond->dev->flags & IFF_UP) {
+			netif_addr_lock_bh(bond->dev);
+			dev_uc_sync(new_active->dev, bond->dev);
+			dev_mc_sync(new_active->dev, bond->dev);
+			netif_addr_unlock_bh(bond->dev);
+		}
 	}
 }
 
@@ -2162,13 +2165,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 			}
 		}
 
-		netif_addr_lock_bh(bond_dev);
-		dev_mc_sync_multiple(slave_dev, bond_dev);
-		dev_uc_sync_multiple(slave_dev, bond_dev);
-		netif_addr_unlock_bh(bond_dev);
+		if (bond_dev->flags & IFF_UP) {
+			netif_addr_lock_bh(bond_dev);
+			dev_mc_sync_multiple(slave_dev, bond_dev);
+			dev_uc_sync_multiple(slave_dev, bond_dev);
+			netif_addr_unlock_bh(bond_dev);
 
-		if (BOND_MODE(bond) == BOND_MODE_8023AD)
-			dev_mc_add(slave_dev, lacpdu_mcast_addr);
+			if (BOND_MODE(bond) == BOND_MODE_8023AD)
+				dev_mc_add(slave_dev, lacpdu_mcast_addr);
+		}
 	}
 
 	bond->slave_cnt++;
@@ -2439,7 +2444,8 @@ static int __bond_release_one(struct net_device *bond_dev,
 		if (old_flags & IFF_ALLMULTI)
 			dev_set_allmulti(slave_dev, -1);
 
-		bond_hw_addr_flush(bond_dev, slave_dev);
+		if (old_flags & IFF_UP)
+			bond_hw_addr_flush(bond_dev, slave_dev);
 	}
 
 	slave_disable_netpoll(slave);
@@ -4213,6 +4219,9 @@ static int bond_open(struct net_device *bond_dev)
 		/* register to receive LACPDUs */
 		bond->recv_probe = bond_3ad_lacpdu_recv;
 		bond_3ad_initiate_agg_selection(bond, 1);
+
+		bond_for_each_slave(bond, slave, iter)
+			dev_mc_add(slave->dev, lacpdu_mcast_addr);
 	}
 
 	if (bond_mode_can_use_xmit_hash(bond))
@@ -4224,6 +4233,7 @@ static int bond_open(struct net_device *bond_dev)
 static int bond_close(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+	struct slave *slave;
 
 	bond_work_cancel_all(bond);
 	bond->send_peer_notif = 0;
@@ -4231,6 +4241,19 @@ static int bond_close(struct net_device *bond_dev)
 		bond_alb_deinitialize(bond);
 	bond->recv_probe = NULL;
 
+	if (bond_uses_primary(bond)) {
+		rcu_read_lock();
+		slave = rcu_dereference(bond->curr_active_slave);
+		if (slave)
+			bond_hw_addr_flush(bond_dev, slave->dev);
+		rcu_read_unlock();
+	} else {
+		struct list_head *iter;
+
+		bond_for_each_slave(bond, slave, iter)
+			bond_hw_addr_flush(bond_dev, slave->dev);
+	}
+
 	return 0;
 }
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net v3 3/4] net: team: Unsync device addresses on ndo_stop
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 1/4] net: bonding: Share lacpdu_mcast_addr definition Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 2/4] net: bonding: Unsync device addresses on ndo_stop Benjamin Poirier
@ 2022-09-07  7:56 ` Benjamin Poirier
  2022-09-07  7:56 ` [PATCH net v3 4/4] net: Add tests for bonding and team address list management Benjamin Poirier
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benjamin Poirier @ 2022-09-07  7:56 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

Netdev drivers are expected to call dev_{uc,mc}_sync() in their
ndo_set_rx_mode method and dev_{uc,mc}_unsync() in their ndo_stop method.
This is mentioned in the kerneldoc for those dev_* functions.

The team driver calls dev_{uc,mc}_unsync() during ndo_uninit instead of
ndo_stop. This is ineffective because address lists (dev->{uc,mc}) have
already been emptied in unregister_netdevice_many() before ndo_uninit is
called. This mistake can result in addresses being leftover on former team
ports after a team device has been deleted; see test_LAG_cleanup() in the
last patch in this series.

Add unsync calls at their expected location, team_close().

v3:
* When adding or deleting a port, only sync/unsync addresses if the team
  device is up. In other cases, it is taken care of at the right time by
  ndo_open/ndo_set_rx_mode/ndo_stop.

Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 drivers/net/team/team.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index aac133a1e27a..154a3c0a6dfd 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1275,10 +1275,12 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 		}
 	}
 
-	netif_addr_lock_bh(dev);
-	dev_uc_sync_multiple(port_dev, dev);
-	dev_mc_sync_multiple(port_dev, dev);
-	netif_addr_unlock_bh(dev);
+	if (dev->flags & IFF_UP) {
+		netif_addr_lock_bh(dev);
+		dev_uc_sync_multiple(port_dev, dev);
+		dev_mc_sync_multiple(port_dev, dev);
+		netif_addr_unlock_bh(dev);
+	}
 
 	port->index = -1;
 	list_add_tail_rcu(&port->list, &team->port_list);
@@ -1349,8 +1351,10 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 	netdev_rx_handler_unregister(port_dev);
 	team_port_disable_netpoll(port);
 	vlan_vids_del_by_dev(port_dev, dev);
-	dev_uc_unsync(port_dev, dev);
-	dev_mc_unsync(port_dev, dev);
+	if (dev->flags & IFF_UP) {
+		dev_uc_unsync(port_dev, dev);
+		dev_mc_unsync(port_dev, dev);
+	}
 	dev_close(port_dev);
 	team_port_leave(team, port);
 
@@ -1700,6 +1704,14 @@ static int team_open(struct net_device *dev)
 
 static int team_close(struct net_device *dev)
 {
+	struct team *team = netdev_priv(dev);
+	struct team_port *port;
+
+	list_for_each_entry(port, &team->port_list, list) {
+		dev_uc_unsync(port->dev, dev);
+		dev_mc_unsync(port->dev, dev);
+	}
+
 	return 0;
 }
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net v3 4/4] net: Add tests for bonding and team address list management
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
                   ` (2 preceding siblings ...)
  2022-09-07  7:56 ` [PATCH net v3 3/4] net: team: " Benjamin Poirier
@ 2022-09-07  7:56 ` Benjamin Poirier
  2022-09-08 23:25 ` [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Jay Vosburgh
  2022-09-16 13:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Benjamin Poirier @ 2022-09-07  7:56 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

Test that the bonding and team drivers clean up an underlying device's
address lists (dev->uc, dev->mc) when the aggregated device is deleted.

Test addition and removal of the LACPDU multicast address on underlying
devices by the bonding driver.

v2:
* add lag_lib.sh to TEST_FILES

v3:
* extend bond_listen_lacpdu_multicast test to init_state up and down cases
* remove some superfluous shell syntax and 'set dev ... up' commands

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 MAINTAINERS                                   |   1 +
 tools/testing/selftests/Makefile              |   1 +
 .../selftests/drivers/net/bonding/Makefile    |   5 +-
 .../selftests/drivers/net/bonding/config      |   1 +
 .../drivers/net/bonding/dev_addr_lists.sh     | 109 ++++++++++++++++++
 .../selftests/drivers/net/bonding/lag_lib.sh  |  61 ++++++++++
 .../selftests/drivers/net/team/Makefile       |   6 +
 .../testing/selftests/drivers/net/team/config |   3 +
 .../drivers/net/team/dev_addr_lists.sh        |  51 ++++++++
 9 files changed, 237 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
 create mode 100644 tools/testing/selftests/drivers/net/bonding/lag_lib.sh
 create mode 100644 tools/testing/selftests/drivers/net/team/Makefile
 create mode 100644 tools/testing/selftests/drivers/net/team/config
 create mode 100755 tools/testing/selftests/drivers/net/team/dev_addr_lists.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 589517372408..4194f44e7bb9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19937,6 +19937,7 @@ S:	Supported
 F:	drivers/net/team/
 F:	include/linux/if_team.h
 F:	include/uapi/linux/if_team.h
+F:	tools/testing/selftests/net/team/
 
 TECHNOLOGIC SYSTEMS TS-5500 PLATFORM SUPPORT
 M:	"Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index c2064a35688b..1fc89b8ef433 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -13,6 +13,7 @@ TARGETS += damon
 TARGETS += drivers/dma-buf
 TARGETS += drivers/s390x/uvdevice
 TARGETS += drivers/net/bonding
+TARGETS += drivers/net/team
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += filesystems
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index ab6c54b12098..0f9659407969 100644
--- a/tools/testing/selftests/drivers/net/bonding/Makefile
+++ b/tools/testing/selftests/drivers/net/bonding/Makefile
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for net selftests
 
-TEST_PROGS := bond-break-lacpdu-tx.sh
+TEST_PROGS := bond-break-lacpdu-tx.sh \
+	      dev_addr_lists.sh
+
+TEST_FILES := lag_lib.sh
 
 include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/bonding/config b/tools/testing/selftests/drivers/net/bonding/config
index dc1c22de3c92..70638fa50b2c 100644
--- a/tools/testing/selftests/drivers/net/bonding/config
+++ b/tools/testing/selftests/drivers/net/bonding/config
@@ -1 +1,2 @@
 CONFIG_BONDING=y
+CONFIG_MACVLAN=y
diff --git a/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh b/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
new file mode 100755
index 000000000000..e6fa24eded5b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
@@ -0,0 +1,109 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test bond device handling of addr lists (dev->uc, mc)
+#
+
+ALL_TESTS="
+	bond_cleanup_mode1
+	bond_cleanup_mode4
+	bond_listen_lacpdu_multicast_case_down
+	bond_listen_lacpdu_multicast_case_up
+"
+
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "$lib_dir"/../../../net/forwarding/lib.sh
+
+source "$lib_dir"/lag_lib.sh
+
+
+destroy()
+{
+	local ifnames=(dummy1 dummy2 bond1 mv0)
+	local ifname
+
+	for ifname in "${ifnames[@]}"; do
+		ip link del "$ifname" &>/dev/null
+	done
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	destroy
+}
+
+
+# bond driver control paths vary between modes that have a primary slave
+# (bond_uses_primary()) and others. Test both kinds of modes.
+
+bond_cleanup_mode1()
+{
+	RET=0
+
+	test_LAG_cleanup "bonding" "active-backup"
+}
+
+bond_cleanup_mode4() {
+	RET=0
+
+	test_LAG_cleanup "bonding" "802.3ad"
+}
+
+bond_listen_lacpdu_multicast()
+{
+	# Initial state of bond device, up | down
+	local init_state=$1
+	local lacpdu_mc="01:80:c2:00:00:02"
+
+	ip link add dummy1 type dummy
+	ip link add bond1 "$init_state" type bond mode 802.3ad
+	ip link set dev dummy1 master bond1
+	if [ "$init_state" = "down" ]; then
+		ip link set dev bond1 up
+	fi
+
+	grep_bridge_fdb "$lacpdu_mc" bridge fdb show brport dummy1 >/dev/null
+	check_err $? "LACPDU multicast address not present on slave (1)"
+
+	ip link set dev bond1 down
+
+	not grep_bridge_fdb "$lacpdu_mc" bridge fdb show brport dummy1 >/dev/null
+	check_err $? "LACPDU multicast address still present on slave"
+
+	ip link set dev bond1 up
+
+	grep_bridge_fdb "$lacpdu_mc" bridge fdb show brport dummy1 >/dev/null
+	check_err $? "LACPDU multicast address not present on slave (2)"
+
+	cleanup
+
+	log_test "bonding LACPDU multicast address to slave (from bond $init_state)"
+}
+
+# The LACPDU mc addr is added by different paths depending on the initial state
+# of the bond when enslaving a device. Test both cases.
+
+bond_listen_lacpdu_multicast_case_down()
+{
+	RET=0
+
+	bond_listen_lacpdu_multicast "down"
+}
+
+bond_listen_lacpdu_multicast_case_up()
+{
+	RET=0
+
+	bond_listen_lacpdu_multicast "up"
+}
+
+
+trap cleanup EXIT
+
+tests_run
+
+exit "$EXIT_STATUS"
diff --git a/tools/testing/selftests/drivers/net/bonding/lag_lib.sh b/tools/testing/selftests/drivers/net/bonding/lag_lib.sh
new file mode 100644
index 000000000000..16c7fb858ac1
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bonding/lag_lib.sh
@@ -0,0 +1,61 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test that a link aggregation device (bonding, team) removes the hardware
+# addresses that it adds on its underlying devices.
+test_LAG_cleanup()
+{
+	local driver=$1
+	local mode=$2
+	local ucaddr="02:00:00:12:34:56"
+	local addr6="fe80::78:9abc/64"
+	local mcaddr="33:33:ff:78:9a:bc"
+	local name
+
+	ip link add dummy1 type dummy
+	ip link add dummy2 type dummy
+	if [ "$driver" = "bonding" ]; then
+		name="bond1"
+		ip link add "$name" up type bond mode "$mode"
+		ip link set dev dummy1 master "$name"
+		ip link set dev dummy2 master "$name"
+	elif [ "$driver" = "team" ]; then
+		name="team0"
+		teamd -d -c '
+			{
+				"device": "'"$name"'",
+				"runner": {
+					"name": "'"$mode"'"
+				},
+				"ports": {
+					"dummy1":
+						{},
+					"dummy2":
+						{}
+				}
+			}
+		'
+		ip link set dev "$name" up
+	else
+		check_err 1
+		log_test test_LAG_cleanup ": unknown driver \"$driver\""
+		return
+	fi
+
+	# Used to test dev->uc handling
+	ip link add mv0 link "$name" up address "$ucaddr" type macvlan
+	# Used to test dev->mc handling
+	ip address add "$addr6" dev "$name"
+	ip link set dev "$name" down
+	ip link del "$name"
+
+	not grep_bridge_fdb "$ucaddr" bridge fdb show >/dev/null
+	check_err $? "macvlan unicast address still present on a slave"
+
+	not grep_bridge_fdb "$mcaddr" bridge fdb show >/dev/null
+	check_err $? "IPv6 solicited-node multicast mac address still present on a slave"
+
+	cleanup
+
+	log_test "$driver cleanup mode $mode"
+}
diff --git a/tools/testing/selftests/drivers/net/team/Makefile b/tools/testing/selftests/drivers/net/team/Makefile
new file mode 100644
index 000000000000..642d8df1c137
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/team/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for net selftests
+
+TEST_PROGS := dev_addr_lists.sh
+
+include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/team/config b/tools/testing/selftests/drivers/net/team/config
new file mode 100644
index 000000000000..265b6882cc21
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/team/config
@@ -0,0 +1,3 @@
+CONFIG_NET_TEAM=y
+CONFIG_NET_TEAM_MODE_LOADBALANCE=y
+CONFIG_MACVLAN=y
diff --git a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
new file mode 100755
index 000000000000..debda7262956
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
@@ -0,0 +1,51 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test team device handling of addr lists (dev->uc, mc)
+#
+
+ALL_TESTS="
+	team_cleanup
+"
+
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "$lib_dir"/../../../net/forwarding/lib.sh
+
+source "$lib_dir"/../bonding/lag_lib.sh
+
+
+destroy()
+{
+	local ifnames=(dummy0 dummy1 team0 mv0)
+	local ifname
+
+	for ifname in "${ifnames[@]}"; do
+		ip link del "$ifname" &>/dev/null
+	done
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	destroy
+}
+
+
+team_cleanup()
+{
+	RET=0
+
+	test_LAG_cleanup "team" "lacp"
+}
+
+
+require_command teamd
+
+trap cleanup EXIT
+
+tests_run
+
+exit "$EXIT_STATUS"
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
                   ` (3 preceding siblings ...)
  2022-09-07  7:56 ` [PATCH net v3 4/4] net: Add tests for bonding and team address list management Benjamin Poirier
@ 2022-09-08 23:25 ` Jay Vosburgh
  2022-09-16 13:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Jay Vosburgh @ 2022-09-08 23:25 UTC (permalink / raw)
  To: Benjamin Poirier
  Cc: netdev, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jiri Pirko, Shuah Khan,
	Jonathan Toppins, linux-kselftest

Benjamin Poirier <bpoirier@nvidia.com> wrote:

>This series fixes similar problems in the bonding and team drivers.
>
>Because of missing dev_{uc,mc}_unsync() calls, addresses added to
>underlying devices may be leftover after the aggregated device is deleted.
>Add the missing calls and a few related tests.

	I'm not seeing any gaps in the logic; so, for the bonding parts
of the series

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

	-J


>v2:
>* fix selftest installation, see patch 3
>
>v3:
>* Split lacpdu_multicast changes to their own patch, #1
>* In ndo_{add,del}_slave methods, only perform address list changes when
>  the aggregated device is up (patches 2 & 3)
>* Add selftest function related to the above change (patch 4)
>
>Benjamin Poirier (4):
>  net: bonding: Share lacpdu_mcast_addr definition
>  net: bonding: Unsync device addresses on ndo_stop
>  net: team: Unsync device addresses on ndo_stop
>  net: Add tests for bonding and team address list management
>
> MAINTAINERS                                   |   1 +
> drivers/net/bonding/bond_3ad.c                |   5 +-
> drivers/net/bonding/bond_main.c               |  57 +++++----
> drivers/net/team/team.c                       |  24 +++-
> include/net/bond_3ad.h                        |   2 -
> include/net/bonding.h                         |   3 +
> tools/testing/selftests/Makefile              |   1 +
> .../selftests/drivers/net/bonding/Makefile    |   5 +-
> .../selftests/drivers/net/bonding/config      |   1 +
> .../drivers/net/bonding/dev_addr_lists.sh     | 109 ++++++++++++++++++
> .../selftests/drivers/net/bonding/lag_lib.sh  |  61 ++++++++++
> .../selftests/drivers/net/team/Makefile       |   6 +
> .../testing/selftests/drivers/net/team/config |   3 +
> .../drivers/net/team/dev_addr_lists.sh        |  51 ++++++++
> 14 files changed, 297 insertions(+), 32 deletions(-)
> create mode 100755 tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
> create mode 100644 tools/testing/selftests/drivers/net/bonding/lag_lib.sh
> create mode 100644 tools/testing/selftests/drivers/net/team/Makefile
> create mode 100644 tools/testing/selftests/drivers/net/team/config
> create mode 100755 tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
>
>-- 
>2.37.2
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices
  2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
                   ` (4 preceding siblings ...)
  2022-09-08 23:25 ` [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Jay Vosburgh
@ 2022-09-16 13:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-09-16 13:40 UTC (permalink / raw)
  To: Benjamin Poirier
  Cc: netdev, j.vosburgh, vfalico, andy, davem, edumazet, kuba, pabeni,
	jiri, shuah, jtoppins, linux-kselftest

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed,  7 Sep 2022 16:56:38 +0900 you wrote:
> This series fixes similar problems in the bonding and team drivers.
> 
> Because of missing dev_{uc,mc}_unsync() calls, addresses added to
> underlying devices may be leftover after the aggregated device is deleted.
> Add the missing calls and a few related tests.
> 
> v2:
> * fix selftest installation, see patch 3
> 
> [...]

Here is the summary with links:
  - [net,v3,1/4] net: bonding: Share lacpdu_mcast_addr definition
    https://git.kernel.org/netdev/net/c/1d9a143ee340
  - [net,v3,2/4] net: bonding: Unsync device addresses on ndo_stop
    https://git.kernel.org/netdev/net/c/86247aba599e
  - [net,v3,3/4] net: team: Unsync device addresses on ndo_stop
    https://git.kernel.org/netdev/net/c/bd60234222b2
  - [net,v3,4/4] net: Add tests for bonding and team address list management
    https://git.kernel.org/netdev/net/c/bbb774d921e2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-09-16 13:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-07  7:56 [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Benjamin Poirier
2022-09-07  7:56 ` [PATCH net v3 1/4] net: bonding: Share lacpdu_mcast_addr definition Benjamin Poirier
2022-09-07  7:56 ` [PATCH net v3 2/4] net: bonding: Unsync device addresses on ndo_stop Benjamin Poirier
2022-09-07  7:56 ` [PATCH net v3 3/4] net: team: " Benjamin Poirier
2022-09-07  7:56 ` [PATCH net v3 4/4] net: Add tests for bonding and team address list management Benjamin Poirier
2022-09-08 23:25 ` [PATCH net v3 0/4] Unsync addresses from ports when stopping aggregated devices Jay Vosburgh
2022-09-16 13:40 ` patchwork-bot+netdevbpf

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).