| 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
| ||
|
Message-ID: <188180.1752018241@famine>
Date: Tue, 08 Jul 2025 16:44:01 -0700
From: Jay Vosburgh <jv@...sburgh.net>
To: David Wilder <wilder@...ibm.com>
cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"pradeeps@...ux.vnet.ibm.com" <pradeeps@...ux.vnet.ibm.com>,
Pradeep Satyanarayana <pradeep@...ibm.com>,
"i.maximets@....org" <i.maximets@....org>,
Adrian Moreno Zapata <amorenoz@...hat.com>,
Hangbin Liu <haliu@...hat.com>,
"stephen@...workplumber.org" <stephen@...workplumber.org>,
"dsahern@...il.com" <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:
>________________________________________
>From: Jay Vosburgh <jv@...sburgh.net>
>Sent: Tuesday, July 8, 2025 9:51 AM
>To: David Wilder
>Cc: netdev@...r.kernel.org; pradeeps@...ux.vnet.ibm.com; Pradeep Satyanarayana; i.maximets@....org; Adrian Moreno Zapata; Hangbin Liu; stephen@...workplumber.org; dsahern@...il.com
>Subject: [EXTERNAL] Re: [PATCH iproute2-next v1 1/1] iproute: Extend bonding's "arp_ip_target" parameter to add vlan tags.
>
>
>>>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;
>>>+
>>>+ 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);
>
>Answering your last question first, IFLA_BOND_ARP_IP_TARGET was
>already NLA_NESTED (see: bond_netlink.c).
>
>>
>> Another question occurred to me: is this method for sending the
>>VLAN tags going to break compatibility? New versions of iproute2 need
>>to work on older kernels, so we can't simply change existing APIs in
>>ways that require a lockstep change of iproute versions (going either
>>forwards or backwards).
>
>I manage to preserve compatibility forward and backward. Both a new
>kernel and old kernel will work with both an new and old iproute2.
>
>> The above looks like it changes the structure being conveyed
>>into the kernel, which I don't think we can do. In the kernel, the old
>>API will need to continue to function, and therefore the new "with VLAN
>>tag" case will need to use a new API.
>
>I thought adding a new API was what we wanted to avoid. But a new API
>is unnecessary as I am extending the existing one in such a way to not
>break the original api.
>
>The original code sent 4 bytes of data (the 32bit ip address) in each nested
>entry. The new code sends the same 4 bytes containing the ip address.
>If a list of tags has been included the data is appended to the 4byte address.
>Type NLA_NESTED has no fixed data size. If no vlans are supplied then the
>data sent to the kernel looks exactly the same as with the old iproute2.
>Therefor a new kernel will continue to work with the old iproute2 command,
>you just cant add vlan tags until you upgrade iproute2.
>
>An old kernel will continue to work with a new iproute2. The kernel will be
>sent the same 4Byte address in each nested entry. If you try to add a list
>of vlan tags the kernel ignores the extra data.
>
>I tested the various combinations.
Excellent, I stand corrected. Could you include the above in
the commit message somewhere? Doesn't necessarily need that amount of
detail, just a mention that the new logic is compatible in the proper
ways.
-J
>>
>> The existing IFLA_BOND_ARP_IP_TARGET type doesn't use nested
>>netlink types, it just sends binary data, so I'm thinking we can't just
>>change that binary data, and would need a new IFLA_BOND_ type.
>>
>> -J
>>
>>> target = strtok(NULL, ",");
>>> }
>>> addattr_nest_end(n, nest);
>>>--
>>>2.43.5
>>>
>>
>>---
>> -Jay Vosburgh, jv@...sburgh.net
>
>David Wilder wilder@...ibm.com
---
-Jay Vosburgh, jv@...sburgh.net
Powered by blists - more mailing lists