| 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:
<MW3PR15MB39133DDF8FCF1FED3798DC0BFA43A@MW3PR15MB3913.namprd15.prod.outlook.com>
Date: Thu, 3 Jul 2025 18:03:02 +0000
From: David Wilder <wilder@...ibm.com>
To: Jay Vosburgh <jv@...sburgh.net>
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.
________________________________________
From: Jay Vosburgh <jv@...sburgh.net>
Sent: Wednesday, July 2, 2025 7:04 PM
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.
>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.
802.1p should be ok, the tpid is the same just an added priority control field.
Could not the priority (PCP) in a 802.1p be different between
flows to the same target? In this case we are only defining the tag for
arp requests the default priority 0 should be ok.
However, Do we need to support other tag types? For example 802.1ad?
I would like to avoided making the configuration complicated with too many
options.
>
> 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.
I will add a limit. The existing implementation has no limit,
although bond_arp_send() gets the order wrong with more than two vlan headers.
That's a bug I hoped to look at. I found I could workaround that by specifying
the vlan tags in the wrong order :)
David Wilder
>
> -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
Powered by blists - more mailing lists