Re: [PATCH RFC] net: bridge: handle ports in locked mode for ll learning
From: Ido Schimmel <idosch@nvidia.com>
Date: 2024-12-12 12:25:56
Also in:
bridge, lkml
On Thu, Dec 12, 2024 at 10:50:10AM +0100, Jonas Gorski wrote:
The original patch (just disabling LL learning if port is locked) has the same issue as mine, it will indirectly break switchdev offloading for case 2 when not using MAB (the kernel feature). Once we disable creating dynamic entries in the kernel, userspace needs to create them,
But the whole point of the "locked" feature is to defer the installation of FDB entries to user space so that the control plane will be able to decide which hosts can communicate through the bridge. Having the kernel auto-populate the FDB based on incoming packets defeats this purpose, which is why the man page mentions the "no_linklocal_learn" option and why I think there is a very low risk of regressions from the original patch.
and userspace dynamic entries have the user bit set, which makes them get ignored by switchdev.
The second use case never worked correctly in the offload case. It is not a regression.
quoted hunk ↗ jump to hunk
Ofc enabling MAB and then unlocking the locked entries hosts that successfully authenticated should still work for 2, as long as the host sent something other than link local traffic to create a (locked) dynamic entry AFAIU. FWIW, my proposed change/fix would be:diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index ceaa5a89b947..41b69ea300bf 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c@@ -238,7 +238,8 @@ static void __br_handle_local_finish(struct sk_buff *skb) nbp_state_should_learn(p) && !br_opt_get(p->br, BROPT_NO_LL_LEARN) && br_should_learn(p, skb, &vid)) - br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, 0); + br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, + p->flags & BR_PORT_MAB ? BIT(BR_FDB_LOCKED) : 0);
IIUC, this will potentially roam FDB entries to unauthorized ports,
unlike the implementation in br_handle_frame_finish(). I documented it
in commit a35ec8e38cdd ("bridge: Add MAC Authentication Bypass (MAB)
support") in "1. Roaming".
} /* note: already called with rcu_read_lock */ which just makes sure that when MAB is enabled, link local learned entries are also locked. This relies on br_fdb_update() ignoring most flags for existing entries, not sure if this is a good idea though. Best Regards, Jonas -- BISDN GmbH Körnerstraße 7-10 10785 Berlin Germany Phone: +49-30-6108-1-6100 Managing Directors: Dr.-Ing. Hagen Woesner, Andreas Köpsel Commercial register: Amtsgericht Berlin-Charlottenburg HRB 141569 B VAT ID No: DE283257294