From: Jakub Kicinski <kuba@kernel•org>
To: David Wilder <wilder@us•ibm.com>
Cc: Paolo Abeni <pabeni@redhat•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"jv@jvosburgh•net" <jv@jvosburgh•net>,
Pradeep Satyanarayana <pradeep@us•ibm.com>,
"i.maximets@ovn•org" <i.maximets@ovn•org>,
Adrian Moreno Zapata <amorenoz@redhat•com>,
Hangbin Liu <haliu@redhat•com>,
"stephen@networkplumber•org" <stephen@networkplumber•org>,
"horms@kernel•org" <horms@kernel•org>,
"andrew+netdev@lunn•ch" <andrew+netdev@lunn•ch>,
"edumazet@google•com" <edumazet@google•com>,
Nikolay Aleksandrov <razor@blackwall•org>
Subject: Re: [PATCH net-next v13 6/7] bonding: Update for extended arp_ip_target format.
Date: Tue, 21 Oct 2025 15:56:28 -0700 [thread overview]
Message-ID: <20251021155628.7383bfca@kernel.org> (raw)
In-Reply-To: <MW3PR15MB3913E83123930C417DDD1AC8FAE9A@MW3PR15MB3913.namprd15.prod.outlook.com>
On Fri, 17 Oct 2025 00:21:02 +0000 David Wilder wrote:
> > > 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?
Are you replacing a line that says nla_put() which outputs raw bytes
or a line which says nla_put_x32() which outputs a scalar?
What I'm saying is that while growing raw byte attrs is pretty common
in Netlink, replacing a scalar with a struct may cause user space
to reject the attrs.
> > 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.
I understand your perspective. I'm not convinced that kernel must
support such custom configurations, if it can't infer the correct
behavior from information it already has.
I don't feel strongly about it, if you manage to collect a review
tag from the bonding maintainers or another netdev maintainer I won't
stand in the way. Otherwise, given that the uAPI is questionable and
there's total of 0 review tags on v13, this series is starting to look
like a dead end.
next prev parent reply other threads:[~2025-10-21 22:56 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-13 23:52 [PATCH net-next v13 0/7] bonding: Extend arp_ip_target format to allow for a list of vlan tags David Wilder
2025-10-13 23:52 ` [PATCH net-next v13 1/7] bonding: Adding struct bond_arp_target David Wilder
2025-10-13 23:52 ` [PATCH net-next v13 2/7] bonding: Adding extra_len field to struct bond_opt_value David Wilder
2025-10-13 23:52 ` [PATCH net-next v13 3/7] bonding: arp_ip_target helpers David Wilder
2025-10-16 11:14 ` Paolo Abeni
2025-10-13 23:52 ` [PATCH net-next v13 4/7] bonding: Processing extended arp_ip_target from user space David Wilder
2025-10-16 11:26 ` Paolo Abeni
2025-10-13 23:52 ` [PATCH net-next v13 5/7] bonding: Update to bond_arp_send_all() to use supplied vlan tags David Wilder
2025-10-16 11:38 ` Paolo Abeni
2025-10-13 23:52 ` [PATCH net-next v13 6/7] bonding: Update for extended arp_ip_target format David Wilder
2025-10-16 11:50 ` Paolo Abeni
2025-10-16 19:49 ` Jakub Kicinski
2025-10-17 0:21 ` David Wilder
2025-10-21 16:00 ` David Wilder
2025-10-21 16:18 ` David Wilder
2025-10-21 22:56 ` Jakub Kicinski [this message]
2025-10-13 23:52 ` [PATCH net-next v13 7/7] bonding: Selftest and documentation for the arp_ip_target parameter David Wilder
2025-10-16 11:55 ` Paolo Abeni
2025-10-16 11:48 ` [PATCH net-next v13 0/7] bonding: Extend arp_ip_target format to allow for a list of vlan tags Nikolay Aleksandrov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251021155628.7383bfca@kernel.org \
--to=kuba@kernel$(echo .)org \
--cc=amorenoz@redhat$(echo .)com \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=edumazet@google$(echo .)com \
--cc=haliu@redhat$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=i.maximets@ovn$(echo .)org \
--cc=jv@jvosburgh$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=pradeep@us$(echo .)ibm.com \
--cc=razor@blackwall$(echo .)org \
--cc=stephen@networkplumber$(echo .)org \
--cc=wilder@us$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox