netdev - RE: [PATCH net-next v13 6/7] bonding: Update for extended 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:
 <MW3PR15MB39130C39ACD2718F927B725EFAF2A@MW3PR15MB3913.namprd15.prod.outlook.com>
Date: Tue, 21 Oct 2025 16:18:21 +0000
From: David Wilder <wilder@...ibm.com>
To: Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jv@...sburgh.net"
	<jv@...sburgh.net>,
        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>,
        "horms@...nel.org"
	<horms@...nel.org>,
        "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Nikolay Aleksandrov
	<razor@...ckwall.org>
Subject: RE: [PATCH net-next v13 6/7] bonding: Update for extended
 arp_ip_target format.




________________________________________
From: David Wilder <wilder@...ibm.com>
Sent: Tuesday, October 21, 2025 9:00 AM
To: Jakub Kicinski; Paolo Abeni
Cc: netdev@...r.kernel.org; jv@...sburgh.net; Pradeep Satyanarayana; i.maximets@....org; Adrian Moreno Zapata; Hangbin Liu; stephen@...workplumber.org; horms@...nel.org; andrew+netdev@...n.ch; edumazet@...gle.com; Nikolay Aleksandrov
Subject: Re: [EXTERNAL] Re: [PATCH net-next v13 6/7] bonding: Update for extended arp_ip_target format.



>> On Thu, 16 Oct 2025 13:50:52 +0200 Paolo Abeni wrote:
>> > > +           if (nla_put(skb, i, size, &data))
>> > > +                   goto nla_put_failure;
>> > >     }
>> > >
>> > >     if (targets_added)
>> >
>> > I guess you should update bond_get_size() accordingly???
>> >
>> > Also changing the binary layout of an existing NL type does not feel
>> > safe. @Jakub: is that something we can safely allow?
>>
>> In general extending attributes is fine, but going from a scalar
>> to a struct is questionable. YNL for example will not allow it.

> I am not sure I understand your concern. I have change the
> netlink socket payload from a fixed 4 bytes to a variable number of bytes.
> 4 bytes for ipv4 address followed by some number of bytes with the
> list of vlans, could be zero. Netlink sockets just need to be told the
> payload size.  Or have I missed the point?

> Is the concern here the variable size of the payload?
>
> I have updated bond_get_size() to use the maximum size of the payload so the payload allocation should correct :

** Correction: 
+struct bond_arp_ip_target_payload {
+       __be32 addr;
+       struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
+} __packed;
+
                                                /* IFLA_BOND_ARP_IP_TARGET */
                nla_total_size(sizeof(struct nlattr)) +
-               nla_total_size(sizeof(u32)) * BOND_MAX_ARP_TARGETS +
+               nla_total_size(sizeof(struct bond_arp_ip_target_payload)) * BOND_MAX_ARP_TARGETS +

>>>
>>> I haven't looked at the series more closely until now.
>>>
>>> Why are there multiple vlan tags per target?
>>
>> You can have a vlan inside a vlan, the original arp_ip_target
>> option code supported this.
>>
>>>
>>> Is this configuration really something we should support in the kernel?
>>> IDK how much we should push "OvS-compatibility" into other parts of the
>>> stack. If user knows that they have to apply this funny configuration
>>> on the bond maybe they should just arp from user space?
>
>> This change is not just for compatibility with OVS. Ilya Maximets pointed out:
>> "..this is true not only for OVS.  You can add various TC qdiscs onto the
>> interface that will break all those assumptions as well, for example.  Loaded
>> BPF/XDP programs will too."
>
>> When using the arp_ip_target option the bond driver must discover what
>> vlans are in the path to the target. These special arps must be generated by
>> the bonding driver to know what bonded slave the packets is to sent and
>> received on and at what frequency.
>>
>> When the the arp_ip_target feature was first introduced discovering vlans in the
>> path to the target was easy by following the linked net_devices. As our
>> networking code has evolved this is no longer possible with all configurations
>> as Ilya pointed out.  What I have done is provide alternate way to provide the
>> list of vlans so this desirable feature can continue to function.
>
> Regards
>  David Wilder