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

On Thu, Jun 27, 2024 at 11:29:21AM +0300, Nikolay Aleksandrov wrote:

On 27/06/2024 11:26, Hangbin Liu wrote:

quoted

On Wed, Jun 26, 2024 at 05:06:00PM -0700, Jay Vosburgh wrote:

quoted

quoted

Hits:

RTNL: assertion failed at net/core/rtnetlink.c (1823)

Thanks for this hits...

quoted

quoted

On two selftests. Please run the selftests on a debug kernel..

OK, I will try run my tests on debug kernel in future.

quoted

	Oh, I forgot about needing RTNL.

+1 & facepalm, completely forgot it was running without rtnl

quoted

quoted

	We cannot simply acquire RTNL in ad_mux_machine(), as the
bond->mode_lock is already held, and the lock ordering must be RTNL
first, then mode_lock, lest we deadlock.

	Hangbin, I'd suggest you look at how bond_netdev_notify_work()
complies with the lock ordering (basically, doing the actual work out of
line in a workqueue event), or how the "should_notify" flag is used in
bond_3ad_state_machine_handler().  The first is more complicated, but
won't skip events; the second may miss intermediate state transitions if
it cannot acquire RTNL and has to delay the notification.

I think the administer doesn't want to loose the state change info. So how
about something like:

You can (and will) miss events with the below code. It is kind of best effort,
but if the notification is not run before the next state change, you will
lose the intermediate changes.

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();
	}

Thanks
Hangbin