search for: switchdev_notifier_fdb_info

...dev.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/include/net/switchdev.h b/include/net/switchdev.h >> index ca0312b78294..aaf918d4ba67 100644 >> --- a/include/net/switchdev.h >> +++ b/include/net/switchdev.h >> @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { >> u8 added_by_user:1, >> is_local:1, >> locked:1, >> + is_dyn:1, >> offloaded:1; >> }; >> >> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c >> index 7eb6fd5bb917..60c05a00a1df 100644 >> --- a/ne...

...+ > net/bridge/br_switchdev.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h > index ca0312b78294..aaf918d4ba67 100644 > --- a/include/net/switchdev.h > +++ b/include/net/switchdev.h > @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { > u8 added_by_user:1, > is_local:1, > locked:1, > + is_dyn:1, > offloaded:1; > }; > > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c > index 7eb6fd5bb917..60c05a00a1df 100644 > --- a/net/bridge/br_switchdev.c > +++ b/net/...

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 (her...

...>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_notifier_fdb_info > struct the common case does not need to be entered. > Otherwise it might also break something when someone uses this struct and if > it was 'is_static' and they forget to code is_static=true they will get > dynamic entries without wanting it and it can be hard to find such an...

...> --- include/net/switchdev.h | 1 + net/bridge/br_switchdev.c | 1 + 2 files changed, 2 insertions(+) diff --git a/include/net/switchdev.h b/include/net/switchdev.h index ca0312b78294..aaf918d4ba67 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { u8 added_by_user:1, is_local:1, locked:1, + is_dyn:1, offloaded:1; }; diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index de18e9c1d7a7..9707d3fdb396 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -134,6 +134,7 @@ static void b...

...t; > > 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_notifier_fdb_info > > struct the common case does not need to be entered. > > Otherwise it might also break something when someone uses this struct and if > > it was 'is_static' and they forget to code is_static=true they will get > > dynamic entries without wanting it and it can be h...

...;t think this is going to work with the assisted_learning_on_cpu_port feature ("if (switchdev_fdb_is_dynamically_learned(fdb_info))"). The reason being that a "dynamically learned" FDB entry (defined as this): static inline bool switchdev_fdb_is_dynamically_learned(const struct switchdev_notifier_fdb_info *fdb_info) { return !fdb_info->added_by_user && !fdb_info->is_local; } is also dynamic in the DSA_FDB_FLAG_DYNAMIC sense. But we install a static FDB entry for it on the CPU port. And in your follow-up patch 3/5, you make all drivers except mv88e6xxx ignore all DSA_FDB_FLAG_DYNAMIC...

On Mon, Apr 10, 2023 at 11:49:51PM +0300, Vladimir Oltean wrote: > 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_no...

On Tue, Mar 28, 2023 at 01:59, Vladimir Oltean <olteanv at gmail.com> wrote: > > which idea is that, again? So I cannot us the offloaded flag as it is added by DSA in the common case when using 'bridge fdb replace ... dynamic'. The idea is then to use the ext_learn flag instead, which is not aged by the bridge. To do this the driver (mv88e6xxx) will send a

...#39;t roam from one bridge port to another until software sees a packet with that MAC DA, and they have the potential of blocking traffic because of that. If for some reason you do think that behavior is useful and still want to keep it (I'm not sure I would), I would consider extending struct switchdev_notifier_fdb_info with a "bool pls_dont_age_out", and I would make dsa_fdb_offload_notify() set this to true if the driver did actually install the dynamic FDB entry as dynamic in the ATU. > > The idea is then to use the ext_learn flag instead, which is not aged by > the bridge. To do this the d...

...> assisted_learning_on_cpu_port > feature ("if (switchdev_fdb_is_dynamically_learned(fdb_info))"). The > reason being > that a "dynamically learned" FDB entry (defined as this): > > static inline bool > switchdev_fdb_is_dynamically_learned(const struct > switchdev_notifier_fdb_info *fdb_info) > { > return !fdb_info->added_by_user && !fdb_info->is_local; > } > > is also dynamic in the DSA_FDB_FLAG_DYNAMIC sense. But we install a > static FDB entry for it on the CPU port. > > And in your follow-up patch 3/5, you make all drivers except m...