Thread (8 messages) 8 messages, 5 authors, 2025-05-12

Re: [PATCH net-next v1 1/2] bonding: Adding struct arp_target

From: Jay Vosburgh <jv@jvosburgh.net>
Date: 2025-05-10 13:17:03

David J Wilder [off-list ref] wrote:
Replacing the definition of bond_params.arp_targets (__be32 arp_targets[])
with:

struct arp_target {
       __be32 target_ip;
       struct bond_vlan_tag *tags;
};
	Since this struct is only for bonding, perhaps "struct
bond_arp_target"?
quoted hunk ↗ jump to hunk
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 J Wilder <redacted>
---
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/bonding.h              | 15 ++++++++++-----
6 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d05226484c64..ab388dab218a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3151,26 +3151,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 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);
	Perhaps add the tags to the debug message?  It might be kind of
a pain to format, but seems like it would be useful.

	-J
quoted hunk ↗ jump to hunk
		/* 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;
		}
@@ -3188,15 +3191,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);
	}
}
@@ -6102,7 +6105,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 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;
@@ -6296,7 +6299,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..54940950079e 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 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 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 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/bonding.h b/include/net/bonding.h
index 95f67b308c19..709ef9a302dd 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -115,6 +115,11 @@ static inline int is_netpoll_tx_blocked(struct net_device *dev)
#define is_netpoll_tx_blocked(dev) (0)
#endif

+struct arp_target {
+	__be32 target_ip;
+	struct bond_vlan_tag *tags;
+};
+
struct bond_params {
	int mode;
	int xmit_policy;
@@ -135,7 +140,7 @@ struct bond_params {
	int ad_select;
	char primary[IFNAMSIZ];
	int primary_reselect;
-	__be32 arp_targets[BOND_MAX_ARP_TARGETS];
+	struct arp_target arp_targets[BOND_MAX_ARP_TARGETS];
	int tx_queues;
	int all_slaves_active;
	int resend_igmp;
@@ -522,7 +527,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 +765,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 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
---
	-Jay Vosburgh, jv@jvosburgh.net
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help