search for: br_fdb_static

On Wed, Apr 12, 2023 at 05:27:33PM +0300, Vladimir Oltean wrote: > How are extern_learn FDB entries processed by spectrum's > SWITCHDEV_FDB_ADD_TO_DEVICE handler? No different than "BR_FDB_STATIC", which is a bug I'm aware of and intend to fix in net-next when I get the time (together with all the other combinations enabled by the bridge). Entry has ageing disabled, but can roam in which case it becomes age-able. TBH, I think most devices don't handle "BR_FDB_STATIC"...

On Wed, Apr 12, 2023 at 07:00:02PM +0300, Ido Schimmel wrote: > On Wed, Apr 12, 2023 at 05:27:33PM +0300, Vladimir Oltean wrote: > > How are extern_learn FDB entries processed by spectrum's > > SWITCHDEV_FDB_ADD_TO_DEVICE handler? > > No different than "BR_FDB_STATIC", which is a bug I'm aware of and > intend to fix in net-next when I get the time (together with all the > other combinations enabled by the bridge). Entry has ageing disabled, > but can roam in which case it becomes age-able. > > TBH, I think most devices don't handle...

On Wed, Apr 12, 2023 at 05:15:03PM +0300, Ido Schimmel wrote: > > Looking at tools/testing/selftests/net/forwarding/, there is no valid > > use of the "bridge fdb add ... master dynamic" command there, so I am > > fairly confident that no one used to rely on this behavior. > > Yes, but there are tests that use "extern_learn". If you post a v2 that >

...dded_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 de...

On Wed, Jan 18, 2023 at 11:14:00PM +0100, netdev at kapio-technology.com wrote: > > > + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags); > > > > Why reverse logic? Why not just name this "is_static" and leave any > > further interpretations up to the consumer? > > My reasoning for this is that the common case is to have static entries, > thus is_dyn=false, so whenever some...

...void br_switchdev_fdb_populate(struct net_bridge *br, item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags); item->locked = false; item->info.dev = (!p || item->is_local) ? br->dev : p->dev; item->info.ctx = ctx; -- 2.34.1

On Thu, Mar 30, 2023 at 18:07, Vladimir Oltean <olteanv at gmail.com> wrote: > As a bug fix, stop reporting to switchdev those FDB entries with > BR_FDB_ADDED_BY_USER && !BR_FDB_STATIC. Then, after "net" is merged into > "net-next" next Thursday (the ship has sailed for today), add "bool static" It is probably too late today (now I have a Debian based VM that can do the selftests), but with this bug fix I have 1) not submitted bug fixes before an...

On Thu, Apr 06, 2023 at 05:17:46PM +0200, Hans Schultz wrote: > On Thu, Mar 30, 2023 at 18:07, Vladimir Oltean <olteanv at gmail.com> wrote: > > As a bug fix, stop reporting to switchdev those FDB entries with > > BR_FDB_ADDED_BY_USER && !BR_FDB_STATIC. Then, after "net" is merged into > > "net-next" next Thursday (the ship has sailed for today), add "bool static" > > It is probably too late today (now I have a Debian based VM that can do > the selftests), but with this bug fix I have 1) not submitted...

...adimir Oltean wrote: > I'll send v2 with BR_FDB_ADDED_BY_EXT_LEARN not prevented from being > notified from switchdev. > > Unless you have any objection, I won't send v2 like this: > > 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; > > but like this: > > /* Entries with just the BR_FDB_ADDED_BY_USER flag set were created > * using 'bridge fdb add ... master dynamic' > */ > if...

On Thu, Jan 19, 2023 at 11:33:58AM +0200, Vladimir Oltean wrote: > On Wed, Jan 18, 2023 at 11:14:00PM +0100, netdev at kapio-technology.com wrote: > > > > + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags); > > > > > > Why reverse logic? Why not just name this "is_static" and leave any > > > further interpretations up to the consumer? > > > > My reasoning for this is that the common case is to have static entries, > > thus...

...; net_bridge *br, >> item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); >> item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); >> item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); >> + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags); > > Why reverse logic? Why not just name this "is_static" and leave any > further interpretations up to the consumer? > My reasoning for this is that the common case is to have static entries, thus is_dyn=false, so whenever someone uses a switchdev_n...

...l free. Didn't I explain what I would do from the first reply on this thread? https://patchwork.kernel.org/project/netdevbpf/patch/20230318141010.513424-3-netdev at kapio-technology.com/#25270613 As a bug fix, stop reporting to switchdev those FDB entries with BR_FDB_ADDED_BY_USER && !BR_FDB_STATIC. Then, after "net" is merged into "net-next" next Thursday (the ship has sailed for today), add "bool static" to the switchdev notifier info, and make all switchdev drivers (everywhere where a SWITCHDEV_FDB_ADD_TO_DEVICE handler appears) ignore the "added_by_user...

...b_populate(struct net_bridge *br, > item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); > item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); > item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); > + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags); Why reverse logic? Why not just name this "is_static" and leave any further interpretations up to the consumer? > item->locked = false; > item->info.dev = (!p || item->is_local) ? br->dev : p->dev; > item->info.ctx = ctx; > -- &...

...c) 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 swp...

On Thu, Mar 30, 2023 at 16:09, Vladimir Oltean <olteanv at gmail.com> wrote: > On Thu, Mar 30, 2023 at 02:59:04PM +0200, Hans Schultz wrote: >> On Thu, Mar 30, 2023 at 15:43, Vladimir Oltean <olteanv at gmail.com> wrote: >> > On Tue, Mar 28, 2023 at 09:45:26PM +0200, Hans Schultz wrote: >> >> So the solution would be to not let the DSA layer send the

On 2023-01-18 00:17, Vladimir Oltean wrote: > On Tue, Jan 17, 2023 at 07:57:11PM +0100, Hans J. Schultz wrote: >> Dynamic FDB flag needs to be propagated through the DSA layer to be >> added to drivers. >> Use a u16 for fdb flags for future use, so that other flags can also >> be >> sent the same way without having to change function interfaces. >> >>

...nology.com> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index e69a872bfc1d..b0c23a72bc76 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -537,6 +537,7 @@ void br_fdb_cleanup(struct work_struct *work) unsigned long this_timer = f->updated + delay; if (test_bit(BR_FDB_STATIC, &f->flags) || + test_bit(BR_FDB_OFFLOADED, &f->flags) || test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags)) { if (test_bit(BR_FDB_NOTIFY, &f->flags)) { if (time_after(this_timer, now)) @@ -1465,7 +1466,9 @@ int br_fdb_external_learn_del(struct net_brid...

...@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr) static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f, bool swdev_notify) { + bool learned = !(f->flags & BR_FDB_NOT_LEARNED_MASK); + trace_fdb_delete(br, f); if (test_bit(BR_FDB_STATIC, &f->flags)) @@ -329,11 +363,16 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f, hlist_del_init_rcu(&f->fdb_node); rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode, br_fdb_rht_params); + br->fdb_cur_learned_entries -=...

On Mon, Mar 27, 2023 at 14:52, Vladimir Oltean <olteanv at gmail.com> wrote: > > By the way, there is a behavior change here. > > Before: > > $ ip link add br0 type bridge && ip link set br0 up > $ ip link set swp0 master br0 && ip link set swp0 up > $ bridge fdb add dev swp0 00:01:02:03:04:05 master dynamic > [ 70.010181] mscc_felix 0000:00:00.5:

Introduce a limit on the amount of learned FDB entries on a bridge, configured by netlink with a build time default on bridge creation in the kernel config. For backwards compatibility the kernel config default is disabling the limit (0). Without any limit a malicious actor may OOM a kernel by spamming packets with changing MAC addresses on their bridge port, so allow the bridge creator to limit