Patch "net: bridge: switchdev: don't notify FDB entries with "master dynamic"" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    net: bridge: switchdev: don't notify FDB entries with "master dynamic"

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     net-bridge-switchdev-don-t-notify-fdb-entries-with-m.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit d4f598102bb9702aa0f1c5aafbc47c7046bb53cf
Author: Vladimir Oltean <vladimir.oltean@xxxxxxx>
Date:   Tue Apr 18 18:59:02 2023 +0300

    net: bridge: switchdev: don't notify FDB entries with "master dynamic"
    
    [ Upstream commit 927cdea5d2095287ddd5246e5aa68eb5d68db2be ]
    
    There is a structural problem in switchdev, where the flag bits in
    struct switchdev_notifier_fdb_info (added_by_user, is_local etc) only
    represent a simplified / denatured view of what's in struct
    net_bridge_fdb_entry :: flags (BR_FDB_ADDED_BY_USER, BR_FDB_LOCAL etc).
    Each time we want to pass more information about struct
    net_bridge_fdb_entry :: flags to struct switchdev_notifier_fdb_info
    (here, BR_FDB_STATIC), we find that FDB entries were already notified to
    switchdev with no regard to this flag, and thus, switchdev drivers had
    no indication whether the notified entries were static or not.
    
    For example, this command:
    
    ip link add br0 type bridge && ip link set swp0 master br0
    bridge fdb add dev swp0 00:01:02:03:04:05 master dynamic
    
    has never worked as intended with switchdev. It causes a struct
    net_bridge_fdb_entry to be passed to br_switchdev_fdb_notify() which has
    a single flag set: BR_FDB_ADDED_BY_USER.
    
    This is further passed to the switchdev notifier chain, where interested
    drivers have no choice but to assume this is a static (does not age) and
    sticky (does not migrate) FDB entry. So currently, all drivers offload
    it to hardware as such, as can be seen below ("offload" is set).
    
    bridge fdb get 00:01:02:03:04:05 dev swp0 master
    00:01:02:03:04:05 dev swp0 offload master br0
    
    The software FDB entry expires $ageing_time centiseconds after the
    kernel last sees a packet with this MAC SA, and the bridge notifies its
    deletion as well, so it eventually disappears from hardware too.
    
    This is a problem, because it is actually desirable to start offloading
    "master dynamic" FDB entries correctly - they should expire $ageing_time
    centiseconds after the *hardware* port last sees a packet with this
    MAC SA - and this is how the current incorrect behavior was discovered.
    With an offloaded data plane, it can be expected that software only sees
    exception path packets, so an otherwise active dynamic FDB entry would
    be aged out by software sooner than it should.
    
    With the change in place, these FDB entries are no longer offloaded:
    
    bridge fdb get 00:01:02:03:04:05 dev swp0 master
    00:01:02:03:04:05 dev swp0 master br0
    
    and this also constitutes a better way (assuming a backport to stable
    kernels) for user space to determine whether the kernel has the
    capability of doing something sane with these or not.
    
    As opposed to "master dynamic" FDB entries, on the current behavior of
    which no one currently depends on (which can be deduced from the lack of
    kselftests), Ido Schimmel explains that entries with the "extern_learn"
    flag (BR_FDB_ADDED_BY_EXT_LEARN) should still be notified to switchdev,
    since the spectrum driver listens to them (and this is kind of okay,
    because although they are treated identically to "static", they are
    expected to not age, and to roam).
    
    Fixes: 6b26b51b1d13 ("net: bridge: Add support for notifying devices about FDB add/del")
    Link: https://lore.kernel.org/netdev/20230327115206.jk5q5l753aoelwus@skbuf/
    Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
    Reviewed-by: Jesse Brandeburg <jesse.brandeburg@xxxxxxxxx>
    Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>
    Tested-by: Ido Schimmel <idosch@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20230418155902.898627-1-vladimir.oltean@xxxxxxx
    Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 8f3d76c751dd0..4b3982c368b35 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -146,6 +146,17 @@ br_switchdev_fdb_notify(struct net_bridge *br,
 {
 	struct switchdev_notifier_fdb_info item;
 
+	/* Entries with these flags were created using ndm_state == NUD_REACHABLE,
+	 * ndm_flags == NTF_MASTER( | NTF_STICKY), ext_flags == 0 by something
+	 * equivalent to 'bridge fdb add ... master dynamic (sticky)'.
+	 * Drivers don't know how to deal with these, so don't notify them to
+	 * avoid confusing them.
+	 */
+	if (test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags) &&
+	    !test_bit(BR_FDB_STATIC, &fdb->flags) &&
+	    !test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags))
+		return;
+
 	br_switchdev_fdb_populate(br, &item, fdb, NULL);
 
 	switch (type) {



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux