The mac address on backup slave should be convert from Solicited-Node Multicast address, not from bonding unicast target address.
v3: also fix the mac setting for slave_set_ns_maddr. (Jay) Add function description for slave_set_ns_maddr/slave_set_ns_maddrs (Jay) v2: fix patch 01's subject
Hangbin Liu (2): bonding: fix incorrect MAC address setting to receive NS messages selftests: bonding: fix incorrect mac address
drivers/net/bonding/bond_options.c | 55 ++++++++++++++++--- .../drivers/net/bonding/bond_options.sh | 4 +- 2 files changed, 49 insertions(+), 10 deletions(-)
When validation on the backup slave is enabled, we need to validate the Neighbor Solicitation (NS) messages received on the backup slave. To receive these messages, the correct destination MAC address must be added to the slave. However, the target in bonding is a unicast address, which we cannot use directly. Instead, we should first convert it to a Solicited-Node Multicast Address and then derive the corresponding MAC address.
Fix the incorrect MAC address setting on both slave_set_ns_maddr() and slave_set_ns_maddrs(). Since the two function names are similar. Add some description for the functions. Also only use one mac_addr variable in slave_set_ns_maddr() to save some code and logic.
Fixes: 8eb36164d1a6 ("bonding: add ns target multicast address to slave device") Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- drivers/net/bonding/bond_options.c | 55 +++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 327b6ecdc77e..d1b095af253b 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -1242,10 +1242,28 @@ static bool slave_can_set_ns_maddr(const struct bonding *bond, struct slave *sla slave->dev->flags & IFF_MULTICAST; }
+/** + * slave_set_ns_maddrs - add/del all NS mac addresses for slave + * @bond: bond device + * @slave: slave device + * @add: add or remove all the NS mac addresses + * + * This function tries to add or delete all the NS mac addresses on the slave + * + * Note, the IPv6 NS target address is the unicast address in Neighbor + * Solicitation (NS) message. The dest address of NS message should be + * solicited-node multicast address of the target. The dest mac of NS message + * is converted from the solicited-node multicast address. + * + * This function is called when + * * arp_validate changes + * * enslaving, releasing new slaves + */ static void slave_set_ns_maddrs(struct bonding *bond, struct slave *slave, bool add) { struct in6_addr *targets = bond->params.ns_targets; char slot_maddr[MAX_ADDR_LEN]; + struct in6_addr mcaddr; int i;
if (!slave_can_set_ns_maddr(bond, slave)) @@ -1255,7 +1273,8 @@ static void slave_set_ns_maddrs(struct bonding *bond, struct slave *slave, bool if (ipv6_addr_any(&targets[i])) break;
- if (!ndisc_mc_map(&targets[i], slot_maddr, slave->dev, 0)) { + addrconf_addr_solict_mult(&targets[i], &mcaddr); + if (!ndisc_mc_map(&mcaddr, slot_maddr, slave->dev, 0)) { if (add) dev_mc_add(slave->dev, slot_maddr); else @@ -1278,23 +1297,43 @@ void bond_slave_ns_maddrs_del(struct bonding *bond, struct slave *slave) slave_set_ns_maddrs(bond, slave, false); }
+/** + * slave_set_ns_maddr - set new NS mac address for slave + * @bond: bond device + * @slave: slave device + * @target: the new IPv6 target + * @slot: the old IPv6 target in the slot + * + * This function tries to replace the old mac address to new one on the slave. + * + * Note, the target/slot IPv6 address is the unicast address in Neighbor + * Solicitation (NS) message. The dest address of NS message should be + * solicited-node multicast address of the target. The dest mac of NS message + * is converted from the solicited-node multicast address. + * + * This function is called when + * * An IPv6 NS target is added or removed. + */ static void slave_set_ns_maddr(struct bonding *bond, struct slave *slave, struct in6_addr *target, struct in6_addr *slot) { - char target_maddr[MAX_ADDR_LEN], slot_maddr[MAX_ADDR_LEN]; + char mac_addr[MAX_ADDR_LEN]; + struct in6_addr mcast_addr;
if (!bond->params.arp_validate || !slave_can_set_ns_maddr(bond, slave)) return;
- /* remove the previous maddr from slave */ + /* remove the previous mac addr from slave */ + addrconf_addr_solict_mult(slot, &mcast_addr); if (!ipv6_addr_any(slot) && - !ndisc_mc_map(slot, slot_maddr, slave->dev, 0)) - dev_mc_del(slave->dev, slot_maddr); + !ndisc_mc_map(&mcast_addr, mac_addr, slave->dev, 0)) + dev_mc_del(slave->dev, mac_addr);
- /* add new maddr on slave if target is set */ + /* add new mac addr on slave if target is set */ + addrconf_addr_solict_mult(target, &mcast_addr); if (!ipv6_addr_any(target) && - !ndisc_mc_map(target, target_maddr, slave->dev, 0)) - dev_mc_add(slave->dev, target_maddr); + !ndisc_mc_map(&mcast_addr, mac_addr, slave->dev, 0)) + dev_mc_add(slave->dev, mac_addr); }
static void _bond_options_ns_ip6_target_set(struct bonding *bond, int slot,
The correct mac address for NS target 2001:db8::254 is 33:33:ff:00:02:54, not 33:33:00:00:02:54. The same with client maddress.
Fixes: 86fb6173d11e ("selftests: bonding: add ns multicast group testing") Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- tools/testing/selftests/drivers/net/bonding/bond_options.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_options.sh b/tools/testing/selftests/drivers/net/bonding/bond_options.sh index edc56e2cc606..7bc148889ca7 100755 --- a/tools/testing/selftests/drivers/net/bonding/bond_options.sh +++ b/tools/testing/selftests/drivers/net/bonding/bond_options.sh @@ -11,8 +11,8 @@ ALL_TESTS="
lib_dir=$(dirname "$0") source ${lib_dir}/bond_topo_3d1c.sh -c_maddr="33:33:00:00:00:10" -g_maddr="33:33:00:00:02:54" +c_maddr="33:33:ff:00:00:10" +g_maddr="33:33:ff:00:02:54"
skip_prio() {
Please hold on this patch. Our QE reported that with bare NIC, the backup NIC can't receive the NS messages even after joining the multicast MAC group. But after remove the backup NIC from bond, the NIC interface could receive the NS message.
This is weird, it looks the backup NIC dropped the NS message somewhere, even using tcpdump (the NIC will be in promisc mode) I can't capture the NS message on backup slave.
I need to debug more.
Thanks Hangbin On Fri, Feb 07, 2025 at 09:29:18AM +0000, Hangbin Liu wrote:
The mac address on backup slave should be convert from Solicited-Node Multicast address, not from bonding unicast target address.
v3: also fix the mac setting for slave_set_ns_maddr. (Jay) Add function description for slave_set_ns_maddr/slave_set_ns_maddrs (Jay) v2: fix patch 01's subject
Hangbin Liu (2): bonding: fix incorrect MAC address setting to receive NS messages selftests: bonding: fix incorrect mac address
drivers/net/bonding/bond_options.c | 55 ++++++++++++++++--- .../drivers/net/bonding/bond_options.sh | 4 +- 2 files changed, 49 insertions(+), 10 deletions(-)
-- 2.46.0
Hi Jay, On Sat, Feb 08, 2025 at 06:34:21AM +0000, Hangbin Liu wrote:
Please hold on this patch. Our QE reported that with bare NIC, the backup NIC can't receive the NS messages even after joining the multicast MAC group. But after remove the backup NIC from bond, the NIC interface could receive the NS message.
This is weird, it looks the backup NIC dropped the NS message somewhere, even using tcpdump (the NIC will be in promisc mode) I can't capture the NS message on backup slave.
I need to debug more.
After debug, I find it's a driver issue. The issue exists with ice dirver NIC. I tried with a mlx5 NIC and the patch works good for me.
So I think you can start review this patch when you have time. I will debug the ice driver later.
Thanks Hangbin
Hi Jay,
Any comments?
Thanks Hangbin On Tue, Feb 11, 2025 at 07:31:32AM +0000, Hangbin Liu wrote:
Hi Jay, On Sat, Feb 08, 2025 at 06:34:21AM +0000, Hangbin Liu wrote:
Please hold on this patch. Our QE reported that with bare NIC, the backup NIC can't receive the NS messages even after joining the multicast MAC group. But after remove the backup NIC from bond, the NIC interface could receive the NS message.
This is weird, it looks the backup NIC dropped the NS message somewhere, even using tcpdump (the NIC will be in promisc mode) I can't capture the NS message on backup slave.
I need to debug more.
After debug, I find it's a driver issue. The issue exists with ice dirver NIC. I tried with a mlx5 NIC and the patch works good for me.
So I think you can start review this patch when you have time. I will debug the ice driver later.
Thanks Hangbin
Hangbin Liu liuhangbin@gmail.com wrote:
The mac address on backup slave should be convert from Solicited-Node Multicast address, not from bonding unicast target address.
v3: also fix the mac setting for slave_set_ns_maddr. (Jay) Add function description for slave_set_ns_maddr/slave_set_ns_maddrs (Jay) v2: fix patch 01's subject
Hangbin Liu (2): bonding: fix incorrect MAC address setting to receive NS messages selftests: bonding: fix incorrect mac address
For the series:
Acked-by: Jay Vosburgh jv@jvosburgh.net
-J
drivers/net/bonding/bond_options.c | 55 ++++++++++++++++--- .../drivers/net/bonding/bond_options.sh | 4 +- 2 files changed, 49 insertions(+), 10 deletions(-)
-- 2.46.0
--- -Jay Vosburgh, jv@jvosburgh.net
On 2/7/25 11:29, Hangbin Liu wrote:
The mac address on backup slave should be convert from Solicited-Node Multicast address, not from bonding unicast target address.
v3: also fix the mac setting for slave_set_ns_maddr. (Jay) Add function description for slave_set_ns_maddr/slave_set_ns_maddrs (Jay) v2: fix patch 01's subject
Hangbin Liu (2): bonding: fix incorrect MAC address setting to receive NS messages selftests: bonding: fix incorrect mac address
drivers/net/bonding/bond_options.c | 55 ++++++++++++++++--- .../drivers/net/bonding/bond_options.sh | 4 +- 2 files changed, 49 insertions(+), 10 deletions(-)
For the set: Reviewed-by: Nikolay Aleksandrov razor@blackwall.org
linux-kselftest-mirror@lists.linaro.org