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: <Zn05dMVVlUmeypas@Laptop-X1>
Date: Thu, 27 Jun 2024 18:05:40 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>
Cc: Jay Vosburgh <jay.vosburgh@...onical.com>,
	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 Thu, Jun 27, 2024 at 11:29:21AM +0300, Nikolay Aleksandrov wrote:
> On 27/06/2024 11:26, Hangbin Liu wrote:
> > On Wed, Jun 26, 2024 at 05:06:00PM -0700, Jay Vosburgh wrote:
> >>> Hits:
> >>>
> >>> RTNL: assertion failed at net/core/rtnetlink.c (1823)
> > 
> > Thanks for this hits...
> > 
> >>>
> >>> On two selftests. Please run the selftests on a debug kernel..
> > 
> > OK, I will try run my tests on debug kernel in future.
> > 
> >>
> >> 	Oh, I forgot about needing RTNL.
> >>
> 
> +1 & facepalm, completely forgot it was running without rtnl
> 
> >> 	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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ