netdev - Re: [PATCH net-next 1/4] net: bonding: add broadcast

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: <ea87b2d2-9b17-4f16-9e40-fe7212f2788d@lunn.ch>
Date: Sun, 11 May 2025 17:53:36 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Tonghao Zhang <tonghao@...aicloud.com>
Cc: Jay Vosburgh <jv@...sburgh.net>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
	Andrew Lunn <andrew+netdev@...n.ch>
Subject: Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option
 for 802.3ad

> static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>                                                   struct sk_buff *skb)
> {
>         if (!bond->params.broadcast_neighbor ||
>             BOND_MODE(bond) != BOND_MODE_8023AD)
>                 return false;

I think you missed the point. You have added these two tests to every
packet on the fast path. And it is very likely to return false. Is
bond.params.broadcast_neighbor likely to be in the cache? A cache miss
is expensive. Is bond.params.mode also likely to be in cache? You
placed broadcast_neighbor at the end of params, so it is unlikely to
be in the same cache line as bond.params.mode. So two cache misses.

What Jay would like is that the cost on the fast path is ~0 for when
this feature is not in use. Jump labels can achieve this. It inserts
either a NOP or a jump instruction, which costs nearly nothing, and
then uses self modifying code to swap between a NOP or a jump. You can
keep a global view of is any bond is using this new mode? If no, this
test is eliminated. If yes, you do the test.

	Andrew