netdev - Re: [PATCHv3 net-next] bonding: 3ad: send ifinfo notify when mux state changed

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]

Date: Thu, 27 Jun 2024 07:24:10 -0700
From: Jay Vosburgh <jay.vosburgh@...onical.com>
To: Hangbin Liu <liuhangbin@...il.com>
cc: Nikolay Aleksandrov <razor@...ckwall.org>,
    Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
    Andy Gospodarek <andy@...yhouse.net>,
    "David S. Miller" <davem@...emloft.net>,
    Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
    Ido Schimmel <idosch@...dia.com>, Jiri Pirko <jiri@...nulli.us>,
    Amit Cohen <amcohen@...dia.com>
Subject: Re: [PATCHv3 net-next] bonding: 3ad: send ifinfo notify when mux
 state changed

Hangbin Liu <liuhangbin@...il.com> wrote:

>On Thu, Jun 27, 2024 at 01:33:10PM +0300, Nikolay Aleksandrov wrote:
>> > Yes, but at least the admin could get the latest state. With the following
>> > code the admin may not get the latest update if lock rtnl failed.
>> > 
>> >         if (should_notify_rtnl && rtnl_trylock()) {
>> >                 bond_slave_lacp_notify(bond);
>> >                 rtnl_unlock();
>> > 	}
>> > 
>> Well, you mentioned administrators want to see the state changes, please
>> better clarify the exact end goal. Note that technically may even not be
>> the last state as the state change itself happens in parallel (different
>> locks) and any update could be delayed depending on rtnl availability
>> and workqueue re-scheduling. But sure, they will get some update at some point. :)
>
>Ah.. Yes, that's a sad fact :(

	There are basically two paths that will change the LACP state
that's passed up via netlink (the aggregator ID, and actor and partner
oper port states): bond_3ad_state_machine_handler(), or incoming
LACPDUs, which call into ad_rx_machine().  Administrative changes to the
bond will do it too, like adding or removing interfaces, but those
originate in user space and aren't happening asynchronously.

	If you want (almost) absolute reliability in communicating every
state change for the state machine and LACPDU processing, I think you'd
have to (a) create an object with the changed state, (b) queue it
somewhere, then (c) call a workqueue event to process that queue out of
line.

>> It all depends on what are the requirements.
>> 
>> An uglier but lockless alternative would be to poll the slave's sysfs oper state,
>> that doesn't require any locks and would be up-to-date.
>
>Hmm, that's a workaround, but the admin need to poll the state frequently as
>they don't know when the state will change.
>
>Hi Jay, are you OK to add this sysfs in bonding?

	I think what Nik is proposing is for your userspace to poll the
/sys/class/net/${DEV}/operstate.

	-J

---
	-Jay Vosburgh, jay.vosburgh@...onical.com