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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <B43CC0DC-286B-44FF-8FA8-1B1BC0C990BF@bamaicloud.com>
Date: Mon, 12 May 2025 10:25:16 +0800
From: Tonghao Zhang <tonghao@...aicloud.com>
To: Andrew Lunn <andrew@...n.ch>
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



> 2025年5月11日 下午11:53，Andrew Lunn <andrew@...n.ch> 写道：
> 
>> 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
No, no mode uses jump labels instead of bond.params checking.
> test is eliminated. If yes, you do the test.
I test the lacp mode with broadcast_neighbor enabled, there is no performance drop. This patch has been running in our production environment for a long time. We only use this option in lacp mode, for performance, the code can be modified as follows:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ce31445e85b6..8743bf007b7e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5330,11 +5330,12 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
        return slaves->arr[hash % count];
 }

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

        if (skb->protocol == htons(ETH_P_ARP))
@@ -5408,9 +5409,6 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
        struct bond_up_slave *slaves;
        struct slave *slave;

-       if (bond_should_broadcast_neighbor(bond, skb))
-               return bond_xmit_broadcast(skb, dev);
-
        slaves = rcu_dereference(bond->usable_slaves);
        slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
        if (likely(slave))
@@ -5625,6 +5623,9 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
        case BOND_MODE_ACTIVEBACKUP:
                return bond_xmit_activebackup(skb, dev);
        case BOND_MODE_8023AD:
+               if (bond_should_broadcast_neighbor(skb, dev))
+                       return bond_xmit_broadcast(skb, dev);
+               fallthrough;
        case BOND_MODE_XOR:
                return bond_3ad_xor_xmit(skb, dev);
        case BOND_MODE_BROADCAST:
> 
> 	Andrew
> 
>