netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next v7 0/1] iproute2-next: Extending bonding's arp_ip_target to include a list of vlan tags.
@ 2025-10-13 23:57 David Wilder
  2025-10-13 23:57 ` [PATCH iproute2-next v7 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add " David Wilder
  0 siblings, 1 reply; 3+ messages in thread
From: David Wilder @ 2025-10-13 23:57 UTC (permalink / raw)
  To: netdev; +Cc: jv, wilder, pradeep, i.maximets, amorenoz, haliu, stephen,
	dsahern

This change extends the "arp_ip_target" option 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 discovering
vlans.  The new logic preserves both forward and backward compatibility with
the kernel and iproute2 versions.

Changes since V6
As each member of the arp_ip_target array consists of
one "addr" key-value pair and one optional "vlan" array it was necessary to
make each arp_ip_target array entry a JSON object.  Here is the new output
(using jq to format).

       "arp_ip_target": [
          {
            "addr": "10.0.0.1",
            "vlan": [
              10,
              20
            ]
          },
          {
            "addr": "10.0.0.2",
            "vlan": [
              5
            ]
          }
        ],

Changes since V5
Thanks to Stephen Hemminger for help on these changes:
- Use array for vlans
- Removed use of packed and Capitalization
- fix incorrect use of color
- Removed temporary string buffer.
- make vlan print a function for likely future IPv6 usage.

Output for ip -d --json <bond-name> has been updated. Example:
"arp_ip_target":["addr":"10.0.0.1","vlan":[4080,4081,4082,4083,4084]],

- changes to error reporting in bond_vlan_tags_parse() for invalid vlan_ids.

Changes since V4
Changed unneeded print_color_string() to print_string(). Thanks Steve.

Change since V3:
1) Add __attribute__((packed)) to struct definition
   to ensure size calculation is correct for memcpy() and
   addattr_l().

Input: arp_ip_target 10.0.0.1[10/20],10.0.0.2[10/20])
Sample JSON output:
...
"arp_ip_target": [
 "10.0.0.1[10/20]",
 "10.0.0.2[10/20]"
],
...

Changes since V2: (bond_print_opt() only)
Based on suggestions from Stephen Hemminger.
1) Removed inline from bond_vlan_tags_parse().
2) Switched to print_color_string() from print_string()
3) Follow kernel style.
4) Fixed JSON output.


Changes since V1:
Updates to support ip link show <bonding-device>.

This change is dependent on this bonding driver patch set:
https://marc.info/?l=linux-netdev&m=175684731919992&w=2

Merge only after the above patch set has been merged.

Thank you for your time and reviews.

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

David Wilder (1):
  iproute: Extend bonding's "arp_ip_target" parameter to add vlan tags.

 ip/iplink_bond.c | 149 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 123 insertions(+), 26 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH iproute2-next v7 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add vlan tags.
  2025-10-13 23:57 [PATCH iproute2-next v7 0/1] iproute2-next: Extending bonding's arp_ip_target to include a list of vlan tags David Wilder
@ 2025-10-13 23:57 ` David Wilder
  2025-10-16 15:32   ` David Ahern
  0 siblings, 1 reply; 3+ messages in thread
From: David Wilder @ 2025-10-13 23:57 UTC (permalink / raw)
  To: netdev; +Cc: jv, wilder, pradeep, i.maximets, amorenoz, haliu, stephen,
	dsahern

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]

The inclusion of the list of vlan tags is optional. The new logic
preserves both forward and backward compatibility with the kernel
and iproute2 versions.

Signed-off-by: David Wilder <wilder@us.ibm.com>
---
 ip/iplink_bond.c | 149 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 123 insertions(+), 26 deletions(-)

diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index d6960f6d..0f34125d 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -173,6 +173,55 @@ static void explain(void)
 	print_explain(stderr);
 }
 
+#define BOND_VLAN_PROTO_NONE htons(0xffff)
+#define BOND_MAX_VLAN_TAGS 5
+#define VLAN_VID_MASK 0x0fff
+
+struct bond_vlan_tag {
+	__be16  vlan_proto;
+	__be16  vlan_id;
+};
+
+static 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 (level > BOND_MAX_VLAN_TAGS) {
+		fprintf(stderr, "Error: Too many vlan tags specified, maximum is %d.\n",
+			BOND_MAX_VLAN_TAGS);
+		exit(1);
+	}
+
+	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 >= VLAN_VID_MASK) {
+			fprintf(stderr, "Error: Invalid vlan_id specified: %hu\n",
+				tags[level].vlan_id);
+			exit(1);
+		}
+
+		return tags;
+	}
+
+	return NULL;
+}
+
 static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
@@ -239,12 +288,28 @@ 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);
-
-					addattr32(n, 1024, i, addr);
+					struct {
+						__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);
+					}
 					target = strtok(NULL, ",");
 				}
 				addattr_nest_end(n, nest);
@@ -429,6 +494,22 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 	return 0;
 }
 
+static void bond_vlan_tags_print(const struct bond_vlan_tag *vlan)
+{
+	for (unsigned int l = 0; l < BOND_MAX_VLAN_TAGS + 1; l++, vlan++) {
+		if (vlan->vlan_proto == BOND_VLAN_PROTO_NONE)
+			return;
+
+		if (l > 0)
+			print_string(PRINT_FP, NULL, "/", NULL);
+
+		print_uint(PRINT_ANY, NULL, "%u", vlan->vlan_id);
+	}
+
+	fprintf(stderr, "Internal Error: too many vlan tags.\n");
+	exit(1);
+}
+
 static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	int i;
@@ -499,24 +580,44 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_BOND_ARP_IP_TARGET]) {
 		struct rtattr *iptb[BOND_MAX_ARP_TARGETS + 1];
 
-		parse_rtattr_nested(iptb, BOND_MAX_ARP_TARGETS,
-				    tb[IFLA_BOND_ARP_IP_TARGET]);
+		parse_rtattr_nested(iptb, BOND_MAX_ARP_TARGETS, tb[IFLA_BOND_ARP_IP_TARGET]);
 
 		if (iptb[0]) {
 			open_json_array(PRINT_JSON, "arp_ip_target");
 			print_string(PRINT_FP, NULL, "arp_ip_target ", NULL);
 		}
 
-		for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
-			if (iptb[i])
-				print_string(PRINT_ANY,
-					     NULL,
-					     "%s",
-					     rt_addr_n2a_rta(AF_INET, iptb[i]));
-			if (!is_json_context()
-			    && i < BOND_MAX_ARP_TARGETS-1
-			    && iptb[i+1])
-				fprintf(f, ",");
+		for (unsigned int i = 0; i < BOND_MAX_ARP_TARGETS && iptb[i]; i++) {
+			struct {
+				__u32 addr;
+				struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
+			} data;
+
+			if (RTA_PAYLOAD(iptb[i]) < sizeof(data.addr) ||
+				RTA_PAYLOAD(iptb[i]) > sizeof(data)) {
+				fprintf(stderr, "Internal Error: Bad payload for arp_ip_target.\n");
+				exit(1);
+			}
+			memcpy(&data, RTA_DATA(iptb[i]), RTA_PAYLOAD(iptb[i]));
+
+			open_json_object(NULL);
+			print_color_string(PRINT_ANY, COLOR_INET, "addr", "%s",
+					   rt_addr_n2a(AF_INET, sizeof(data.addr), &data.addr));
+
+			if (RTA_PAYLOAD(iptb[i]) > sizeof(data.addr)) {
+				open_json_array(PRINT_JSON, "vlan");
+				print_string(PRINT_FP, NULL, "[", NULL);
+
+				bond_vlan_tags_print(data.vlans);
+
+				close_json_array(PRINT_JSON, NULL);
+				print_string(PRINT_FP, NULL, "]", NULL);
+			}
+
+			if (i < BOND_MAX_ARP_TARGETS - 1 && iptb[i+1])
+				print_string(PRINT_FP, NULL, ",", NULL);
+
+			close_json_object();
 		}
 
 		if (iptb[0]) {
@@ -528,8 +629,7 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_BOND_NS_IP6_TARGET]) {
 		struct rtattr *ip6tb[BOND_MAX_NS_TARGETS + 1];
 
-		parse_rtattr_nested(ip6tb, BOND_MAX_NS_TARGETS,
-				    tb[IFLA_BOND_NS_IP6_TARGET]);
+		parse_rtattr_nested(ip6tb, BOND_MAX_NS_TARGETS, tb[IFLA_BOND_NS_IP6_TARGET]);
 
 		if (ip6tb[0]) {
 			open_json_array(PRINT_JSON, "ns_ip6_target");
@@ -538,14 +638,11 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 		for (i = 0; i < BOND_MAX_NS_TARGETS; i++) {
 			if (ip6tb[i])
-				print_string(PRINT_ANY,
-					     NULL,
-					     "%s",
-					     rt_addr_n2a_rta(AF_INET6, ip6tb[i]));
-			if (!is_json_context()
-			    && i < BOND_MAX_NS_TARGETS-1
-			    && ip6tb[i+1])
-				fprintf(f, ",");
+				print_color_string(PRINT_ANY, COLOR_INET6, NULL, "%s",
+						   rt_addr_n2a_rta(AF_INET6, ip6tb[i]));
+
+			if (i < BOND_MAX_NS_TARGETS - 1 && ip6tb[i+1])
+				print_string(PRINT_FP, NULL, ",", NULL);
 		}
 
 		if (ip6tb[0]) {
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH iproute2-next v7 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add vlan tags.
  2025-10-13 23:57 ` [PATCH iproute2-next v7 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add " David Wilder
@ 2025-10-16 15:32   ` David Ahern
  0 siblings, 0 replies; 3+ messages in thread
From: David Ahern @ 2025-10-16 15:32 UTC (permalink / raw)
  To: David Wilder, netdev; +Cc: jv, pradeep, i.maximets, amorenoz, haliu, stephen

On 10/13/25 5:57 PM, David Wilder wrote:
> +static struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int level, int *size)

iproute2 follows netdev wrt coding standards. Please run checkpatch and
fixup long lines -- max ~80 columns. Column width in the low 80s is fine
if it improves readability versus splitting the line.

Applying: iproute: Extend bonding's "arp_ip_target" parameter to add
vlan tags.
WARNING: line length of 88 exceeds 80 columns
#18: FILE: ip/iplink_bond.c:188:
+static struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int
level, int *size)

WARNING: line length of 88 exceeds 80 columns
#25: FILE: ip/iplink_bond.c:195:
+		fprintf(stderr, "Error: Too many vlan tags specified, maximum is %d.\n",

WARNING: line length of 96 exceeds 80 columns
#46: FILE: ip/iplink_bond.c:216:
+		if (n != 1 || tags[level].vlan_id < 1 || tags[level].vlan_id >=
VLAN_VID_MASK) {

...

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-10-16 15:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 23:57 [PATCH iproute2-next v7 0/1] iproute2-next: Extending bonding's arp_ip_target to include a list of vlan tags David Wilder
2025-10-13 23:57 ` [PATCH iproute2-next v7 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add " David Wilder
2025-10-16 15:32   ` David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).