netdev - Re: [PATCH iproute2-next v1 1/1] iproute: Extend bonding's "arp_ip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2156542.1751508276@vermin>
Date: Wed, 02 Jul 2025 19:04:36 -0700
From: Jay Vosburgh <jv@...sburgh.net>
To: David Wilder <wilder@...ibm.com>
cc: netdev@...r.kernel.org, pradeeps@...ux.vnet.ibm.com,
    pradeep@...ibm.com, i.maximets@....org, amorenoz@...hat.com,
    haliu@...hat.com, stephen@...workplumber.org, dsahern@...il.com
Subject: Re: [PATCH iproute2-next v1 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add vlan tags.

David Wilder <wilder@...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.
>
>The new format for arp_ip_target is:
>arp_ip_target=ipv4-address[vlan-tag\...],...
>
>Examples:
>arp_ip_target=10.0.0.1[10]
>arp_ip_target=10.0.0.1[100/200]
>
>Signed-off-by: David Wilder <wilder@...ibm.com>
>---
> ip/iplink_bond.c | 62 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 59 insertions(+), 3 deletions(-)
>
>diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
>index 19af67d0..bb0b6e84 100644
>--- a/ip/iplink_bond.c
>+++ b/ip/iplink_bond.c
>@@ -173,6 +173,45 @@ static void explain(void)
> 	print_explain(stderr);
> }
> 
>+#define BOND_VLAN_PROTO_NONE htons(0xffff)
>+
>+struct bond_vlan_tag {
>+	__be16	vlan_proto;
>+	__be16	vlan_id;
>+};
>+
>+static inline struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int level, int *size)
>+{
>+	struct bond_vlan_tag *tags = NULL;
>+	char *vlan;
>+	int n;
>+
>+	if (!vlan_list || strlen(vlan_list) == 0) {
>+		tags = calloc(level + 1, sizeof(*tags));
>+		*size = (level + 1) * (sizeof(*tags));
>+		if (tags)
>+			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, size);
>+		if (!tags)
>+			continue;
>+
>+		tags[level].vlan_proto = htons(ETH_P_8021Q);
>+		n = sscanf(vlan, "%hu", &(tags[level].vlan_id));
>+
>+		if (n != 1 || tags[level].vlan_id < 1 ||
>+		    tags[level].vlan_id > 4094)
>+			return NULL;

	Two questions:

	1) Do we care about 802.1p priority tags?  If memory serves,
those manifest as VLAN tags with a VLAN ID of 0 and some other bits set
to provide the priority.  The above appears to disallow such tags.

	2) This loop appears to be unbounded, and will process an
unlimited number of VLANs.  Do we need more than 2 (the original 802.1ad
limit)?  Even if we need more than 2, the upper limit should probably be
some reasonably small number.  The addattr loop (below) will conk out if
the whole thing exceeds 1024 bytes, but that would still permit 120 or
so.

	-J

>+
>+		return tags;
>+	}
>+
>+	return NULL;
>+}
>+
> static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
> 			  struct nlmsghdr *n)
> {
>@@ -239,12 +278,29 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
> 				NEXT_ARG();
> 				char *targets = strdupa(*argv);
> 				char *target = strtok(targets, ",");
>-				int i;
>+				struct bond_vlan_tag *tags;
>+				int size, i;
> 
> 				for (i = 0; target && i < BOND_MAX_ARP_TARGETS; i++) {
>-					__u32 addr = get_addr32(target);
>+					struct Data {
>+						__u32 addr;
>+						struct bond_vlan_tag vlans[];
>+					} data;
>+					char *vlan_list, *dup;
>+
>+					dup = strdupa(target);
>+					data.addr = get_addr32(strsep(&dup, "["));
>+					vlan_list = strsep(&dup, "]");
>+
>+					if (vlan_list) {
>+						tags = bond_vlan_tags_parse(vlan_list, 0, &size);
>+						memcpy(&data.vlans, tags, size);
>+						addattr_l(n, 1024, i, &data,
>+							  sizeof(data.addr)+size);
>+					} else {
>+						addattr32(n, 1024, i, data.addr);
>+					}
> 
>-					addattr32(n, 1024, i, addr);
> 					target = strtok(NULL, ",");
> 				}
> 				addattr_nest_end(n, nest);
>-- 
>2.43.5
>

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