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: Fri, 28 Jun 2024 10:22:25 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Hangbin Liu <liuhangbin@...il.com>,
 Jay Vosburgh <jay.vosburgh@...onical.com>
Cc: 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

On 28/06/2024 10:04, Nikolay Aleksandrov wrote:
> On 28/06/2024 06:10, Hangbin Liu wrote:
>> On Thu, Jun 27, 2024 at 07:24:10AM -0700, Jay Vosburgh wrote:
>>> Hangbin Liu <liuhangbin@...il.com> wrote:
>>>> 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
>>
>> Ah, thanks, I didn't notice this. I will also enable lacp notify
>> in ad_rx_machine().
>>
>>> 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.
>>
>> Hmm... This looks too complex. If we store all the states. A frequent flashing
>> may consume the memory. If we made a limit for the queue, we may still loosing
>> some state changes.
>>
>> I'm not sure which way is better.
>>
>>>
>>>>> 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.
>>
> 
> Actually I was talking about:
>  /sys/class/net/<bond port>/bonding_slave/ad_actor_oper_port_state
>  /sys/class/net/<bond port>/bonding_slave/ad_partner_oper_port_state
> etc
> 
> Wouldn't these work for you?
> 

But it gets much more complicated, I guess it will be easier to read the
proc bond file with all the LACP information. That is under RCU only as
well.

>> OK. There are 2 scenarios I got.
>>
>> 1) the local user want to get the local/partner state and make sure not
>> send pkts before they are in DISTRIBUTING state to avoid pkts drop, Or vice
>> versa. Only checking link operstate or up/down status is not enough.
>>
>> 2) the admin want to get the switch/partner status via LACP status incase
>> the switch is crashed.
>>
>> Do you have any suggestion for the implementation?
>>
>> Thanks
>> Hangbin
>