Commit a5b3363d authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'bonding-overflow' 



Hangbin Liu says:

====================
bonding: fix send_peer_notif overflow

Bonding send_peer_notif was defined as u8. But the value is
num_peer_notif multiplied by peer_notif_delay, which is u8 * u32.
This would cause the send_peer_notif overflow.

Before the fix:
TEST: num_grat_arp (active-backup miimon num_grat_arp 10)           [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 20)           [ OK ]
4 garp packets sent on active slave eth1
TEST: num_grat_arp (active-backup miimon num_grat_arp 30)           [FAIL]
24 garp packets sent on active slave eth1
TEST: num_grat_arp (active-backup miimon num_grat_arp 50)           [FAIL]

After the fix:
TEST: num_grat_arp (active-backup miimon num_grat_arp 10)           [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 20)           [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 30)           [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 50)           [ OK ]
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 7c83e28f 6cbe791c

Documentation/networking/bonding.rst

+5 −4
Original line number Diff line number Diff line
@@ -776,10 +776,11 @@ peer_notif_delay 
	Specify the delay, in milliseconds, between each peer

	notification (gratuitous ARP and unsolicited IPv6 Neighbor

	Advertisement) when they are issued after a failover event.

	This delay should be a multiple of the link monitor interval

	(arp_interval or miimon, whichever is active). The default

	value is 0 which means to match the value of the link monitor

	interval.

	This delay should be a multiple of the MII link monitor interval

	(miimon).



	The valid range is 0 - 300000. The default value is 0, which means

	to match the value of the MII link monitor interval.



prio

	Slave priority. A higher number means higher priority.

drivers/net/bonding/bond_netlink.c

+6 −1
Original line number Diff line number Diff line
@@ -84,6 +84,11 @@ static int bond_fill_slave_info(struct sk_buff *skb, 
	return -EMSGSIZE;

}



/* Limit the max delay range to 300s */

static struct netlink_range_validation delay_range = {

	.max = 300000,

};



static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {

	[IFLA_BOND_MODE]		= { .type = NLA_U8 },

	[IFLA_BOND_ACTIVE_SLAVE]	= { .type = NLA_U32 },
@@ -114,7 +119,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { 
	[IFLA_BOND_AD_ACTOR_SYSTEM]	= { .type = NLA_BINARY,

					    .len  = ETH_ALEN },

	[IFLA_BOND_TLB_DYNAMIC_LB]	= { .type = NLA_U8 },

	[IFLA_BOND_PEER_NOTIF_DELAY]    = { .type = NLA_U32 },

	[IFLA_BOND_PEER_NOTIF_DELAY]    = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),

	[IFLA_BOND_MISSED_MAX]		= { .type = NLA_U8 },

	[IFLA_BOND_NS_IP6_TARGET]	= { .type = NLA_NESTED },

};

drivers/net/bonding/bond_options.c

+7 −1
Original line number Diff line number Diff line
@@ -169,6 +169,12 @@ static const struct bond_opt_value bond_num_peer_notif_tbl[] = { 
	{ NULL,      -1,  0}

};



static const struct bond_opt_value bond_peer_notif_delay_tbl[] = {

	{ "off",     0,   0},

	{ "maxval",  300000, BOND_VALFLAG_MAX},

	{ NULL,      -1,  0}

};



static const struct bond_opt_value bond_primary_reselect_tbl[] = {

	{ "always",  BOND_PRI_RESELECT_ALWAYS,  BOND_VALFLAG_DEFAULT},

	{ "better",  BOND_PRI_RESELECT_BETTER,  0},
@@ -488,7 +494,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = { 
		.id = BOND_OPT_PEER_NOTIF_DELAY,

		.name = "peer_notif_delay",

		.desc = "Delay between each peer notification on failover event, in milliseconds",

		.values = bond_intmax_tbl,

		.values = bond_peer_notif_delay_tbl,

		.set = bond_option_peer_notif_delay_set

	}

};

include/net/bonding.h

+1 −1
Original line number Diff line number Diff line
@@ -233,7 +233,7 @@ struct bonding { 
	 */

	spinlock_t mode_lock;

	spinlock_t stats_lock;

	u8	 send_peer_notif;

	u32	 send_peer_notif;

	u8       igmp_retrans;

#ifdef CONFIG_PROC_FS

	struct   proc_dir_entry *proc_entry;

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

+50 −0
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ 
ALL_TESTS="

	prio

	arp_validate

	num_grat_arp

"



REQUIRE_MZ=no
@@ -255,6 +256,55 @@ arp_validate() 
	arp_validate_ns "active-backup"

}



garp_test()

{

	local param="$1"

	local active_slave exp_num real_num i

	RET=0



	# create bond

	bond_reset "${param}"



	bond_check_connection

	[ $RET -ne 0 ] && log_test "num_grat_arp" "$retmsg"





	# Add tc rules to count GARP number

	for i in $(seq 0 2); do

		tc -n ${g_ns} filter add dev s$i ingress protocol arp pref 1 handle 101 \

			flower skip_hw arp_op request arp_sip ${s_ip4} arp_tip ${s_ip4} action pass

	done



	# Do failover

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

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



	exp_num=$(echo "${param}" | cut -f6 -d ' ')

	sleep $((exp_num + 2))



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



	# check result

	real_num=$(tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}")

	if [ "${real_num}" -ne "${exp_num}" ]; then

		echo "$real_num garp packets sent on active slave ${active_slave}"

		RET=1

	fi



	for i in $(seq 0 2); do

		tc -n ${g_ns} filter del dev s$i ingress

	done

}



num_grat_arp()

{

	local val

	for val in 10 20 30 50; do

		garp_test "mode active-backup miimon 100 num_grat_arp $val peer_notify_delay 1000"

		log_test "num_grat_arp" "active-backup miimon num_grat_arp $val"

	done

}



trap cleanup EXIT



setup_prepare