[PATCH net-next v3 0/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[PATCH net-next v3 0/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[David Wilder]

Changes since V1:
Changed the name of struct ip_arp_target to struct bond_arp_target.
Added patch 2, 3 and 4 to the patch set.

Changes since V2
Patch 1 was updated to add a flags element to struct bond_arp_target and
I moved the definition from bonding.h to bond_options.h.

Reduced a large stack allocation.

Cleaned up declarations to use the reverse Christmas tree order.

Updated iproute changes to allow for backward compatibility. See below.
--
I have run into issues with the ns_ip6_target feature.  I am unable to get
the existing code to function with vlans. My changes have the same issue.
I found that a multicast ns (with no vlan header) is not passed by the interface
to the it's vlan siblings. Broadcast arps will be propagated to the sibling so no
issue is seen with ipv4. I will post a RFC patch with my ns_ip6_target changes
along with my test code. Let me know if you have any ideas as to the problem with
the existing code.

I don't want the issue with the ns_ip6_target feature to hold up the review
and acceptance of the ip_arp_target changes. If the ns_ip6_target issues can
be resolved they can be submitted as a separate patch set.

Thank you for your time and reviews.

Note:
The iprout2 package will also need to be updated with the following change:

@@ -242,9 +242,14 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
                                int i;
 
                                for (i = 0; target && i < BOND_MAX_ARP_TARGETS; i++) {
-                                       __u32 addr = get_addr32(target);
-
-                                       addattr32(n, 1024, i, addr);
+                                       inet_prefix ipaddr;
+                                       __u32 addr;
+                                       if (get_addr_1(&ipaddr, target, AF_INET)) {
+                                               addattrstrz(n, 1024, i, target);
+                                       } else {
+                                               addr = get_addr32(target);
+                                               addattr32(n, 1024, i, addr);
+                                       }
                                        target = strtok(NULL, ",");
                                }
                                addattr_nest_end(n, nest);

Signed-off-by: David Wilder <wilder@us.ibm.com>

David J Wilder (1):
  bonding: Update to the bonding documentation.

David Wilder (3):
  bonding: Adding struct bond_arp_target
  bonding: Extend arp_ip_target format to allow for a list of vlan tags.
  bonding: Selftest for the arp_ip_target parameter.

 Documentation/networking/bonding.rst          |  11 +
 drivers/net/bonding/bond_main.c               |  74 ++++---
 drivers/net/bonding/bond_netlink.c            |  15 +-
 drivers/net/bonding/bond_options.c            |  71 ++++---
 drivers/net/bonding/bond_procfs.c             |   7 +-
 drivers/net/bonding/bond_sysfs.c              |   9 +-
 include/net/bond_options.h                    |  20 ++
 include/net/bonding.h                         | 161 ++++++++++++++-
 .../selftests/drivers/net/bonding/Makefile    |   3 +-
 .../drivers/net/bonding/bond-arp-ip-target.sh | 194 ++++++++++++++++++
 10 files changed, 484 insertions(+), 81 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/bonding/bond-arp-ip-target.sh

-- 
2.43.5

[PATCH net-next v3 1/4] bonding: Adding struct bond_arp_target

[David Wilder]

Replacing the definition of bond_params.arp_targets (__be32 arp_targets[])
with:

struct bond_arp_target {
	__be32			target_ip;
	struct bond_vlan_tag	*tags;
	u32			flags;
};

To provide storage for a list of vlan tags for each target.

All references to arp_target are change to use the new structure.

Signed-off-by: David Wilder <wilder@us.ibm.com>
---
 drivers/net/bonding/bond_main.c    | 29 ++++++++++++++++-------------
 drivers/net/bonding/bond_netlink.c |  4 ++--
 drivers/net/bonding/bond_options.c | 18 +++++++++---------
 drivers/net/bonding/bond_procfs.c  |  4 ++--
 drivers/net/bonding/bond_sysfs.c   |  4 ++--
 include/net/bond_options.h         | 20 ++++++++++++++++++++
 include/net/bonding.h              | 15 +++++----------
 7 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c4d53e8e7c15..ac654b384ea1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3146,26 +3146,29 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 {
 	struct rtable *rt;
 	struct bond_vlan_tag *tags;
-	__be32 *targets = bond->params.arp_targets, addr;
+	struct bond_arp_target *targets = bond->params.arp_targets;
+	__be32 target_ip, addr;
 	int i;
 
-	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) {
+	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i].target_ip; i++) {
+		target_ip = targets[i].target_ip;
+		tags = targets[i].tags;
+
 		slave_dbg(bond->dev, slave->dev, "%s: target %pI4\n",
-			  __func__, &targets[i]);
-		tags = NULL;
+			  __func__, &target_ip);
 
 		/* Find out through which dev should the packet go */
-		rt = ip_route_output(dev_net(bond->dev), targets[i], 0, 0, 0,
+		rt = ip_route_output(dev_net(bond->dev), target_ip, 0, 0, 0,
 				     RT_SCOPE_LINK);
 		if (IS_ERR(rt)) {
-			/* there's no route to target - try to send arp
+			/* there's no route to target_ip - try to send arp
 			 * probe to generate any traffic (arp_validate=0)
 			 */
 			if (bond->params.arp_validate)
 				pr_warn_once("%s: no route to arp_ip_target %pI4 and arp_validate is set\n",
 					     bond->dev->name,
-					     &targets[i]);
-			bond_arp_send(slave, ARPOP_REQUEST, targets[i],
+					     &target_ip);
+			bond_arp_send(slave, ARPOP_REQUEST, target_ip,
 				      0, tags);
 			continue;
 		}
@@ -3183,15 +3186,15 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 
 		/* Not our device - skip */
 		slave_dbg(bond->dev, slave->dev, "no path to arp_ip_target %pI4 via rt.dev %s\n",
-			   &targets[i], rt->dst.dev ? rt->dst.dev->name : "NULL");
+			   &target_ip, rt->dst.dev ? rt->dst.dev->name : "NULL");
 
 		ip_rt_put(rt);
 		continue;
 
 found:
-		addr = bond_confirm_addr(rt->dst.dev, targets[i], 0);
+		addr = bond_confirm_addr(rt->dst.dev, target_ip, 0);
 		ip_rt_put(rt);
-		bond_arp_send(slave, ARPOP_REQUEST, targets[i], addr, tags);
+		bond_arp_send(slave, ARPOP_REQUEST, target_ip, addr, tags);
 		kfree(tags);
 	}
 }
@@ -6096,7 +6099,7 @@ static int __init bond_check_params(struct bond_params *params)
 	int arp_all_targets_value = 0;
 	u16 ad_actor_sys_prio = 0;
 	u16 ad_user_port_key = 0;
-	__be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
+	struct bond_arp_target arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
 	int arp_ip_count;
 	int bond_mode	= BOND_MODE_ROUNDROBIN;
 	int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
@@ -6290,7 +6293,7 @@ static int __init bond_check_params(struct bond_params *params)
 			arp_interval = 0;
 		} else {
 			if (bond_get_targets_ip(arp_target, ip) == -1)
-				arp_target[arp_ip_count++] = ip;
+				arp_target[arp_ip_count++].target_ip = ip;
 			else
 				pr_warn("Warning: duplicate address %pI4 in arp_ip_target, skipping\n",
 					&ip);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index ac5e402c34bc..1a3d17754c0a 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -688,8 +688,8 @@ static int bond_fill_info(struct sk_buff *skb,
 
 	targets_added = 0;
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
-		if (bond->params.arp_targets[i]) {
-			if (nla_put_be32(skb, i, bond->params.arp_targets[i]))
+		if (bond->params.arp_targets[i].target_ip) {
+			if (nla_put_be32(skb, i, bond->params.arp_targets[i].target_ip))
 				goto nla_put_failure;
 			targets_added = 1;
 		}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 91893c29b899..e4b7eb376575 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1090,7 +1090,7 @@ static int bond_option_arp_interval_set(struct bonding *bond,
 			netdev_dbg(bond->dev, "ARP monitoring cannot be used with MII monitoring. Disabling MII monitoring\n");
 			bond->params.miimon = 0;
 		}
-		if (!bond->params.arp_targets[0])
+		if (!bond->params.arp_targets[0].target_ip)
 			netdev_dbg(bond->dev, "ARP monitoring has been set up, but no ARP targets have been specified\n");
 	}
 	if (bond->dev->flags & IFF_UP) {
@@ -1118,20 +1118,20 @@ static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot,
 					    __be32 target,
 					    unsigned long last_rx)
 {
-	__be32 *targets = bond->params.arp_targets;
+	struct bond_arp_target *targets = bond->params.arp_targets;
 	struct list_head *iter;
 	struct slave *slave;
 
 	if (slot >= 0 && slot < BOND_MAX_ARP_TARGETS) {
 		bond_for_each_slave(bond, slave, iter)
 			slave->target_last_arp_rx[slot] = last_rx;
-		targets[slot] = target;
+		targets[slot].target_ip = target;
 	}
 }
 
 static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 {
-	__be32 *targets = bond->params.arp_targets;
+	struct bond_arp_target *targets = bond->params.arp_targets;
 	int ind;
 
 	if (!bond_is_ip_target_ok(target)) {
@@ -1166,7 +1166,7 @@ static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 
 static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 {
-	__be32 *targets = bond->params.arp_targets;
+	struct bond_arp_target *targets = bond->params.arp_targets;
 	struct list_head *iter;
 	struct slave *slave;
 	unsigned long *targets_rx;
@@ -1185,20 +1185,20 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 		return -EINVAL;
 	}
 
-	if (ind == 0 && !targets[1] && bond->params.arp_interval)
+	if (ind == 0 && !targets[1].target_ip && bond->params.arp_interval)
 		netdev_warn(bond->dev, "Removing last arp target with arp_interval on\n");
 
 	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target);
 
 	bond_for_each_slave(bond, slave, iter) {
 		targets_rx = slave->target_last_arp_rx;
-		for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
+		for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++)
 			targets_rx[i] = targets_rx[i+1];
 		targets_rx[i] = 0;
 	}
-	for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
+	for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++)
 		targets[i] = targets[i+1];
-	targets[i] = 0;
+	targets[i].target_ip = 0;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 7edf72ec816a..94e6fd7041ee 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -121,11 +121,11 @@ static void bond_info_show_master(struct seq_file *seq)
 		seq_printf(seq, "ARP IP target/s (n.n.n.n form):");
 
 		for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
-			if (!bond->params.arp_targets[i])
+			if (!bond->params.arp_targets[i].target_ip)
 				break;
 			if (printed)
 				seq_printf(seq, ",");
-			seq_printf(seq, " %pI4", &bond->params.arp_targets[i]);
+			seq_printf(seq, " %pI4", &bond->params.arp_targets[i].target_ip);
 			printed = 1;
 		}
 		seq_printf(seq, "\n");
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 1e13bb170515..d7c09e0a14dd 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -290,9 +290,9 @@ static ssize_t bonding_show_arp_targets(struct device *d,
 	int i, res = 0;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
-		if (bond->params.arp_targets[i])
+		if (bond->params.arp_targets[i].target_ip)
 			res += sysfs_emit_at(buf, res, "%pI4 ",
-					     &bond->params.arp_targets[i]);
+					     &bond->params.arp_targets[i].target_ip);
 	}
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 18687ccf0638..0a7d690cb005 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -119,6 +119,26 @@ struct bond_option {
 	int (*set)(struct bonding *bond, const struct bond_opt_value *val);
 };
 
+struct bond_vlan_tag {
+	__be16		vlan_proto;
+	unsigned short	vlan_id;
+};
+
+/* Value type flags:
+ *  BOND_TARGET_DONTFREE - never free the tags
+ *  BOND_TARGET_USERTAGS - tags have been supplied by the user
+ */
+enum {
+	BOND_TARGET_DONTFREE = BIT(0),
+	BOND_TARGET_USERTAGS = BIT(1),
+};
+
+struct bond_arp_target {
+	__be32			target_ip;
+	struct bond_vlan_tag	*tags;
+	u32			flags;
+};
+
 int __bond_opt_set(struct bonding *bond, unsigned int option,
 		   struct bond_opt_value *val,
 		   struct nlattr *bad_attr, struct netlink_ext_ack *extack);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 95f67b308c19..5b4c43f02c89 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -135,7 +135,7 @@ struct bond_params {
 	int ad_select;
 	char primary[IFNAMSIZ];
 	int primary_reselect;
-	__be32 arp_targets[BOND_MAX_ARP_TARGETS];
+	struct bond_arp_target arp_targets[BOND_MAX_ARP_TARGETS];
 	int tx_queues;
 	int all_slaves_active;
 	int resend_igmp;
@@ -274,11 +274,6 @@ struct bonding {
 void bond_queue_slave_event(struct slave *slave);
 void bond_lower_state_changed(struct slave *slave);
 
-struct bond_vlan_tag {
-	__be16		vlan_proto;
-	unsigned short	vlan_id;
-};
-
 /*
  * Returns NULL if the net_device does not belong to any of the bond's slaves
  *
@@ -522,7 +517,7 @@ static inline unsigned long slave_oldest_target_arp_rx(struct bonding *bond,
 	int i = 1;
 	unsigned long ret = slave->target_last_arp_rx[0];
 
-	for (; (i < BOND_MAX_ARP_TARGETS) && bond->params.arp_targets[i]; i++)
+	for (; (i < BOND_MAX_ARP_TARGETS) && bond->params.arp_targets[i].target_ip; i++)
 		if (time_before(slave->target_last_arp_rx[i], ret))
 			ret = slave->target_last_arp_rx[i];
 
@@ -760,14 +755,14 @@ static inline bool bond_slave_has_mac_rcu(struct bonding *bond, const u8 *mac)
 /* Check if the ip is present in arp ip list, or first free slot if ip == 0
  * Returns -1 if not found, index if found
  */
-static inline int bond_get_targets_ip(__be32 *targets, __be32 ip)
+static inline int bond_get_targets_ip(struct bond_arp_target *targets, __be32 ip)
 {
 	int i;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
-		if (targets[i] == ip)
+		if (targets[i].target_ip == ip)
 			return i;
-		else if (targets[i] == 0)
+		else if (targets[i].target_ip == 0)
 			break;
 
 	return -1;
-- 
2.43.5

[PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[David Wilder]

This change extends the "arp_ip_target" parameter format to allow for a
list of vlan tags to be included for each arp target. This new list of
tags is optional and may be omitted to preserve the current format and
process of gathering tags.  When provided the list of tags circumvents
the process of gathering tags by using the supplied list. An empty list
can be provided to simply skip the process of gathering tags.

An update to iproute is required to use this change.

Signed-off-by: David Wilder <wilder@us.ibm.com>
---
 drivers/net/bonding/bond_main.c    |  51 +++++-----
 drivers/net/bonding/bond_netlink.c |  11 ++-
 drivers/net/bonding/bond_options.c |  57 ++++++-----
 drivers/net/bonding/bond_procfs.c  |   5 +-
 drivers/net/bonding/bond_sysfs.c   |   9 +-
 include/net/bonding.h              | 146 +++++++++++++++++++++++++++++
 6 files changed, 229 insertions(+), 50 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ac654b384ea1..a26e17bdcc48 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3029,8 +3029,6 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
 	return ret;
 }
 
-#define BOND_VLAN_PROTO_NONE cpu_to_be16(0xffff)
-
 static bool bond_handle_vlan(struct slave *slave, struct bond_vlan_tag *tags,
 			     struct sk_buff *skb)
 {
@@ -3144,22 +3142,24 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
 
 static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 {
-	struct rtable *rt;
-	struct bond_vlan_tag *tags;
 	struct bond_arp_target *targets = bond->params.arp_targets;
+	struct bond_vlan_tag *tags;
 	__be32 target_ip, addr;
+	struct rtable *rt;
 	int i;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i].target_ip; i++) {
 		target_ip = targets[i].target_ip;
 		tags = targets[i].tags;
+		char pbuf[BOND_OPTION_STRING_MAX_SIZE];
 
-		slave_dbg(bond->dev, slave->dev, "%s: target %pI4\n",
-			  __func__, &target_ip);
+		bond_arp_target_to_string(&targets[i], pbuf, sizeof(pbuf));
+		slave_dbg(bond->dev, slave->dev, "%s: target %s\n", __func__, pbuf);
 
 		/* Find out through which dev should the packet go */
 		rt = ip_route_output(dev_net(bond->dev), target_ip, 0, 0, 0,
 				     RT_SCOPE_LINK);
+
 		if (IS_ERR(rt)) {
 			/* there's no route to target_ip - try to send arp
 			 * probe to generate any traffic (arp_validate=0)
@@ -3177,9 +3177,13 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		if (rt->dst.dev == bond->dev)
 			goto found;
 
-		rcu_read_lock();
-		tags = bond_verify_device_path(bond->dev, rt->dst.dev, 0);
-		rcu_read_unlock();
+		if (!tags) {
+			rcu_read_lock();
+			tags = bond_verify_device_path(bond->dev, rt->dst.dev, 0);
+			/* cache the tags */
+			targets[i].tags = tags;
+			rcu_read_unlock();
+		}
 
 		if (!IS_ERR_OR_NULL(tags))
 			goto found;
@@ -3195,7 +3199,6 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		addr = bond_confirm_addr(rt->dst.dev, target_ip, 0);
 		ip_rt_put(rt);
 		bond_arp_send(slave, ARPOP_REQUEST, target_ip, addr, tags);
-		kfree(tags);
 	}
 }
 
@@ -6077,6 +6080,7 @@ static void bond_uninit(struct net_device *bond_dev)
 	bond_for_each_slave(bond, slave, iter)
 		__bond_release_one(bond_dev, slave->dev, true, true);
 	netdev_info(bond_dev, "Released all slaves\n");
+	bond_free_vlan_tags(bond->params.arp_targets);
 
 #ifdef CONFIG_XFRM_OFFLOAD
 	mutex_destroy(&bond->ipsec_lock);
@@ -6283,21 +6287,26 @@ static int __init bond_check_params(struct bond_params *params)
 
 	for (arp_ip_count = 0, i = 0;
 	     (arp_ip_count < BOND_MAX_ARP_TARGETS) && arp_ip_target[i]; i++) {
-		__be32 ip;
+		struct bond_arp_target tmp_arp_target;
 
-		/* not a complete check, but good enough to catch mistakes */
-		if (!in4_pton(arp_ip_target[i], -1, (u8 *)&ip, -1, NULL) ||
-		    !bond_is_ip_target_ok(ip)) {
+		if (bond_arp_ip_target_opt_parse(arp_ip_target[i], &tmp_arp_target)) {
 			pr_warn("Warning: bad arp_ip_target module parameter (%s), ARP monitoring will not be performed\n",
 				arp_ip_target[i]);
 			arp_interval = 0;
-		} else {
-			if (bond_get_targets_ip(arp_target, ip) == -1)
-				arp_target[arp_ip_count++].target_ip = ip;
-			else
-				pr_warn("Warning: duplicate address %pI4 in arp_ip_target, skipping\n",
-					&ip);
+			break;
 		}
+
+		if (bond_get_targets_ip(arp_target, tmp_arp_target.target_ip) != -1) {
+			pr_warn("Warning: duplicate address %pI4 in arp_ip_target, skipping\n",
+				&tmp_arp_target.target_ip);
+			kfree(tmp_arp_target.tags);
+			continue;
+		}
+
+		arp_target[i].target_ip = tmp_arp_target.target_ip;
+		arp_target[i].tags = tmp_arp_target.tags;
+		arp_target[i].flags |= BOND_TARGET_DONTFREE;
+		++arp_ip_count;
 	}
 
 	if (arp_interval && !arp_ip_count) {
@@ -6663,7 +6672,7 @@ static void __exit bonding_exit(void)
 
 	bond_netlink_fini();
 	unregister_pernet_subsys(&bond_net_ops);
-
+	bond_free_vlan_tags_all(bonding_defaults.arp_targets);
 	bond_destroy_debugfs();
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 1a3d17754c0a..bab980e84c08 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -287,14 +287,21 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
 
 		bond_option_arp_ip_targets_clear(bond);
 		nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) {
+			char target_str[BOND_OPTION_STRING_MAX_SIZE];
 			__be32 target;
 
 			if (nla_len(attr) < sizeof(target))
 				return -EINVAL;
 
-			target = nla_get_be32(attr);
+			if (nla_len(attr) > sizeof(target)) {
+				snprintf(target_str, sizeof(target_str),
+					 "%s%s", "+", (__force char *)nla_data(attr));
+				bond_opt_initstr(&newval, target_str);
+			} else {
+				target = nla_get_be32(attr);
+				bond_opt_initval(&newval, (__force u64)target);
+			}
 
-			bond_opt_initval(&newval, (__force u64)target);
 			err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
 					     &newval,
 					     data[IFLA_BOND_ARP_IP_TARGET],
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index e4b7eb376575..42a11483320c 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -31,8 +31,8 @@ static int bond_option_use_carrier_set(struct bonding *bond,
 				       const struct bond_opt_value *newval);
 static int bond_option_arp_interval_set(struct bonding *bond,
 					const struct bond_opt_value *newval);
-static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
-static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
+static int bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target);
+static int bond_option_arp_ip_target_rem(struct bonding *bond, struct bond_arp_target target);
 static int bond_option_arp_ip_targets_set(struct bonding *bond,
 					  const struct bond_opt_value *newval);
 static int bond_option_ns_ip6_targets_set(struct bonding *bond,
@@ -1115,7 +1115,7 @@ static int bond_option_arp_interval_set(struct bonding *bond,
 }
 
 static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot,
-					    __be32 target,
+					    struct bond_arp_target target,
 					    unsigned long last_rx)
 {
 	struct bond_arp_target *targets = bond->params.arp_targets;
@@ -1125,24 +1125,24 @@ static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot,
 	if (slot >= 0 && slot < BOND_MAX_ARP_TARGETS) {
 		bond_for_each_slave(bond, slave, iter)
 			slave->target_last_arp_rx[slot] = last_rx;
-		targets[slot].target_ip = target;
+		memcpy(&targets[slot], &target, sizeof(target));
 	}
 }
 
-static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
+static int _bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target)
 {
 	struct bond_arp_target *targets = bond->params.arp_targets;
 	int ind;
 
-	if (!bond_is_ip_target_ok(target)) {
+	if (!bond_is_ip_target_ok(target.target_ip)) {
 		netdev_err(bond->dev, "invalid ARP target %pI4 specified for addition\n",
-			   &target);
+			   &target.target_ip);
 		return -EINVAL;
 	}
 
-	if (bond_get_targets_ip(targets, target) != -1) { /* dup */
+	if (bond_get_targets_ip(targets, target.target_ip) != -1) { /* dup */
 		netdev_err(bond->dev, "ARP target %pI4 is already present\n",
-			   &target);
+			   &target.target_ip);
 		return -EINVAL;
 	}
 
@@ -1152,19 +1152,19 @@ static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 		return -EINVAL;
 	}
 
-	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target);
+	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target.target_ip);
 
 	_bond_options_arp_ip_target_set(bond, ind, target, jiffies);
 
 	return 0;
 }
 
-static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
+static int bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target)
 {
 	return _bond_option_arp_ip_target_add(bond, target);
 }
 
-static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
+static int bond_option_arp_ip_target_rem(struct bonding *bond, struct bond_arp_target target)
 {
 	struct bond_arp_target *targets = bond->params.arp_targets;
 	struct list_head *iter;
@@ -1172,23 +1172,23 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 	unsigned long *targets_rx;
 	int ind, i;
 
-	if (!bond_is_ip_target_ok(target)) {
+	if (!bond_is_ip_target_ok(target.target_ip)) {
 		netdev_err(bond->dev, "invalid ARP target %pI4 specified for removal\n",
-			   &target);
+			   &target.target_ip);
 		return -EINVAL;
 	}
 
-	ind = bond_get_targets_ip(targets, target);
+	ind = bond_get_targets_ip(targets, target.target_ip);
 	if (ind == -1) {
 		netdev_err(bond->dev, "unable to remove nonexistent ARP target %pI4\n",
-			   &target);
+			   &target.target_ip);
 		return -EINVAL;
 	}
 
 	if (ind == 0 && !targets[1].target_ip && bond->params.arp_interval)
 		netdev_warn(bond->dev, "Removing last arp target with arp_interval on\n");
 
-	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target);
+	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target.target_ip);
 
 	bond_for_each_slave(bond, slave, iter) {
 		targets_rx = slave->target_last_arp_rx;
@@ -1196,9 +1196,16 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 			targets_rx[i] = targets_rx[i+1];
 		targets_rx[i] = 0;
 	}
-	for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++)
-		targets[i] = targets[i+1];
+
+	bond_free_vlan_tag(&targets[ind]);
+
+	for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++) {
+		targets[i].target_ip = targets[i + 1].target_ip;
+		targets[i].tags = targets[i + 1].tags;
+		targets[i].flags = targets[i + 1].flags;
+	}
 	targets[i].target_ip = 0;
+	targets[i].tags = NULL;
 
 	return 0;
 }
@@ -1206,20 +1213,24 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 void bond_option_arp_ip_targets_clear(struct bonding *bond)
 {
 	int i;
+	struct bond_arp_target empty_target;
+
+	empty_target.target_ip = 0;
+	empty_target.tags = NULL;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
-		_bond_options_arp_ip_target_set(bond, i, 0, 0);
+		_bond_options_arp_ip_target_set(bond, i, empty_target, 0);
 }
 
 static int bond_option_arp_ip_targets_set(struct bonding *bond,
 					  const struct bond_opt_value *newval)
 {
 	int ret = -EPERM;
-	__be32 target;
+	struct bond_arp_target target;
 
 	if (newval->string) {
 		if (strlen(newval->string) < 1 ||
-		    !in4_pton(newval->string + 1, -1, (u8 *)&target, -1, NULL)) {
+		    bond_arp_ip_target_opt_parse(newval->string + 1, &target)) {
 			netdev_err(bond->dev, "invalid ARP target specified\n");
 			return ret;
 		}
@@ -1230,7 +1241,7 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
 		else
 			netdev_err(bond->dev, "no command found in arp_ip_targets file - use +<addr> or -<addr>\n");
 	} else {
-		target = newval->value;
+		target.target_ip = newval->value;
 		ret = bond_option_arp_ip_target_add(bond, target);
 	}
 
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 94e6fd7041ee..b07944396912 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -111,6 +111,7 @@ static void bond_info_show_master(struct seq_file *seq)
 
 	/* ARP information */
 	if (bond->params.arp_interval > 0) {
+		char pbuf[BOND_OPTION_STRING_MAX_SIZE];
 		int printed = 0;
 
 		seq_printf(seq, "ARP Polling Interval (ms): %d\n",
@@ -125,7 +126,9 @@ static void bond_info_show_master(struct seq_file *seq)
 				break;
 			if (printed)
 				seq_printf(seq, ",");
-			seq_printf(seq, " %pI4", &bond->params.arp_targets[i].target_ip);
+			bond_arp_target_to_string(&bond->params.arp_targets[i],
+						  pbuf, sizeof(pbuf));
+			seq_printf(seq, " %s", pbuf);
 			printed = 1;
 		}
 		seq_printf(seq, "\n");
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index d7c09e0a14dd..feff53e69bd3 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -288,11 +288,14 @@ static ssize_t bonding_show_arp_targets(struct device *d,
 {
 	struct bonding *bond = to_bond(d);
 	int i, res = 0;
+	char pbuf[BOND_OPTION_STRING_MAX_SIZE];
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
-		if (bond->params.arp_targets[i].target_ip)
-			res += sysfs_emit_at(buf, res, "%pI4 ",
-					     &bond->params.arp_targets[i].target_ip);
+		if (bond->params.arp_targets[i].target_ip) {
+			bond_arp_target_to_string(&bond->params.arp_targets[i],
+						  pbuf, sizeof(pbuf));
+			res += sysfs_emit_at(buf, res, "%s ", pbuf);
+		}
 	}
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 5b4c43f02c89..2067aa4df123 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -23,6 +23,7 @@
 #include <linux/etherdevice.h>
 #include <linux/reciprocal_div.h>
 #include <linux/if_link.h>
+#include <linux/inet.h>
 
 #include <net/bond_3ad.h>
 #include <net/bond_alb.h>
@@ -787,6 +788,151 @@ static inline int bond_get_targets_ip6(struct in6_addr *targets, struct in6_addr
 }
 #endif
 
+#define BOND_VLAN_PROTO_NONE cpu_to_be16(0xffff)
+#define BOND_OPTION_STRING_MAX_SIZE 256
+
+/* Convert vlan_list into struct bond_vlan_tag.
+ * Inspired by bond_verify_device_path();
+ */
+static inline struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int level)
+{
+	struct bond_vlan_tag *tags;
+	char *vlan;
+
+	if (!vlan_list || strlen(vlan_list) == 0) {
+		tags = kcalloc(level + 1, sizeof(*tags), GFP_ATOMIC);
+		if (!tags)
+			return ERR_PTR(-ENOMEM);
+		tags[level].vlan_proto = BOND_VLAN_PROTO_NONE;
+		return tags;
+	}
+
+	for (vlan = strsep(&vlan_list, "/"); (vlan != 0); level++) {
+		tags = bond_vlan_tags_parse(vlan_list, level + 1);
+		if (IS_ERR_OR_NULL(tags)) {
+			if (IS_ERR(tags))
+				return tags;
+			continue;
+		}
+
+		tags[level].vlan_proto = __cpu_to_be16(ETH_P_8021Q);
+		if (kstrtou16(vlan, 0, &tags[level].vlan_id)) {
+			kfree(tags);
+			return ERR_PTR(-EINVAL);
+		}
+
+		if (tags[level].vlan_id < 1 || tags[level].vlan_id > 4094) {
+			kfree(tags);
+			return ERR_PTR(-EINVAL);
+		}
+
+		return tags;
+	}
+
+	return NULL;
+}
+
+/**
+ * bond_arp_ip_target_opt_parse - parse a single arp_ip_target option value string
+ * @src: the option value to be parsed
+ * @dest: struct bond_arp_target to place the results.
+ *
+ * This function parses a single arp_ip_target string in the form:
+ * x.x.x.x[tag/....] into a struct bond_arp_target.
+ * Returns 0 on success.
+ */
+static inline int bond_arp_ip_target_opt_parse(char *src, struct bond_arp_target *dest)
+{
+	char *ipv4, *vlan_list;
+	char target[BOND_OPTION_STRING_MAX_SIZE], *args;
+	struct bond_vlan_tag *tags = NULL;
+	__be32 ip;
+
+	if (strlen(src) > BOND_OPTION_STRING_MAX_SIZE)
+		return -E2BIG;
+
+	pr_debug("Parsing arp_ip_target (%s)\n", src);
+
+	/* copy arp_ip_target[i] to local array, strsep works
+	 * destructively...
+	 */
+	args = target;
+	strscpy(target, src);
+	ipv4 = strsep(&args, "[");
+
+	/* not a complete check, but good enough to catch mistakes */
+	if (!in4_pton(ipv4, -1, (u8 *)&ip, -1, NULL) ||
+	    !bond_is_ip_target_ok(ip)) {
+		return -EINVAL;
+	}
+
+	/* extract vlan tags */
+	vlan_list = strsep(&args, "]");
+
+	/* If a vlan list was not supplied skip the processing of the list.
+	 * A value of "[]" is a valid list and should be handled a such.
+	 */
+	if (vlan_list) {
+		tags = bond_vlan_tags_parse(vlan_list, 0);
+		dest->flags |= BOND_TARGET_USERTAGS;
+		if (IS_ERR(tags))
+			return PTR_ERR(tags);
+	}
+
+	dest->target_ip = ip;
+	dest->tags = tags;
+
+	return 0;
+}
+
+static inline int bond_arp_target_to_string(struct bond_arp_target *target,
+					    char *buf, int size)
+{
+	struct bond_vlan_tag *tags = target->tags;
+	int i, num = 0;
+
+	if (!(target->flags & BOND_TARGET_USERTAGS)) {
+		num = snprintf(&buf[0], size, "%pI4", &target->target_ip);
+		return num;
+	}
+
+	num = snprintf(&buf[0], size, "%pI4[", &target->target_ip);
+	if (tags) {
+		for (i = 0; (tags[i].vlan_proto != BOND_VLAN_PROTO_NONE); i++) {
+			if (!tags[i].vlan_id)
+				continue;
+			if (i != 0)
+				num = num + snprintf(&buf[num], size-num, "/");
+			num = num + snprintf(&buf[num], size-num, "%u",
+					     tags[i].vlan_id);
+		}
+	}
+	snprintf(&buf[num], size-num, "]");
+	return num;
+}
+
+static inline void bond_free_vlan_tag(struct bond_arp_target *target)
+{
+	if (!(target->flags & BOND_TARGET_DONTFREE))
+		kfree(target->tags);
+}
+
+static inline void __bond_free_vlan_tags(struct bond_arp_target *targets, int all)
+{
+	int i;
+
+	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i].tags; i++) {
+		if (!all)
+			bond_free_vlan_tag(&targets[i]);
+		else
+			kfree(targets[i].tags);
+	}
+}
+
+#define bond_free_vlan_tags(targets)  __bond_free_vlan_tags(targets, 0)
+#define bond_free_vlan_tags_all(targets) __bond_free_vlan_tags(targets, 1)
+
+
 /* exported from bond_main.c */
 extern unsigned int bond_net_id;
 
-- 
2.43.5

[PATCH net-next v3 3/4] bonding: Selftest for the arp_ip_target parameter.

[David Wilder]

This selftest provided a functional test for the arp_ip_target parameter
both with and without user supplied vlan tags.

Signed-off-by: David Wilder <wilder@us.ibm.com>
---
 .../selftests/drivers/net/bonding/Makefile    |   3 +-
 .../drivers/net/bonding/bond-arp-ip-target.sh | 194 ++++++++++++++++++
 2 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/drivers/net/bonding/bond-arp-ip-target.sh

diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index 2b10854e4b1e..c59bb2912a38 100644
--- a/tools/testing/selftests/drivers/net/bonding/Makefile
+++ b/tools/testing/selftests/drivers/net/bonding/Makefile
@@ -10,7 +10,8 @@ TEST_PROGS := \
 	mode-2-recovery-updelay.sh \
 	bond_options.sh \
 	bond-eth-type-change.sh \
-	bond_macvlan_ipvlan.sh
+	bond_macvlan_ipvlan.sh \
+	bond-arp-ip-target.sh
 
 TEST_FILES := \
 	lag_lib.sh \
diff --git a/tools/testing/selftests/drivers/net/bonding/bond-arp-ip-target.sh b/tools/testing/selftests/drivers/net/bonding/bond-arp-ip-target.sh
new file mode 100755
index 000000000000..89c698d28701
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bonding/bond-arp-ip-target.sh
@@ -0,0 +1,194 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test bonding arp_ip_target.
+# Topology for Bond mode 1,5,6 testing
+#
+#  +-------------------------+
+#  |                         | Server
+#  |       bond.10.20.30     | 192.30.2.1/24
+#  |            |            |
+#  |        bond0.10.20      | 192.20.2.1/24
+#  |            |            |
+#  |         bond0.10        | 192.10.2.1/24
+#  |            |            |
+#  |          bond0          | 192.0.2.1/24
+#  |            |            |
+#  |            +            |
+#  |      eth0  |  eth1      |
+#  |        +---+---+        |
+#  |        |       |        |
+#  +-------------------------+
+#           |       |
+#  +-------------------------+
+#  |        |       |        |
+#  |    +---+-------+---+    |  Gateway
+#  |    |      br0      |    |
+#  |    +-------+-------+    |
+#  |            |            |
+#  +-------------------------+
+#               |
+#  +-------------------------+
+#  |            |            |  Client
+#  |          eth0           | 192.0.0.2/24
+#  |            |            |
+#  |         eth0.10         | 192.10.10.2/24
+#  |            |            |
+#  |        eth0.10.20       | 192.20.20.2/24
+#  |            |            |
+#  |      eth0.10.20.30      | 192.30.30.2/24
+#  +-------------------------+
+
+
+lib_dir=$(dirname "$0")
+source ${lib_dir}/bond_topo_2d1c.sh
+
+DEBUG=${DEBUG:-0}
+test ${DEBUG} -ne 0 && set -x
+
+# vlan subnets
+s_ip4v10="192.10.2.1"
+s_ip4v20="192.20.2.1"
+s_ip4v30="192.30.2.1"
+c_ip4v10="192.10.2.10"
+c_ip4v20="192.20.2.10"
+c_ip4v30="192.30.2.10"
+
+ALL_TESTS="
+    no_vlan_hints
+    with_vlan_hints
+"
+
+# load bonding modules and set options.
+load_bonding() {
+    local bond_options="$*"
+
+    lsmod | grep bonding > /dev/null
+    if [ $? == 0 ]; then
+        rmmod bonding
+    fi
+    modprobe bonding ${bond_options}
+}
+
+# Build stacked vlans on top of an interface.
+stack_vlans()
+{
+    RET=0
+    local interface=$1
+    local ns=$2
+    local last=$interface
+    local tags="10 20"
+
+    ip -n ${ns} link show ${interface} > /dev/null
+    if [[ $? -ne 0 ]] && RET=1; then
+        msg="Failed to create ${interface}"
+        return
+    fi
+
+    if [ $ns == ${s_ns} ]; then host=1; else host=10;fi
+
+    for tag in $tags; do
+        ip -n ${ns} link add link $last name $last.$tag type vlan id $tag
+        ip -n ${ns} address add 192.$tag.2.$host/24 dev $last.$tag
+        ip -n ${ns} link set up dev $last.$tag
+        last=$last.$tag
+    done
+}
+
+# Check for link flapping
+check_failure_count()
+{
+    RET=0
+    local ns=$1
+    local proc_file=/proc/net/bonding/$2
+    local  counts
+
+    # Give the bond time to settle.
+    sleep 10
+
+    counts=$(ip netns exec ${ns} grep -F "Link Failure Count" ${proc_file} | awk -F: '{print $2}')
+
+    local i
+    for i in $counts; do
+        [ $i != 0 ] && RET=1
+    done
+}
+
+setup_bond_topo()
+{
+    load_bonding $*
+    setup_prepare
+    setup_wait
+    stack_vlans bond0 ${s_ns}
+    stack_vlans eth0 ${c_ns}
+}
+
+skip_with_vlan_hints()
+{
+    local skip=1
+
+    # check if iproute support prio option
+    ip -n ${s_ns} link add bond2 type bond arp_ip_target 10.0.0.1[10]
+    [[ $? -ne 0 ]] && skip=0
+    ip -n ${s_ns} link del bond2 2> /dev/null
+
+    return $skip
+}
+
+no_vlan_hints()
+{
+        RET=0
+        local targets="${c_ip4} ${c_ip4v10} ${c_ip4v20}"
+        local target
+        msg=""
+
+        for target in $targets; do
+                bond_reset "mode $mode arp_interval 100 arp_ip_target ${target}"
+
+                stack_vlans bond0 ${s_ns}
+                if [ $RET -ne 0 ]; then
+                    log_test "no_vlan_hints" "${msg}"
+                    return
+                fi
+
+                check_failure_count ${s_ns} bond0
+                log_test "arp_ip_target=${target} ${msg}"
+        done
+}
+
+with_vlan_hints()
+{
+        RET=0
+        local targets="${c_ip4}[] ${c_ip4v10}[10] ${c_ip4v20}[10/20]"
+        local target
+        msg=""
+
+        if skip_with_vlan_hints; then
+            log_test_skip "skip_with_vlan_hints" \
+	    "Current iproute doesn't support extended arp_ip_target options."
+            return 0
+        fi
+
+        for target in $targets; do
+                bond_reset "mode $mode arp_interval 100 arp_ip_target ${target}"
+
+                stack_vlans bond0 ${s_ns}
+                if [ $RET -ne 0 ]; then
+                    log_test "no_vlan_hints" "${msg}"
+                    return
+                fi
+
+                check_failure_count ${s_ns} bond0
+                log_test "arp_ip_target=${target} ${msg}"
+        done
+}
+
+
+trap cleanup EXIT
+
+mode=active-backup
+
+setup_bond_topo # dyndbg=+p
+tests_run
+
+exit $EXIT_STATUS
-- 
2.43.5

[PATCH net-next v3 4/4] bonding: Update to the bonding documentation.

[David Wilder]

From: David J Wilder <wilder@us.ibm.com>

How to extend the arp_ip_target parameter to include
a list of vlan tags.

Signed-off-by: David J Wilder <wilder@us.ibm.com>
---
 Documentation/networking/bonding.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index a4c1291d2561..aa76b94b1d88 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -313,6 +313,17 @@ arp_ip_target
 	maximum number of targets that can be specified is 16.  The
 	default value is no IP addresses.
 
+        When an arp_ip_target is configured the bonding driver will
+        attempt to automatically determine what vlans the arp probe will
+        pass through. This process of gathering vlan tags is required
+        for the arp probe to be sent. However, in some configurations
+        this process may fail. In these cases you may manually
+        supply a list of vlan tags. To specify a list of vlan tags
+        append the ipv4 address with [tag1/tag2...]. For example:
+        arp_ip_target=10.0.0.1[10]. If you simply need to disable the
+        vlan discovery process you may provide an empty list, for example:
+        arp_ip_target=10.0.0.1[].
+
 ns_ip6_target
 
 	Specifies the IPv6 addresses to use as IPv6 monitoring peers when
-- 
2.43.5

Re: [PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[kernel test robot]

Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Wilder/bonding-Adding-struct-bond_arp_target/20250614-095027
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250614014900.226472-3-wilder%40us.ibm.com
patch subject: [PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.
config: openrisc-randconfig-r133-20250615 (https://download.01.org/0day-ci/archive/20250615/202506151306.ebIcSmWf-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0
reproduce: (https://download.01.org/0day-ci/archive/20250615/202506151306.ebIcSmWf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506151306.ebIcSmWf-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/bonding/bond_options.c:1244:34: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] target_ip @@     got unsigned long long const [usertype] value @@
   drivers/net/bonding/bond_options.c:1244:34: sparse:     expected restricted __be32 [addressable] [usertype] target_ip
   drivers/net/bonding/bond_options.c:1244:34: sparse:     got unsigned long long const [usertype] value
   drivers/net/bonding/bond_options.c: note: in included file:
>> include/net/bonding.h:810:55: sparse: sparse: Using plain integer as NULL pointer
--
   drivers/net/bonding/bond_main.c: note: in included file:
>> include/net/bonding.h:810:55: sparse: sparse: Using plain integer as NULL pointer

vim +1244 drivers/net/bonding/bond_options.c

  1224	
  1225	static int bond_option_arp_ip_targets_set(struct bonding *bond,
  1226						  const struct bond_opt_value *newval)
  1227	{
  1228		int ret = -EPERM;
  1229		struct bond_arp_target target;
  1230	
  1231		if (newval->string) {
  1232			if (strlen(newval->string) < 1 ||
  1233			    bond_arp_ip_target_opt_parse(newval->string + 1, &target)) {
  1234				netdev_err(bond->dev, "invalid ARP target specified\n");
  1235				return ret;
  1236			}
  1237			if (newval->string[0] == '+')
  1238				ret = bond_option_arp_ip_target_add(bond, target);
  1239			else if (newval->string[0] == '-')
  1240				ret = bond_option_arp_ip_target_rem(bond, target);
  1241			else
  1242				netdev_err(bond->dev, "no command found in arp_ip_targets file - use +<addr> or -<addr>\n");
  1243		} else {
> 1244			target.target_ip = newval->value;
  1245			ret = bond_option_arp_ip_target_add(bond, target);
  1246		}
  1247	
  1248		return ret;
  1249	}
  1250	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[Jay Vosburgh]

David Wilder <wilder@us.ibm.com> wrote:

>This change extends the "arp_ip_target" parameter format to allow for a
>list of vlan tags to be included for each arp target. This new list of
>tags is optional and may be omitted to preserve the current format and
>process of gathering tags.  When provided the list of tags circumvents
>the process of gathering tags by using the supplied list. An empty list
>can be provided to simply skip the process of gathering tags.
>
>An update to iproute is required to use this change.
>
>Signed-off-by: David Wilder <wilder@us.ibm.com>
>---
> drivers/net/bonding/bond_main.c    |  51 +++++-----
> drivers/net/bonding/bond_netlink.c |  11 ++-
> drivers/net/bonding/bond_options.c |  57 ++++++-----
> drivers/net/bonding/bond_procfs.c  |   5 +-
> drivers/net/bonding/bond_sysfs.c   |   9 +-
> include/net/bonding.h              | 146 +++++++++++++++++++++++++++++
> 6 files changed, 229 insertions(+), 50 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ac654b384ea1..a26e17bdcc48 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3029,8 +3029,6 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
> 	return ret;
> }
> 
>-#define BOND_VLAN_PROTO_NONE cpu_to_be16(0xffff)
>-
> static bool bond_handle_vlan(struct slave *slave, struct bond_vlan_tag *tags,
> 			     struct sk_buff *skb)
> {
>@@ -3144,22 +3142,24 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> 
> static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> {
>-	struct rtable *rt;
>-	struct bond_vlan_tag *tags;
> 	struct bond_arp_target *targets = bond->params.arp_targets;
>+	struct bond_vlan_tag *tags;
> 	__be32 target_ip, addr;
>+	struct rtable *rt;
> 	int i;
> 
> 	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i].target_ip; i++) {
> 		target_ip = targets[i].target_ip;
> 		tags = targets[i].tags;
>+		char pbuf[BOND_OPTION_STRING_MAX_SIZE];
> 
>-		slave_dbg(bond->dev, slave->dev, "%s: target %pI4\n",
>-			  __func__, &target_ip);
>+		bond_arp_target_to_string(&targets[i], pbuf, sizeof(pbuf));
>+		slave_dbg(bond->dev, slave->dev, "%s: target %s\n", __func__, pbuf);

	As an aside to my main question, below, I'll point out here that
the target is going to be put into a string even if debug is disabled.

> 
> 		/* Find out through which dev should the packet go */
> 		rt = ip_route_output(dev_net(bond->dev), target_ip, 0, 0, 0,
> 				     RT_SCOPE_LINK);
>+
> 		if (IS_ERR(rt)) {
> 			/* there's no route to target_ip - try to send arp
> 			 * probe to generate any traffic (arp_validate=0)
>@@ -3177,9 +3177,13 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> 		if (rt->dst.dev == bond->dev)
> 			goto found;
> 
>-		rcu_read_lock();
>-		tags = bond_verify_device_path(bond->dev, rt->dst.dev, 0);
>-		rcu_read_unlock();
>+		if (!tags) {
>+			rcu_read_lock();
>+			tags = bond_verify_device_path(bond->dev, rt->dst.dev, 0);
>+			/* cache the tags */
>+			targets[i].tags = tags;
>+			rcu_read_unlock();
>+		}
> 
> 		if (!IS_ERR_OR_NULL(tags))
> 			goto found;
>@@ -3195,7 +3199,6 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> 		addr = bond_confirm_addr(rt->dst.dev, target_ip, 0);
> 		ip_rt_put(rt);
> 		bond_arp_send(slave, ARPOP_REQUEST, target_ip, addr, tags);
>-		kfree(tags);
> 	}
> }
> 
>@@ -6077,6 +6080,7 @@ static void bond_uninit(struct net_device *bond_dev)
> 	bond_for_each_slave(bond, slave, iter)
> 		__bond_release_one(bond_dev, slave->dev, true, true);
> 	netdev_info(bond_dev, "Released all slaves\n");
>+	bond_free_vlan_tags(bond->params.arp_targets);
> 
> #ifdef CONFIG_XFRM_OFFLOAD
> 	mutex_destroy(&bond->ipsec_lock);
>@@ -6283,21 +6287,26 @@ static int __init bond_check_params(struct bond_params *params)
> 
> 	for (arp_ip_count = 0, i = 0;
> 	     (arp_ip_count < BOND_MAX_ARP_TARGETS) && arp_ip_target[i]; i++) {
>-		__be32 ip;
>+		struct bond_arp_target tmp_arp_target;
> 
>-		/* not a complete check, but good enough to catch mistakes */
>-		if (!in4_pton(arp_ip_target[i], -1, (u8 *)&ip, -1, NULL) ||
>-		    !bond_is_ip_target_ok(ip)) {
>+		if (bond_arp_ip_target_opt_parse(arp_ip_target[i], &tmp_arp_target)) {
> 			pr_warn("Warning: bad arp_ip_target module parameter (%s), ARP monitoring will not be performed\n",
> 				arp_ip_target[i]);
> 			arp_interval = 0;
>-		} else {
>-			if (bond_get_targets_ip(arp_target, ip) == -1)
>-				arp_target[arp_ip_count++].target_ip = ip;
>-			else
>-				pr_warn("Warning: duplicate address %pI4 in arp_ip_target, skipping\n",
>-					&ip);
>+			break;
> 		}
>+
>+		if (bond_get_targets_ip(arp_target, tmp_arp_target.target_ip) != -1) {
>+			pr_warn("Warning: duplicate address %pI4 in arp_ip_target, skipping\n",
>+				&tmp_arp_target.target_ip);
>+			kfree(tmp_arp_target.tags);
>+			continue;
>+		}
>+
>+		arp_target[i].target_ip = tmp_arp_target.target_ip;
>+		arp_target[i].tags = tmp_arp_target.tags;
>+		arp_target[i].flags |= BOND_TARGET_DONTFREE;
>+		++arp_ip_count;
> 	}
> 
> 	if (arp_interval && !arp_ip_count) {
>@@ -6663,7 +6672,7 @@ static void __exit bonding_exit(void)
> 
> 	bond_netlink_fini();
> 	unregister_pernet_subsys(&bond_net_ops);
>-
>+	bond_free_vlan_tags_all(bonding_defaults.arp_targets);
> 	bond_destroy_debugfs();
> 
> #ifdef CONFIG_NET_POLL_CONTROLLER
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 1a3d17754c0a..bab980e84c08 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -287,14 +287,21 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> 
> 		bond_option_arp_ip_targets_clear(bond);
> 		nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) {
>+			char target_str[BOND_OPTION_STRING_MAX_SIZE];
> 			__be32 target;
> 
> 			if (nla_len(attr) < sizeof(target))
> 				return -EINVAL;
> 
>-			target = nla_get_be32(attr);
>+			if (nla_len(attr) > sizeof(target)) {
>+				snprintf(target_str, sizeof(target_str),
>+					 "%s%s", "+", (__force char *)nla_data(attr));
>+				bond_opt_initstr(&newval, target_str);
>+			} else {
>+				target = nla_get_be32(attr);
>+				bond_opt_initval(&newval, (__force u64)target);
>+			}

	Here, and further down in bond_arp_ip_target_opt_parse(),
there's a lot of string handling that seems out place.  Why isn't the
string parsing done in user space (iproute, et al), and the tags passed
to the kernel in IFLA_BOND_ARP_IP_TARGET as an optional nested
attribute?

> 
>-			bond_opt_initval(&newval, (__force u64)target);
> 			err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
> 					     &newval,
> 					     data[IFLA_BOND_ARP_IP_TARGET],
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index e4b7eb376575..42a11483320c 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -31,8 +31,8 @@ static int bond_option_use_carrier_set(struct bonding *bond,
> 				       const struct bond_opt_value *newval);
> static int bond_option_arp_interval_set(struct bonding *bond,
> 					const struct bond_opt_value *newval);
>-static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
>-static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
>+static int bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target);
>+static int bond_option_arp_ip_target_rem(struct bonding *bond, struct bond_arp_target target);
> static int bond_option_arp_ip_targets_set(struct bonding *bond,
> 					  const struct bond_opt_value *newval);
> static int bond_option_ns_ip6_targets_set(struct bonding *bond,
>@@ -1115,7 +1115,7 @@ static int bond_option_arp_interval_set(struct bonding *bond,
> }
> 
> static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot,
>-					    __be32 target,
>+					    struct bond_arp_target target,
> 					    unsigned long last_rx)
> {
> 	struct bond_arp_target *targets = bond->params.arp_targets;
>@@ -1125,24 +1125,24 @@ static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot,
> 	if (slot >= 0 && slot < BOND_MAX_ARP_TARGETS) {
> 		bond_for_each_slave(bond, slave, iter)
> 			slave->target_last_arp_rx[slot] = last_rx;
>-		targets[slot].target_ip = target;
>+		memcpy(&targets[slot], &target, sizeof(target));
> 	}
> }
> 
>-static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
>+static int _bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target)
> {
> 	struct bond_arp_target *targets = bond->params.arp_targets;
> 	int ind;
> 
>-	if (!bond_is_ip_target_ok(target)) {
>+	if (!bond_is_ip_target_ok(target.target_ip)) {
> 		netdev_err(bond->dev, "invalid ARP target %pI4 specified for addition\n",
>-			   &target);
>+			   &target.target_ip);
> 		return -EINVAL;
> 	}
> 
>-	if (bond_get_targets_ip(targets, target) != -1) { /* dup */
>+	if (bond_get_targets_ip(targets, target.target_ip) != -1) { /* dup */
> 		netdev_err(bond->dev, "ARP target %pI4 is already present\n",
>-			   &target);
>+			   &target.target_ip);
> 		return -EINVAL;
> 	}
> 
>@@ -1152,19 +1152,19 @@ static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
> 		return -EINVAL;
> 	}
> 
>-	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target);
>+	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target.target_ip);
> 
> 	_bond_options_arp_ip_target_set(bond, ind, target, jiffies);
> 
> 	return 0;
> }
> 
>-static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
>+static int bond_option_arp_ip_target_add(struct bonding *bond, struct bond_arp_target target)
> {
> 	return _bond_option_arp_ip_target_add(bond, target);
> }
> 
>-static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
>+static int bond_option_arp_ip_target_rem(struct bonding *bond, struct bond_arp_target target)
> {
> 	struct bond_arp_target *targets = bond->params.arp_targets;
> 	struct list_head *iter;
>@@ -1172,23 +1172,23 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
> 	unsigned long *targets_rx;
> 	int ind, i;
> 
>-	if (!bond_is_ip_target_ok(target)) {
>+	if (!bond_is_ip_target_ok(target.target_ip)) {
> 		netdev_err(bond->dev, "invalid ARP target %pI4 specified for removal\n",
>-			   &target);
>+			   &target.target_ip);
> 		return -EINVAL;
> 	}
> 
>-	ind = bond_get_targets_ip(targets, target);
>+	ind = bond_get_targets_ip(targets, target.target_ip);
> 	if (ind == -1) {
> 		netdev_err(bond->dev, "unable to remove nonexistent ARP target %pI4\n",
>-			   &target);
>+			   &target.target_ip);
> 		return -EINVAL;
> 	}
> 
> 	if (ind == 0 && !targets[1].target_ip && bond->params.arp_interval)
> 		netdev_warn(bond->dev, "Removing last arp target with arp_interval on\n");
> 
>-	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target);
>+	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target.target_ip);
> 
> 	bond_for_each_slave(bond, slave, iter) {
> 		targets_rx = slave->target_last_arp_rx;
>@@ -1196,9 +1196,16 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
> 			targets_rx[i] = targets_rx[i+1];
> 		targets_rx[i] = 0;
> 	}
>-	for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++)
>-		targets[i] = targets[i+1];
>+
>+	bond_free_vlan_tag(&targets[ind]);
>+
>+	for (i = ind; (i < BOND_MAX_ARP_TARGETS - 1) && targets[i + 1].target_ip; i++) {
>+		targets[i].target_ip = targets[i + 1].target_ip;
>+		targets[i].tags = targets[i + 1].tags;
>+		targets[i].flags = targets[i + 1].flags;
>+	}
> 	targets[i].target_ip = 0;
>+	targets[i].tags = NULL;
> 
> 	return 0;
> }
>@@ -1206,20 +1213,24 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
> void bond_option_arp_ip_targets_clear(struct bonding *bond)
> {
> 	int i;
>+	struct bond_arp_target empty_target;
>+
>+	empty_target.target_ip = 0;
>+	empty_target.tags = NULL;
> 
> 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
>-		_bond_options_arp_ip_target_set(bond, i, 0, 0);
>+		_bond_options_arp_ip_target_set(bond, i, empty_target, 0);
> }
> 
> static int bond_option_arp_ip_targets_set(struct bonding *bond,
> 					  const struct bond_opt_value *newval)
> {
> 	int ret = -EPERM;
>-	__be32 target;
>+	struct bond_arp_target target;
> 
> 	if (newval->string) {
> 		if (strlen(newval->string) < 1 ||
>-		    !in4_pton(newval->string + 1, -1, (u8 *)&target, -1, NULL)) {
>+		    bond_arp_ip_target_opt_parse(newval->string + 1, &target)) {
> 			netdev_err(bond->dev, "invalid ARP target specified\n");
> 			return ret;
> 		}
>@@ -1230,7 +1241,7 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
> 		else
> 			netdev_err(bond->dev, "no command found in arp_ip_targets file - use +<addr> or -<addr>\n");
> 	} else {
>-		target = newval->value;
>+		target.target_ip = newval->value;
> 		ret = bond_option_arp_ip_target_add(bond, target);
> 	}
> 
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index 94e6fd7041ee..b07944396912 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -111,6 +111,7 @@ static void bond_info_show_master(struct seq_file *seq)
> 
> 	/* ARP information */
> 	if (bond->params.arp_interval > 0) {
>+		char pbuf[BOND_OPTION_STRING_MAX_SIZE];
> 		int printed = 0;
> 
> 		seq_printf(seq, "ARP Polling Interval (ms): %d\n",
>@@ -125,7 +126,9 @@ static void bond_info_show_master(struct seq_file *seq)
> 				break;
> 			if (printed)
> 				seq_printf(seq, ",");
>-			seq_printf(seq, " %pI4", &bond->params.arp_targets[i].target_ip);
>+			bond_arp_target_to_string(&bond->params.arp_targets[i],
>+						  pbuf, sizeof(pbuf));
>+			seq_printf(seq, " %s", pbuf);
> 			printed = 1;
> 		}
> 		seq_printf(seq, "\n");
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index d7c09e0a14dd..feff53e69bd3 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -288,11 +288,14 @@ static ssize_t bonding_show_arp_targets(struct device *d,
> {
> 	struct bonding *bond = to_bond(d);
> 	int i, res = 0;
>+	char pbuf[BOND_OPTION_STRING_MAX_SIZE];
> 
> 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
>-		if (bond->params.arp_targets[i].target_ip)
>-			res += sysfs_emit_at(buf, res, "%pI4 ",
>-					     &bond->params.arp_targets[i].target_ip);
>+		if (bond->params.arp_targets[i].target_ip) {
>+			bond_arp_target_to_string(&bond->params.arp_targets[i],
>+						  pbuf, sizeof(pbuf));
>+			res += sysfs_emit_at(buf, res, "%s ", pbuf);
>+		}

	There is no expectation that sysfs should support new bonding
API elements; only netlink / iproute2 support matters.  If sysfs is the
reason to do the string parsing in the kernel, then I imagine this could
all move into userspace.

	-J

> 	}
> 	if (res)
> 		buf[res-1] = '\n'; /* eat the leftover space */
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 5b4c43f02c89..2067aa4df123 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -23,6 +23,7 @@
> #include <linux/etherdevice.h>
> #include <linux/reciprocal_div.h>
> #include <linux/if_link.h>
>+#include <linux/inet.h>
> 
> #include <net/bond_3ad.h>
> #include <net/bond_alb.h>
>@@ -787,6 +788,151 @@ static inline int bond_get_targets_ip6(struct in6_addr *targets, struct in6_addr
> }
> #endif
> 
>+#define BOND_VLAN_PROTO_NONE cpu_to_be16(0xffff)
>+#define BOND_OPTION_STRING_MAX_SIZE 256
>+
>+/* Convert vlan_list into struct bond_vlan_tag.
>+ * Inspired by bond_verify_device_path();
>+ */
>+static inline struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int level)
>+{
>+	struct bond_vlan_tag *tags;
>+	char *vlan;
>+
>+	if (!vlan_list || strlen(vlan_list) == 0) {
>+		tags = kcalloc(level + 1, sizeof(*tags), GFP_ATOMIC);
>+		if (!tags)
>+			return ERR_PTR(-ENOMEM);
>+		tags[level].vlan_proto = BOND_VLAN_PROTO_NONE;
>+		return tags;
>+	}
>+
>+	for (vlan = strsep(&vlan_list, "/"); (vlan != 0); level++) {
>+		tags = bond_vlan_tags_parse(vlan_list, level + 1);
>+		if (IS_ERR_OR_NULL(tags)) {
>+			if (IS_ERR(tags))
>+				return tags;
>+			continue;
>+		}
>+
>+		tags[level].vlan_proto = __cpu_to_be16(ETH_P_8021Q);
>+		if (kstrtou16(vlan, 0, &tags[level].vlan_id)) {
>+			kfree(tags);
>+			return ERR_PTR(-EINVAL);
>+		}
>+
>+		if (tags[level].vlan_id < 1 || tags[level].vlan_id > 4094) {
>+			kfree(tags);
>+			return ERR_PTR(-EINVAL);
>+		}
>+
>+		return tags;
>+	}
>+
>+	return NULL;
>+}
>+
>+/**
>+ * bond_arp_ip_target_opt_parse - parse a single arp_ip_target option value string
>+ * @src: the option value to be parsed
>+ * @dest: struct bond_arp_target to place the results.
>+ *
>+ * This function parses a single arp_ip_target string in the form:
>+ * x.x.x.x[tag/....] into a struct bond_arp_target.
>+ * Returns 0 on success.
>+ */
>+static inline int bond_arp_ip_target_opt_parse(char *src, struct bond_arp_target *dest)
>+{
>+	char *ipv4, *vlan_list;
>+	char target[BOND_OPTION_STRING_MAX_SIZE], *args;
>+	struct bond_vlan_tag *tags = NULL;
>+	__be32 ip;
>+
>+	if (strlen(src) > BOND_OPTION_STRING_MAX_SIZE)
>+		return -E2BIG;
>+
>+	pr_debug("Parsing arp_ip_target (%s)\n", src);
>+
>+	/* copy arp_ip_target[i] to local array, strsep works
>+	 * destructively...
>+	 */
>+	args = target;
>+	strscpy(target, src);
>+	ipv4 = strsep(&args, "[");
>+
>+	/* not a complete check, but good enough to catch mistakes */
>+	if (!in4_pton(ipv4, -1, (u8 *)&ip, -1, NULL) ||
>+	    !bond_is_ip_target_ok(ip)) {
>+		return -EINVAL;
>+	}
>+
>+	/* extract vlan tags */
>+	vlan_list = strsep(&args, "]");
>+
>+	/* If a vlan list was not supplied skip the processing of the list.
>+	 * A value of "[]" is a valid list and should be handled a such.
>+	 */
>+	if (vlan_list) {
>+		tags = bond_vlan_tags_parse(vlan_list, 0);
>+		dest->flags |= BOND_TARGET_USERTAGS;
>+		if (IS_ERR(tags))
>+			return PTR_ERR(tags);
>+	}
>+
>+	dest->target_ip = ip;
>+	dest->tags = tags;
>+
>+	return 0;
>+}
>+
>+static inline int bond_arp_target_to_string(struct bond_arp_target *target,
>+					    char *buf, int size)
>+{
>+	struct bond_vlan_tag *tags = target->tags;
>+	int i, num = 0;
>+
>+	if (!(target->flags & BOND_TARGET_USERTAGS)) {
>+		num = snprintf(&buf[0], size, "%pI4", &target->target_ip);
>+		return num;
>+	}
>+
>+	num = snprintf(&buf[0], size, "%pI4[", &target->target_ip);
>+	if (tags) {
>+		for (i = 0; (tags[i].vlan_proto != BOND_VLAN_PROTO_NONE); i++) {
>+			if (!tags[i].vlan_id)
>+				continue;
>+			if (i != 0)
>+				num = num + snprintf(&buf[num], size-num, "/");
>+			num = num + snprintf(&buf[num], size-num, "%u",
>+					     tags[i].vlan_id);
>+		}
>+	}
>+	snprintf(&buf[num], size-num, "]");
>+	return num;
>+}
>+
>+static inline void bond_free_vlan_tag(struct bond_arp_target *target)
>+{
>+	if (!(target->flags & BOND_TARGET_DONTFREE))
>+		kfree(target->tags);
>+}
>+
>+static inline void __bond_free_vlan_tags(struct bond_arp_target *targets, int all)
>+{
>+	int i;
>+
>+	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i].tags; i++) {
>+		if (!all)
>+			bond_free_vlan_tag(&targets[i]);
>+		else
>+			kfree(targets[i].tags);
>+	}
>+}
>+
>+#define bond_free_vlan_tags(targets)  __bond_free_vlan_tags(targets, 0)
>+#define bond_free_vlan_tags_all(targets) __bond_free_vlan_tags(targets, 1)
>+
>+
> /* exported from bond_main.c */
> extern unsigned int bond_net_id;
> 
>-- 
>2.43.5
>

---
	-Jay Vosburgh, jv@jvosburgh.net

RE: [PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[David Wilder]

>        Here, and further down in bond_arp_ip_target_opt_parse(),
>there's a lot of string handling that seems out place.  Why isn't the
>string parsing done in user space (iproute, et al), and the tags passed
>to the kernel in IFLA_BOND_ARP_IP_TARGET as an optional nested
>attribute?

>>+              }

>        There is no expectation that sysfs should support new bonding
>API elements; only netlink / iproute2 support matters.  If sysfs is the
>reason to do the string parsing in the kernel, then I imagine this could
>all move into userspace.
>
>     -J

Module parameter support also requires string parsing in the kernel. Can that be dropped as well?

David Wilder wilder@us.ibm.com

Re: [PATCH net-next v3 2/4] bonding: Extend arp_ip_target format to allow for a list of vlan tags.

[Jay Vosburgh]

David Wilder <wilder@us.ibm.com> wrote:

>
>
>>        Here, and further down in bond_arp_ip_target_opt_parse(),
>>there's a lot of string handling that seems out place.  Why isn't the
>>string parsing done in user space (iproute, et al), and the tags passed
>>to the kernel in IFLA_BOND_ARP_IP_TARGET as an optional nested
>>attribute?
>
>>>+              }
>
>>        There is no expectation that sysfs should support new bonding
>>API elements; only netlink / iproute2 support matters.  If sysfs is the
>>reason to do the string parsing in the kernel, then I imagine this could
>>all move into userspace.
>>
>>     -J
>
>Module parameter support also requires string parsing in the kernel. Can that be dropped as well?

	Yes.  New bonding functionality need not be supported by sysfs
or module parameters.

	-J

---
	-Jay Vosburgh, jv@jvosburgh.net