[PATCHv2 net-next] Bonding: add missed_max option

Subsystems: bonding driver, documentation, networking drivers, networking [general], the rest

7 messages, 2 authors, 2021-11-18 · open the first message on its own page

[PATCHv2 net-next] Bonding: add missed_max option

From: Hangbin Liu <hidden>
Date: 2021-11-17 08:03:53

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.

The name of arp_misssed_max is not used as we may use this option for
Bonding IPv6 NS/NA monitor in future.

Signed-off-by: Hangbin Liu <redacted>

---
v2: set IFLA_BOND_MISSED_MAX to NLA_U8, and limit the values to 1-255
---
 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..4a28b350bb02 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
 
+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.
+
+        default 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..10ea93097737 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 = "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..04da21f17503 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(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_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..7b0bcddf9f26 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h

@@ -124,6 +124,7 @@ struct bond_params {
 	int arp_interval;
 	int arp_validate;
 	int arp_all_targets;
+	unsigned int missed_max;
 	int use_carrier;
 	int fail_over_mac;
 	int updelay;

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

[PATCHv2 iproute2-next] bond: add missed_max option

From: Hangbin Liu <hidden>
Date: 2021-11-17 08:03:56

Bond 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 <redacted>
---
v2: use u8 for 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..be5d73b1 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"
+		"                [ 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, "missed_max") == 0) {
+			NEXT_ARG();
+			if (get_u8(&missed_max, *argv, 0))
+				invarg("invalid 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,
+			   "missed_max",
+			   "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: [PATCHv2 net-next] Bonding: add missed_max option

From: Jay Vosburgh <hidden>
Date: 2021-11-17 08:40:30

Hangbin Liu [off-list ref] 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.

The name of arp_misssed_max is not used as we may use this option for
Bonding IPv6 NS/NA monitor in future.

	Why reserve "arp_missed_max" for IPv6 which doesn't use ARP?  If
the option is for the ARP monitor, then prefixing it with "arp_" would
be consistent with the other arp_* options.

	-J

quoted hunk

Signed-off-by: Hangbin Liu <redacted>

---
v2: set IFLA_BOND_MISSED_MAX to NLA_U8, and limit the values to 1-255
---
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..4a28b350bb02 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

+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.
+
+        default 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..10ea93097737 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 = "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..04da21f17503 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(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_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..7b0bcddf9f26 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h

@@ -124,6 +124,7 @@ struct bond_params {

	int arp_interval;
	int arp_validate;
	int arp_all_targets;
+	unsigned int missed_max;
	int use_carrier;
	int fail_over_mac;
	int updelay;

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

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

Re: [PATCHv2 net-next] Bonding: add missed_max option

From: Hangbin Liu <hidden>
Date: 2021-11-17 09:58:53

On Wed, Nov 17, 2021 at 08:40:25AM +0000, Jay Vosburgh wrote:

Hangbin Liu [off-list ref] wrote:

quoted

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.

The name of arp_misssed_max is not used as we may use this option for
Bonding IPv6 NS/NA monitor in future.

	Why reserve "arp_missed_max" for IPv6 which doesn't use ARP?  If
the option is for the ARP monitor, then prefixing it with "arp_" would
be consistent with the other arp_* options.

I didn't explain it clearly. I want to say:

I'm not using arp_misssed_max as the new option name because I plan to add
bonding IPv6 NS/NA monitor in future. At that time the option "missed_max"
could be used for both IPv4/IPv6 monitor.

I will update the commit description in next version.

Thanks
Hangbin

Re: [PATCHv2 net-next] Bonding: add missed_max option

From: Jay Vosburgh <hidden>
Date: 2021-11-17 16:16:53

Hangbin Liu [off-list ref] wrote:

On Wed, Nov 17, 2021 at 08:40:25AM +0000, Jay Vosburgh wrote:

quoted

Hangbin Liu [off-list ref] wrote:

quoted

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.

The name of arp_misssed_max is not used as we may use this option for
Bonding IPv6 NS/NA monitor in future.

	Why reserve "arp_missed_max" for IPv6 which doesn't use ARP?  If
the option is for the ARP monitor, then prefixing it with "arp_" would
be consistent with the other arp_* options.

I didn't explain it clearly. I want to say:

I'm not using arp_misssed_max as the new option name because I plan to add
bonding IPv6 NS/NA monitor in future. At that time the option "missed_max"
could be used for both IPv4/IPv6 monitor.

I will update the commit description in next version.

	There has been talk of adding an IPv6 NS monitor for years, but
it hasn't manifested.  I would prefer to see a consistent set of options
nomenclature in what we have here and now.  If and when an IPv6 version
is added, depending on the implementation, either the IPv6 item can be a
discrete tunable, or an alias could be added, similar to num_grat_arp /
num_unsol_na.

	-J

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

Re: [PATCHv2 net-next] Bonding: add missed_max option

From: Hangbin Liu <hidden>
Date: 2021-11-18 01:13:19

On Wed, Nov 17, 2021 at 04:16:46PM +0000, Jay Vosburgh wrote:

quoted

I didn't explain it clearly. I want to say:

I'm not using arp_misssed_max as the new option name because I plan to add
bonding IPv6 NS/NA monitor in future. At that time the option "missed_max"
could be used for both IPv4/IPv6 monitor.

I will update the commit description in next version.

	There has been talk of adding an IPv6 NS monitor for years, but
it hasn't manifested.  I would prefer to see a consistent set of options

I'm working on it now. I should send a simple draft patch in 2 weeks.

nomenclature in what we have here and now.  If and when an IPv6 version
is added, depending on the implementation, either the IPv6 item can be a
discrete tunable, or an alias could be added, similar to num_grat_arp /
num_unsol_na.

The name of num_grat_arp looks better than missed_max :) . In my
IPv6 implementation, the function bond_ab_arp_inspect() will be reused
directly. So one name or an alias looks more reasonable.

For the alias options, do you mean to let both num_grat_arp and num_unsol_na
change a same option in bond->params?

Thanks
Hangbin

Re: [PATCHv2 net-next] Bonding: add missed_max option

From: Jay Vosburgh <hidden>
Date: 2021-11-18 14:10:25

Hangbin Liu [off-list ref] wrote:

On Wed, Nov 17, 2021 at 04:16:46PM +0000, Jay Vosburgh wrote:

quoted

I didn't explain it clearly. I want to say:

I'm not using arp_misssed_max as the new option name because I plan to add
bonding IPv6 NS/NA monitor in future. At that time the option "missed_max"
could be used for both IPv4/IPv6 monitor.

I will update the commit description in next version.

	There has been talk of adding an IPv6 NS monitor for years, but
it hasn't manifested.  I would prefer to see a consistent set of options

I'm working on it now. I should send a simple draft patch in 2 weeks.

quoted

nomenclature in what we have here and now.  If and when an IPv6 version
is added, depending on the implementation, either the IPv6 item can be a
discrete tunable, or an alias could be added, similar to num_grat_arp /
num_unsol_na.

The name of num_grat_arp looks better than missed_max :) . In my
IPv6 implementation, the function bond_ab_arp_inspect() will be reused
directly. So one name or an alias looks more reasonable.

For the alias options, do you mean to let both num_grat_arp and num_unsol_na
change a same option in bond->params?

	The current options num_grat_arp and num_unsol_na change the
same underlying setting (params->num_peer_notif).  Your new "missed_max"
functionality could have "arp_missed_max" as the option name today, and
then whenever an IPv6 version is added, a "na_missed_max" option name
could be added as an alias for the arp_missed_max option.

	-J

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help