[PATCHv3 net-next] Bonding: add arp_missed_max option

[PATCHv3 net-next] Bonding: add arp_missed_max option

[Hangbin Liu]

Currently, we use hard code number to verify if we are in the
arp_interval timeslice. But some user may want to reduce/extend
the verify timeslice. With the similar team option 'missed_max'
the uers could change that number based on their own environment.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

---
v2: set IFLA_BOND_MISSED_MAX to NLA_U8, and limit the values to 1-255
v3: rename the option name to arp_missed_max
---
 Documentation/networking/bonding.rst | 10 ++++++++++
 drivers/net/bonding/bond_main.c      | 17 +++++++++--------
 drivers/net/bonding/bond_netlink.c   | 15 +++++++++++++++
 drivers/net/bonding/bond_options.c   | 28 ++++++++++++++++++++++++++++
 drivers/net/bonding/bond_procfs.c    |  2 ++
 drivers/net/bonding/bond_sysfs.c     | 13 +++++++++++++
 include/net/bond_options.h           |  1 +
 include/net/bonding.h                |  1 +
 include/uapi/linux/if_link.h         |  1 +
 tools/include/uapi/linux/if_link.h   |  1 +
 10 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index 31cfd7d674a6..dc28c9551b9b 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -421,6 +421,16 @@ arp_all_targets
 		consider the slave up only when all of the arp_ip_targets
 		are reachable
 
+arp_missed_max
+
+	Maximum number of arp_interval monitor cycle for missed ARP replies.
+	If this number is exceeded, link is reported as down.
+
+	Normally 2 monitor cycles are needed. One cycle for missed ARP request
+	and one cycle for waiting ARP reply.
+
+	The valid range is 1 - 255; the default value is 2.
+
 downdelay
 
 	Specifies the time, in milliseconds, to wait before disabling
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ff8da720a33a..9a28d3de798e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3129,8 +3129,8 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
 			 * when the source ip is 0, so don't take the link down
 			 * if we don't know our ip yet
 			 */
-			if (!bond_time_in_interval(bond, trans_start, 2) ||
-			    !bond_time_in_interval(bond, slave->last_rx, 2)) {
+			if (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
+			    !bond_time_in_interval(bond, slave->last_rx, bond->params.missed_max)) {
 
 				bond_propose_link_state(slave, BOND_LINK_DOWN);
 				slave_state_changed = 1;
@@ -3224,7 +3224,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
 
 		/* Backup slave is down if:
 		 * - No current_arp_slave AND
-		 * - more than 3*delta since last receive AND
+		 * - more than (missed_max+1)*delta since last receive AND
 		 * - the bond has an IP address
 		 *
 		 * Note: a non-null current_arp_slave indicates
@@ -3236,20 +3236,20 @@ static int bond_ab_arp_inspect(struct bonding *bond)
 		 */
 		if (!bond_is_active_slave(slave) &&
 		    !rcu_access_pointer(bond->current_arp_slave) &&
-		    !bond_time_in_interval(bond, last_rx, 3)) {
+		    !bond_time_in_interval(bond, last_rx, bond->params.missed_max+1)) {
 			bond_propose_link_state(slave, BOND_LINK_DOWN);
 			commit++;
 		}
 
 		/* Active slave is down if:
-		 * - more than 2*delta since transmitting OR
-		 * - (more than 2*delta since receive AND
+		 * - more than missed_max*delta since transmitting OR
+		 * - (more than missed_max*delta since receive AND
 		 *    the bond has an IP address)
 		 */
 		trans_start = dev_trans_start(slave->dev);
 		if (bond_is_active_slave(slave) &&
-		    (!bond_time_in_interval(bond, trans_start, 2) ||
-		     !bond_time_in_interval(bond, last_rx, 2))) {
+		    (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
+		     !bond_time_in_interval(bond, last_rx, bond->params.missed_max))) {
 			bond_propose_link_state(slave, BOND_LINK_DOWN);
 			commit++;
 		}
@@ -5822,6 +5822,7 @@ static int bond_check_params(struct bond_params *params)
 	params->arp_interval = arp_interval;
 	params->arp_validate = arp_validate_value;
 	params->arp_all_targets = arp_all_targets_value;
+	params->missed_max = 2;
 	params->updelay = updelay;
 	params->downdelay = downdelay;
 	params->peer_notif_delay = 0;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 5d54e11d18fa..1007bf6d385d 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -110,6 +110,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
 					    .len  = ETH_ALEN },
 	[IFLA_BOND_TLB_DYNAMIC_LB]	= { .type = NLA_U8 },
 	[IFLA_BOND_PEER_NOTIF_DELAY]    = { .type = NLA_U32 },
+	[IFLA_BOND_MISSED_MAX]		= { .type = NLA_U8 },
 };
 
 static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -453,6 +454,15 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
 			return err;
 	}
 
+	if (data[IFLA_BOND_MISSED_MAX]) {
+		int missed_max = nla_get_u8(data[IFLA_BOND_MISSED_MAX]);
+
+		bond_opt_initval(&newval, missed_max);
+		err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -515,6 +525,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
 		nla_total_size(ETH_ALEN) + /* IFLA_BOND_AD_ACTOR_SYSTEM */
 		nla_total_size(sizeof(u8)) + /* IFLA_BOND_TLB_DYNAMIC_LB */
 		nla_total_size(sizeof(u32)) +	/* IFLA_BOND_PEER_NOTIF_DELAY */
+		nla_total_size(sizeof(u8)) +	/* IFLA_BOND_MISSED_MAX */
 		0;
 }
 
@@ -650,6 +661,10 @@ static int bond_fill_info(struct sk_buff *skb,
 		       bond->params.tlb_dynamic_lb))
 		goto nla_put_failure;
 
+	if (nla_put_u8(skb, IFLA_BOND_MISSED_MAX,
+		       bond->params.missed_max))
+		goto nla_put_failure;
+
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
 		struct ad_info info;
 
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index a8fde3bc458f..0f48921c4f15 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -78,6 +78,8 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 					   const struct bond_opt_value *newval);
 static int bond_option_ad_user_port_key_set(struct bonding *bond,
 					    const struct bond_opt_value *newval);
+static int bond_option_missed_max_set(struct bonding *bond,
+				      const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -213,6 +215,13 @@ static const struct bond_opt_value bond_ad_user_port_key_tbl[] = {
 	{ NULL,      -1,    0},
 };
 
+static const struct bond_opt_value bond_missed_max_tbl[] = {
+	{ "minval",	1,	BOND_VALFLAG_MIN},
+	{ "maxval",	255,	BOND_VALFLAG_MAX},
+	{ "default",	2,	BOND_VALFLAG_DEFAULT},
+	{ NULL,		-1,	0},
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -270,6 +279,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_arp_interval_set
 	},
+	[BOND_OPT_MISSED_MAX] = {
+		.id = BOND_OPT_MISSED_MAX,
+		.name = "arp_missed_max",
+		.desc = "Maximum number of missed ARP interval",
+		.unsuppmodes = BIT(BOND_MODE_8023AD) | BIT(BOND_MODE_TLB) |
+			       BIT(BOND_MODE_ALB),
+		.values = bond_missed_max_tbl,
+		.set = bond_option_missed_max_set
+	},
 	[BOND_OPT_ARP_TARGETS] = {
 		.id = BOND_OPT_ARP_TARGETS,
 		.name = "arp_ip_target",
@@ -1186,6 +1204,16 @@ static int bond_option_arp_all_targets_set(struct bonding *bond,
 	return 0;
 }
 
+static int bond_option_missed_max_set(struct bonding *bond,
+				      const struct bond_opt_value *newval)
+{
+	netdev_dbg(bond->dev, "Setting missed max to %s (%llu)\n",
+		   newval->string, newval->value);
+	bond->params.missed_max = newval->value;
+
+	return 0;
+}
+
 static int bond_option_primary_set(struct bonding *bond,
 				   const struct bond_opt_value *newval)
 {
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index f3e3bfd72556..2ec11af5f0cc 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -115,6 +115,8 @@ static void bond_info_show_master(struct seq_file *seq)
 
 		seq_printf(seq, "ARP Polling Interval (ms): %d\n",
 				bond->params.arp_interval);
+		seq_printf(seq, "ARP Missed Max: %u\n",
+				bond->params.missed_max);
 
 		seq_printf(seq, "ARP IP target/s (n.n.n.n form):");
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index c48b77167fab..9b5a5df23d21 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -303,6 +303,18 @@ static ssize_t bonding_show_arp_targets(struct device *d,
 static DEVICE_ATTR(arp_ip_target, 0644,
 		   bonding_show_arp_targets, bonding_sysfs_store_option);
 
+/* Show the arp missed max. */
+static ssize_t bonding_show_missed_max(struct device *d,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	return sprintf(buf, "%u\n", bond->params.missed_max);
+}
+static DEVICE_ATTR(arp_missed_max, 0644,
+		   bonding_show_missed_max, bonding_sysfs_store_option);
+
 /* Show the up and down delays. */
 static ssize_t bonding_show_downdelay(struct device *d,
 				      struct device_attribute *attr,
@@ -779,6 +791,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_ad_actor_sys_prio.attr,
 	&dev_attr_ad_actor_system.attr,
 	&dev_attr_ad_user_port_key.attr,
+	&dev_attr_arp_missed_max.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index e64833a674eb..dd75c071f67e 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -65,6 +65,7 @@ enum {
 	BOND_OPT_NUM_PEER_NOTIF_ALIAS,
 	BOND_OPT_PEER_NOTIF_DELAY,
 	BOND_OPT_LACP_ACTIVE,
+	BOND_OPT_MISSED_MAX,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 15e083e18f75..f6ae3a4baea4 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -121,6 +121,7 @@ struct bond_params {
 	int xmit_policy;
 	int miimon;
 	u8 num_peer_notif;
+	u8 missed_max;
 	int arp_interval;
 	int arp_validate;
 	int arp_all_targets;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index eebd3894fe89..4ac53b30b6dc 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -858,6 +858,7 @@ enum {
 	IFLA_BOND_TLB_DYNAMIC_LB,
 	IFLA_BOND_PEER_NOTIF_DELAY,
 	IFLA_BOND_AD_LACP_ACTIVE,
+	IFLA_BOND_MISSED_MAX,
 	__IFLA_BOND_MAX,
 };
 
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index b3610fdd1fee..4772a115231a 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -655,6 +655,7 @@ enum {
 	IFLA_BOND_TLB_DYNAMIC_LB,
 	IFLA_BOND_PEER_NOTIF_DELAY,
 	IFLA_BOND_AD_LACP_ACTIVE,
+	IFLA_BOND_MISSED_MAX,
 	__IFLA_BOND_MAX,
 };
 
-- 
2.31.1

[PATCHv3 iproute2-next] bond: add arp_missed_max option

[Hangbin Liu]

Bond arp_missed_max is the maximum number of arp_interval monitor cycle
for missed ARP replies. If this number is exceeded, link is reported as
down.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

---
v2: use u8 for missed_max
v3: rename the option name to arp_missed_max
---
 ip/iplink_bond.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 59c9e36d..2bfdf82f 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -153,6 +153,7 @@ static void print_explain(FILE *f)
 		"                [ ad_user_port_key PORTKEY ]\n"
 		"                [ ad_actor_sys_prio SYSPRIO ]\n"
 		"                [ ad_actor_system LLADDR ]\n"
+		"                [ arp_missed_max MISSED_MAX ]\n"
 		"\n"
 		"BONDMODE := balance-rr|active-backup|balance-xor|broadcast|802.3ad|balance-tlb|balance-alb\n"
 		"ARP_VALIDATE := none|active|backup|all|filter|filter_active|filter_backup\n"
@@ -181,6 +182,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 	__u32 miimon, updelay, downdelay, peer_notify_delay, arp_interval, arp_validate;
 	__u32 arp_all_targets, resend_igmp, min_links, lp_interval;
 	__u32 packets_per_slave;
+	__u8 missed_max;
 	unsigned int ifindex;
 
 	while (argc > 0) {
@@ -258,6 +260,12 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 				invarg("invalid arp_all_targets", *argv);
 			arp_all_targets = get_index(arp_all_targets_tbl, *argv);
 			addattr32(n, 1024, IFLA_BOND_ARP_ALL_TARGETS, arp_all_targets);
+		} else if (strcmp(*argv, "arp_missed_max") == 0) {
+			NEXT_ARG();
+			if (get_u8(&missed_max, *argv, 0))
+				invarg("invalid arp_missed_max", *argv);
+
+			addattr8(n, 1024, IFLA_BOND_MISSED_MAX, missed_max);
 		} else if (matches(*argv, "primary") == 0) {
 			NEXT_ARG();
 			ifindex = ll_name_to_index(*argv);
@@ -453,6 +461,12 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "arp_interval %u ",
 			   rta_getattr_u32(tb[IFLA_BOND_ARP_INTERVAL]));
 
+	if (tb[IFLA_BOND_MISSED_MAX])
+		print_uint(PRINT_ANY,
+			   "arp_missed_max",
+			   "arp_missed_max %u ",
+			   rta_getattr_u8(tb[IFLA_BOND_MISSED_MAX]));
+
 	if (tb[IFLA_BOND_ARP_IP_TARGET]) {
 		struct rtattr *iptb[BOND_MAX_ARP_TARGETS + 1];
 		int i;
-- 
2.31.1

Re: [PATCHv3 net-next] Bonding: add arp_missed_max option

[Jakub Kicinski]

On Tue, 23 Nov 2021 18:18:53 +0800 Hangbin Liu wrote:
> Currently, we use hard code number to verify if we are in the
> arp_interval timeslice. But some user may want to reduce/extend
> the verify timeslice. With the similar team option 'missed_max'
> the uers could change that number based on their own environment.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> ---
> v2: set IFLA_BOND_MISSED_MAX to NLA_U8, and limit the values to 1-255
> v3: rename the option name to arp_missed_max

Jay, looks good now?

Re: [PATCHv3 net-next] Bonding: add arp_missed_max option

[Jay Vosburgh]

Hangbin Liu <liuhangbin@gmail.com> wrote:

>Currently, we use hard code number to verify if we are in the
>arp_interval timeslice. But some user may want to reduce/extend
>the verify timeslice. With the similar team option 'missed_max'
>the uers could change that number based on their own environment.
>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
>---
>v2: set IFLA_BOND_MISSED_MAX to NLA_U8, and limit the values to 1-255
>v3: rename the option name to arp_missed_max
>---
> Documentation/networking/bonding.rst | 10 ++++++++++
> drivers/net/bonding/bond_main.c      | 17 +++++++++--------
> drivers/net/bonding/bond_netlink.c   | 15 +++++++++++++++
> drivers/net/bonding/bond_options.c   | 28 ++++++++++++++++++++++++++++
> drivers/net/bonding/bond_procfs.c    |  2 ++
> drivers/net/bonding/bond_sysfs.c     | 13 +++++++++++++
> include/net/bond_options.h           |  1 +
> include/net/bonding.h                |  1 +
> include/uapi/linux/if_link.h         |  1 +
> tools/include/uapi/linux/if_link.h   |  1 +
> 10 files changed, 81 insertions(+), 8 deletions(-)
>
>diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>index 31cfd7d674a6..dc28c9551b9b 100644
>--- a/Documentation/networking/bonding.rst
>+++ b/Documentation/networking/bonding.rst
>@@ -421,6 +421,16 @@ arp_all_targets
> 		consider the slave up only when all of the arp_ip_targets
> 		are reachable
> 
>+arp_missed_max
>+
>+	Maximum number of arp_interval monitor cycle for missed ARP replies.
>+	If this number is exceeded, link is reported as down.
>+
>+	Normally 2 monitor cycles are needed. One cycle for missed ARP request
>+	and one cycle for waiting ARP reply.
>+
>+	The valid range is 1 - 255; the default value is 2.
>+

	[ Apologies for the delay in responding, I was out for the US
holiday last week. ]

	For the documentation here, since deleted code commentary from
many years ago came up in discussion (re: backup interfaces get one more
cycle), I'd suggest we rewrite the above as:

arp_missed_max

	Specifies the number of arp_interval monitor checks that must
	fail in order for an interface to be marked down by the ARP
	monitor.

	In order to provide orderly failover semantics, backup
	interfaces are permitted an extra monitor check (i.e., they must
	fail arp_missed_max + 1 times before being marked down).

	The default value is 2, and the allowable range is 1 - 255.
	

	With the above caveat,

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

	-J


> downdelay
> 
> 	Specifies the time, in milliseconds, to wait before disabling
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ff8da720a33a..9a28d3de798e 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3129,8 +3129,8 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> 			 * when the source ip is 0, so don't take the link down
> 			 * if we don't know our ip yet
> 			 */
>-			if (!bond_time_in_interval(bond, trans_start, 2) ||
>-			    !bond_time_in_interval(bond, slave->last_rx, 2)) {
>+			if (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
>+			    !bond_time_in_interval(bond, slave->last_rx, bond->params.missed_max)) {
> 
> 				bond_propose_link_state(slave, BOND_LINK_DOWN);
> 				slave_state_changed = 1;
>@@ -3224,7 +3224,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
> 
> 		/* Backup slave is down if:
> 		 * - No current_arp_slave AND
>-		 * - more than 3*delta since last receive AND
>+		 * - more than (missed_max+1)*delta since last receive AND
> 		 * - the bond has an IP address
> 		 *
> 		 * Note: a non-null current_arp_slave indicates
>@@ -3236,20 +3236,20 @@ static int bond_ab_arp_inspect(struct bonding *bond)
> 		 */
> 		if (!bond_is_active_slave(slave) &&
> 		    !rcu_access_pointer(bond->current_arp_slave) &&
>-		    !bond_time_in_interval(bond, last_rx, 3)) {
>+		    !bond_time_in_interval(bond, last_rx, bond->params.missed_max+1)) {
> 			bond_propose_link_state(slave, BOND_LINK_DOWN);
> 			commit++;
> 		}
> 
> 		/* Active slave is down if:
>-		 * - more than 2*delta since transmitting OR
>-		 * - (more than 2*delta since receive AND
>+		 * - more than missed_max*delta since transmitting OR
>+		 * - (more than missed_max*delta since receive AND
> 		 *    the bond has an IP address)
> 		 */
> 		trans_start = dev_trans_start(slave->dev);
> 		if (bond_is_active_slave(slave) &&
>-		    (!bond_time_in_interval(bond, trans_start, 2) ||
>-		     !bond_time_in_interval(bond, last_rx, 2))) {
>+		    (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
>+		     !bond_time_in_interval(bond, last_rx, bond->params.missed_max))) {
> 			bond_propose_link_state(slave, BOND_LINK_DOWN);
> 			commit++;
> 		}
>@@ -5822,6 +5822,7 @@ static int bond_check_params(struct bond_params *params)
> 	params->arp_interval = arp_interval;
> 	params->arp_validate = arp_validate_value;
> 	params->arp_all_targets = arp_all_targets_value;
>+	params->missed_max = 2;
> 	params->updelay = updelay;
> 	params->downdelay = downdelay;
> 	params->peer_notif_delay = 0;
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 5d54e11d18fa..1007bf6d385d 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -110,6 +110,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> 					    .len  = ETH_ALEN },
> 	[IFLA_BOND_TLB_DYNAMIC_LB]	= { .type = NLA_U8 },
> 	[IFLA_BOND_PEER_NOTIF_DELAY]    = { .type = NLA_U32 },
>+	[IFLA_BOND_MISSED_MAX]		= { .type = NLA_U8 },
> };
> 
> static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
>@@ -453,6 +454,15 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> 			return err;
> 	}
> 
>+	if (data[IFLA_BOND_MISSED_MAX]) {
>+		int missed_max = nla_get_u8(data[IFLA_BOND_MISSED_MAX]);
>+
>+		bond_opt_initval(&newval, missed_max);
>+		err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval);
>+		if (err)
>+			return err;
>+	}
>+
> 	return 0;
> }
> 
>@@ -515,6 +525,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> 		nla_total_size(ETH_ALEN) + /* IFLA_BOND_AD_ACTOR_SYSTEM */
> 		nla_total_size(sizeof(u8)) + /* IFLA_BOND_TLB_DYNAMIC_LB */
> 		nla_total_size(sizeof(u32)) +	/* IFLA_BOND_PEER_NOTIF_DELAY */
>+		nla_total_size(sizeof(u8)) +	/* IFLA_BOND_MISSED_MAX */
> 		0;
> }
> 
>@@ -650,6 +661,10 @@ static int bond_fill_info(struct sk_buff *skb,
> 		       bond->params.tlb_dynamic_lb))
> 		goto nla_put_failure;
> 
>+	if (nla_put_u8(skb, IFLA_BOND_MISSED_MAX,
>+		       bond->params.missed_max))
>+		goto nla_put_failure;
>+
> 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> 		struct ad_info info;
> 
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index a8fde3bc458f..0f48921c4f15 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -78,6 +78,8 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
> 					   const struct bond_opt_value *newval);
> static int bond_option_ad_user_port_key_set(struct bonding *bond,
> 					    const struct bond_opt_value *newval);
>+static int bond_option_missed_max_set(struct bonding *bond,
>+				      const struct bond_opt_value *newval);
> 
> 
> static const struct bond_opt_value bond_mode_tbl[] = {
>@@ -213,6 +215,13 @@ static const struct bond_opt_value bond_ad_user_port_key_tbl[] = {
> 	{ NULL,      -1,    0},
> };
> 
>+static const struct bond_opt_value bond_missed_max_tbl[] = {
>+	{ "minval",	1,	BOND_VALFLAG_MIN},
>+	{ "maxval",	255,	BOND_VALFLAG_MAX},
>+	{ "default",	2,	BOND_VALFLAG_DEFAULT},
>+	{ NULL,		-1,	0},
>+};
>+
> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 	[BOND_OPT_MODE] = {
> 		.id = BOND_OPT_MODE,
>@@ -270,6 +279,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 		.values = bond_intmax_tbl,
> 		.set = bond_option_arp_interval_set
> 	},
>+	[BOND_OPT_MISSED_MAX] = {
>+		.id = BOND_OPT_MISSED_MAX,
>+		.name = "arp_missed_max",
>+		.desc = "Maximum number of missed ARP interval",
>+		.unsuppmodes = BIT(BOND_MODE_8023AD) | BIT(BOND_MODE_TLB) |
>+			       BIT(BOND_MODE_ALB),
>+		.values = bond_missed_max_tbl,
>+		.set = bond_option_missed_max_set
>+	},
> 	[BOND_OPT_ARP_TARGETS] = {
> 		.id = BOND_OPT_ARP_TARGETS,
> 		.name = "arp_ip_target",
>@@ -1186,6 +1204,16 @@ static int bond_option_arp_all_targets_set(struct bonding *bond,
> 	return 0;
> }
> 
>+static int bond_option_missed_max_set(struct bonding *bond,
>+				      const struct bond_opt_value *newval)
>+{
>+	netdev_dbg(bond->dev, "Setting missed max to %s (%llu)\n",
>+		   newval->string, newval->value);
>+	bond->params.missed_max = newval->value;
>+
>+	return 0;
>+}
>+
> static int bond_option_primary_set(struct bonding *bond,
> 				   const struct bond_opt_value *newval)
> {
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index f3e3bfd72556..2ec11af5f0cc 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -115,6 +115,8 @@ static void bond_info_show_master(struct seq_file *seq)
> 
> 		seq_printf(seq, "ARP Polling Interval (ms): %d\n",
> 				bond->params.arp_interval);
>+		seq_printf(seq, "ARP Missed Max: %u\n",
>+				bond->params.missed_max);
> 
> 		seq_printf(seq, "ARP IP target/s (n.n.n.n form):");
> 
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index c48b77167fab..9b5a5df23d21 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -303,6 +303,18 @@ static ssize_t bonding_show_arp_targets(struct device *d,
> static DEVICE_ATTR(arp_ip_target, 0644,
> 		   bonding_show_arp_targets, bonding_sysfs_store_option);
> 
>+/* Show the arp missed max. */
>+static ssize_t bonding_show_missed_max(struct device *d,
>+				       struct device_attribute *attr,
>+				       char *buf)
>+{
>+	struct bonding *bond = to_bond(d);
>+
>+	return sprintf(buf, "%u\n", bond->params.missed_max);
>+}
>+static DEVICE_ATTR(arp_missed_max, 0644,
>+		   bonding_show_missed_max, bonding_sysfs_store_option);
>+
> /* Show the up and down delays. */
> static ssize_t bonding_show_downdelay(struct device *d,
> 				      struct device_attribute *attr,
>@@ -779,6 +791,7 @@ static struct attribute *per_bond_attrs[] = {
> 	&dev_attr_ad_actor_sys_prio.attr,
> 	&dev_attr_ad_actor_system.attr,
> 	&dev_attr_ad_user_port_key.attr,
>+	&dev_attr_arp_missed_max.attr,
> 	NULL,
> };
> 
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index e64833a674eb..dd75c071f67e 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -65,6 +65,7 @@ enum {
> 	BOND_OPT_NUM_PEER_NOTIF_ALIAS,
> 	BOND_OPT_PEER_NOTIF_DELAY,
> 	BOND_OPT_LACP_ACTIVE,
>+	BOND_OPT_MISSED_MAX,
> 	BOND_OPT_LAST
> };
> 
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 15e083e18f75..f6ae3a4baea4 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -121,6 +121,7 @@ struct bond_params {
> 	int xmit_policy;
> 	int miimon;
> 	u8 num_peer_notif;
>+	u8 missed_max;
> 	int arp_interval;
> 	int arp_validate;
> 	int arp_all_targets;
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index eebd3894fe89..4ac53b30b6dc 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -858,6 +858,7 @@ enum {
> 	IFLA_BOND_TLB_DYNAMIC_LB,
> 	IFLA_BOND_PEER_NOTIF_DELAY,
> 	IFLA_BOND_AD_LACP_ACTIVE,
>+	IFLA_BOND_MISSED_MAX,
> 	__IFLA_BOND_MAX,
> };
> 
>diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
>index b3610fdd1fee..4772a115231a 100644
>--- a/tools/include/uapi/linux/if_link.h
>+++ b/tools/include/uapi/linux/if_link.h
>@@ -655,6 +655,7 @@ enum {
> 	IFLA_BOND_TLB_DYNAMIC_LB,
> 	IFLA_BOND_PEER_NOTIF_DELAY,
> 	IFLA_BOND_AD_LACP_ACTIVE,
>+	IFLA_BOND_MISSED_MAX,
> 	__IFLA_BOND_MAX,
> };
> 
>-- 
>2.31.1
>

Re: [PATCHv3 net-next] Bonding: add arp_missed_max option

[Hangbin Liu]

On Mon, Nov 29, 2021 at 10:50:01AM -0800, Jay Vosburgh wrote:
> >+arp_missed_max
> >+
> >+	Maximum number of arp_interval monitor cycle for missed ARP replies.
> >+	If this number is exceeded, link is reported as down.
> >+
> >+	Normally 2 monitor cycles are needed. One cycle for missed ARP request
> >+	and one cycle for waiting ARP reply.
> >+
> >+	The valid range is 1 - 255; the default value is 2.
> >+
> 
> 	[ Apologies for the delay in responding, I was out for the US
> holiday last week. ]
> 
> 	For the documentation here, since deleted code commentary from
> many years ago came up in discussion (re: backup interfaces get one more
> cycle), I'd suggest we rewrite the above as:
> 
> arp_missed_max
> 
> 	Specifies the number of arp_interval monitor checks that must
> 	fail in order for an interface to be marked down by the ARP
> 	monitor.
> 
> 	In order to provide orderly failover semantics, backup
> 	interfaces are permitted an extra monitor check (i.e., they must
> 	fail arp_missed_max + 1 times before being marked down).
> 
> 	The default value is 2, and the allowable range is 1 - 255.
> 	
> 
> 	With the above caveat,
> 
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

Thanks Jay for the review. Patch v4 posted.

Hangbin