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]

Message-ID: <ZoOzge5Xn42QtG91@Laptop-X1>
Date: Tue, 2 Jul 2024 16:00:01 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Jay Vosburgh <jay.vosburgh@...onical.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

On Fri, Jun 28, 2024 at 04:36:17PM -0700, Jay Vosburgh wrote:
> >Good question. The monitor application want a more elegant/general way
> >to deal with the LACP state and do other network reconfigurations.
> >Here is the requirement I got from customer.
> >
> >1) As a server administrator, I want ip monitor to show state change events
> >   related to LACP bonds so that I can react quickly to network reconfigurations.
> >2) As a network monitoring application developer, I want my application to be
> >   notified about LACP bond operational state changes without having to
> >   poll /proc/net/bonding/<bond> and parse its output so that it can trigger
> >   predefined failover remediation policies.
> >3) As a server administrator, I want my LACP bond monitoring application to
> >   receive a Netlink-based notification whenever the number of member
> >   interfaces is reduced so that the operations support system can provision
> >   a member interface replacement.
> 
> 	What notifications are they not getting that they want?  Or is
> it that events happen but lack some information they want?

Yes, e.g. when the switch crashed while link still up, they can get notified
from LACP partner state if we send the info via netlink message.

> 
> 	Currently, the end of bond_3ad_state_machine_handler() will send
> notifications via bond_slave_state_notify() if the state of any member
> of the bond has changed (via the struct slave.should_notify field).
> 
> 	The notifications here come from bond_lower_state_changed(),
> which propagates link up/down and tx_enabled (active-ness), but not any
> LACP specifics.  These are sent as NETDEV_CHANGELOWERSTATE events, which
> rtnetlink_event() handles, so they should be visible to ip monitor.

That's the problem. It only propagates link info via link up/down. The link
info is not send if only LACP state changed.

> 
> >What I understand is the user/admin need to know the latest stable state so
> >they can do some other network configuration based on the status. Losing
> >a middle state notification during fast changes is acceptable.
> 
> 	This is a much simpler problem to solve.

Good.

> 
> >> 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. :)
> >
> >Would you please help explain why we may not get the latest state? From what
> >I understand:
> >
> >1) State A -> B, queue notify
> >       rtnl_trylock, fail, queue again
> >2) State B -> C, queue notify
> >      rtnl_trylock, success, post current state C
> >3) State C -> D, queue notify
> >      rtnl_trylock, fail, queue again
> >4) State D -> A, queue notify
> >      rtnl_trylock, fail, queue again
> >      rtnl_trylock, fail, queue again
> >      rtnl_trylock, success, post current state A
> >
> >So how could the step 3) state send but step 4) state not send?
> 
> 	I'm going to speculate here that the scenario envisioned would
> be something like CPU A is in the midst of generating an event at the
> end of the state machine, but CPU B could be processing a LACPDU
> simultaneously-ish.  The CPU A event is sent, but the CPU B event is
> delayed due to RTNL contention.  In this scenario, the last event seen
> in user space is CPU A, but the actual state has moved on to that set by
> CPU B, whose notification will be received eventually.

Is is possible that LACP event (The second to last state) in CPU A is delayed
due to RTNL contention. While the LACP event (the latest state) in CPU B is
sent successfully. And later the LACP event in CPU A is sent eventually.

This would case the user space receives miss order message.

> 	Looking at the current notifications in bonding, I wonder if it
> would be sufficient to add the desired information to what
> bond_lower_state_changed() sends, rather than trying to shoehorn in
> another rtnl_trylock() gizmo.

I'm not sure if the LACP state count for lower state. What do you think of
my previous draft patch[1] that replied to you.

[1] https://lore.kernel.org/netdev/Zn0iI3SPdRkmfnS1@Laptop-X1/

Thanks
Hangbin