Commit f8d670b1 authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'bonding-fix-ns-targets-not-work-on-hardware-nic' 

Hangbin Liu says:

====================
bonding: fix ns targets not work on hardware NIC

The first patch fixed ns targets not work on hardware NIC when bonding
set arp_validate.

The second patch add a related selftest for bonding.

v4: Thanks Nikolay for the comments:
    use bond_slave_ns_maddrs_{add/del} with clear name
    fix comments typos
    remove _slave_set_ns_maddrs underscore directly
    update bond_option_arp_validate_set() change logic
v3: use ndisc_mc_map to convert the mcast mac address (Jay Vosburgh)
v2: only add/del mcast group on backup slaves when arp_validate is set (Jay Vosburgh)
    arp_validate doesn't support 3ad, tlb, alb. So let's only do it on ab mode.
====================

Link: https://patch.msgid.link/20241111101650.27685-1-liuhangbin@gmail.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents dc065076 86fb6173

drivers/net/bonding/bond_main.c

+15 −1
Original line number Diff line number Diff line
@@ -1008,6 +1008,8 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active, 


		if (bond->dev->flags & IFF_UP)

			bond_hw_addr_flush(bond->dev, old_active->dev);



		bond_slave_ns_maddrs_add(bond, old_active);

	}



	if (new_active) {
@@ -1024,6 +1026,8 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active, 
			dev_mc_sync(new_active->dev, bond->dev);

			netif_addr_unlock_bh(bond->dev);

		}



		bond_slave_ns_maddrs_del(bond, new_active);

	}

}
@@ -2341,6 +2345,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, 
	bond_compute_features(bond);

	bond_set_carrier(bond);



	/* Needs to be called before bond_select_active_slave(), which will

	 * remove the maddrs if the slave is selected as active slave.

	 */

	bond_slave_ns_maddrs_add(bond, new_slave);



	if (bond_uses_primary(bond)) {

		block_netpoll_tx();

		bond_select_active_slave(bond);
@@ -2350,7 +2359,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, 
	if (bond_mode_can_use_xmit_hash(bond))

		bond_update_slave_arr(bond, NULL);





	if (!slave_dev->netdev_ops->ndo_bpf ||

	    !slave_dev->netdev_ops->ndo_xdp_xmit) {

		if (bond->xdp_prog) {
@@ -2548,6 +2556,12 @@ static int __bond_release_one(struct net_device *bond_dev, 
	if (oldcurrent == slave)

		bond_change_active_slave(bond, NULL);



	/* Must be called after bond_change_active_slave () as the slave

	 * might change from an active slave to a backup slave. Then it is

	 * necessary to clear the maddrs on the backup slave.

	 */

	bond_slave_ns_maddrs_del(bond, slave);



	if (bond_is_lb(bond)) {

		/* Must be called only after the slave has been

		 * detached from the list and the curr_active_slave

drivers/net/bonding/bond_options.c

+81 −1
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
#include <linux/sched/signal.h>



#include <net/bonding.h>

#include <net/ndisc.h>



static int bond_option_active_slave_set(struct bonding *bond,

					const struct bond_opt_value *newval);
@@ -1234,6 +1235,68 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond, 
}



#if IS_ENABLED(CONFIG_IPV6)

static bool slave_can_set_ns_maddr(const struct bonding *bond, struct slave *slave)

{

	return BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP &&

	       !bond_is_active_slave(slave) &&

	       slave->dev->flags & IFF_MULTICAST;

}



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];

	int i;



	if (!slave_can_set_ns_maddr(bond, slave))

		return;



	for (i = 0; i < BOND_MAX_NS_TARGETS; i++) {

		if (ipv6_addr_any(&targets[i]))

			break;



		if (!ndisc_mc_map(&targets[i], slot_maddr, slave->dev, 0)) {

			if (add)

				dev_mc_add(slave->dev, slot_maddr);

			else

				dev_mc_del(slave->dev, slot_maddr);

		}

	}

}



void bond_slave_ns_maddrs_add(struct bonding *bond, struct slave *slave)

{

	if (!bond->params.arp_validate)

		return;

	slave_set_ns_maddrs(bond, slave, true);

}



void bond_slave_ns_maddrs_del(struct bonding *bond, struct slave *slave)

{

	if (!bond->params.arp_validate)

		return;

	slave_set_ns_maddrs(bond, slave, false);

}



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];



	if (!bond->params.arp_validate || !slave_can_set_ns_maddr(bond, slave))

		return;



	/* remove the previous maddr from slave */

	if (!ipv6_addr_any(slot) &&

	    !ndisc_mc_map(slot, slot_maddr, slave->dev, 0))

		dev_mc_del(slave->dev, slot_maddr);



	/* add new maddr on slave if target is set */

	if (!ipv6_addr_any(target) &&

	    !ndisc_mc_map(target, target_maddr, slave->dev, 0))

		dev_mc_add(slave->dev, target_maddr);

}



static void _bond_options_ns_ip6_target_set(struct bonding *bond, int slot,

					    struct in6_addr *target,

					    unsigned long last_rx)
@@ -1243,8 +1306,10 @@ static void _bond_options_ns_ip6_target_set(struct bonding *bond, int slot, 
	struct slave *slave;



	if (slot >= 0 && slot < BOND_MAX_NS_TARGETS) {

		bond_for_each_slave(bond, slave, iter)

		bond_for_each_slave(bond, slave, iter) {

			slave->target_last_arp_rx[slot] = last_rx;

			slave_set_ns_maddr(bond, slave, target, &targets[slot]);

		}

		targets[slot] = *target;

	}

}
@@ -1296,15 +1361,30 @@ static int bond_option_ns_ip6_targets_set(struct bonding *bond, 
{

	return -EPERM;

}



static void slave_set_ns_maddrs(struct bonding *bond, struct slave *slave, bool add) {}



void bond_slave_ns_maddrs_add(struct bonding *bond, struct slave *slave) {}



void bond_slave_ns_maddrs_del(struct bonding *bond, struct slave *slave) {}

#endif



static int bond_option_arp_validate_set(struct bonding *bond,

					const struct bond_opt_value *newval)

{

	bool changed = !!bond->params.arp_validate != !!newval->value;

	struct list_head *iter;

	struct slave *slave;



	netdev_dbg(bond->dev, "Setting arp_validate to %s (%llu)\n",

		   newval->string, newval->value);

	bond->params.arp_validate = newval->value;



	if (changed) {

		bond_for_each_slave(bond, slave, iter)

			slave_set_ns_maddrs(bond, slave, !!bond->params.arp_validate);

	}



	return 0;

}

include/net/bond_options.h

+2 −0
Original line number Diff line number Diff line
@@ -161,5 +161,7 @@ void bond_option_arp_ip_targets_clear(struct bonding *bond); 
#if IS_ENABLED(CONFIG_IPV6)

void bond_option_ns_ip6_targets_clear(struct bonding *bond);

#endif

void bond_slave_ns_maddrs_add(struct bonding *bond, struct slave *slave);

void bond_slave_ns_maddrs_del(struct bonding *bond, struct slave *slave);



#endif /* _NET_BOND_OPTIONS_H */

tools/testing/selftests/drivers/net/bonding/bond_options.sh

+53 −1
Original line number Diff line number Diff line
@@ -11,6 +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"



skip_prio()

{
@@ -240,6 +242,54 @@ arp_validate_test() 
	done

}



# Testing correct multicast groups are added to slaves for ns targets

arp_validate_mcast()

{

	RET=0

	local arp_valid=$(cmd_jq "ip -n ${s_ns} -j -d link show bond0" ".[].linkinfo.info_data.arp_validate")

	local active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")



	for i in $(seq 0 2); do

		maddr_list=$(ip -n ${s_ns} maddr show dev eth${i})



		# arp_valid == 0 or active_slave should not join any maddrs

		if { [ "$arp_valid" == "null" ] || [ "eth${i}" == ${active_slave} ]; } && \

			echo "$maddr_list" | grep -qE "${c_maddr}|${g_maddr}"; then

			RET=1

			check_err 1 "arp_valid $arp_valid active_slave $active_slave, eth$i has mcast group"

		# arp_valid != 0 and backup_slave should join both maddrs

		elif [ "$arp_valid" != "null" ] && [ "eth${i}" != ${active_slave} ] && \

		     ( ! echo "$maddr_list" | grep -q "${c_maddr}" || \

		       ! echo "$maddr_list" | grep -q "${m_maddr}"); then

			RET=1

			check_err 1 "arp_valid $arp_valid active_slave $active_slave, eth$i has mcast group"

		fi

	done



	# Do failover

	ip -n ${s_ns} link set ${active_slave} down

	# wait for active link change

	slowwait 2 active_slave_changed $active_slave

	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")



	for i in $(seq 0 2); do

		maddr_list=$(ip -n ${s_ns} maddr show dev eth${i})



		# arp_valid == 0 or active_slave should not join any maddrs

		if { [ "$arp_valid" == "null" ] || [ "eth${i}" == ${active_slave} ]; } && \

			echo "$maddr_list" | grep -qE "${c_maddr}|${g_maddr}"; then

			RET=1

			check_err 1 "arp_valid $arp_valid active_slave $active_slave, eth$i has mcast group"

		# arp_valid != 0 and backup_slave should join both maddrs

		elif [ "$arp_valid" != "null" ] && [ "eth${i}" != ${active_slave} ] && \

		     ( ! echo "$maddr_list" | grep -q "${c_maddr}" || \

		       ! echo "$maddr_list" | grep -q "${m_maddr}"); then

			RET=1

			check_err 1 "arp_valid $arp_valid active_slave $active_slave, eth$i has mcast group"

		fi

	done

}



arp_validate_arp()

{

	local mode=$1
@@ -261,8 +311,10 @@ arp_validate_ns() 
	fi



	for val in $(seq 0 6); do

		arp_validate_test "mode $mode arp_interval 100 ns_ip6_target ${g_ip6} arp_validate $val"

		arp_validate_test "mode $mode arp_interval 100 ns_ip6_target ${g_ip6},${c_ip6} arp_validate $val"

		log_test "arp_validate" "$mode ns_ip6_target arp_validate $val"

		arp_validate_mcast

		log_test "arp_validate" "join mcast group"

	done

}