| Message ID | 20210318141550.646383-2-tobias@waldekranz.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: mv88e6xxx: Offload bridge port flags | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 15 this patch: 15 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 15 this patch: 15 |
| netdev/header_inline | success | Link |
On Thu, Mar 18, 2021 at 03:15:43PM +0100, Tobias Waldekranz wrote: > In order for a driver to be able to query a bridge for information > about itself, e.g. reading out port flags, it has to use a netdev that > is known to the bridge. In the simple case, that is just the netdev > representing the port, e.g. swp0 or swp1 in this example: > > br0 > / \ > swp0 swp1 > > But in the case of an offloaded lag, this will be the bond or team > interface, e.g. bond0 in this example: > > br0 > / > bond0 > / \ > swp0 swp1 > > Add a helper that hides some of this complexity from the > drivers. Then, redefine dsa_port_offloads_bridge_port using the helper > to avoid double accounting of the set of possible offloaded uppers. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > include/net/dsa.h | 14 ++++++++++++++ > net/dsa/dsa_priv.h | 14 +------------- > 2 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index dac303edd33d..5c4340ecfeb2 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -493,6 +493,20 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp) > return dp->vlan_filtering; > } > > +static inline > +struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) > +{ > + if (!dsa_is_user_port(dp->ds, dp->index)) > + return NULL; > + According to my comment from 8/8, you could have replaced this here with if (!dp->bridge_dev) return NULL; I think it's more intuitive to not return a bridge port if there isn't any bridge to speak of. Whether you prefer to do that or not is up to you, here's my review tag anyway. Reviewed-by: Vladimir Oltean <olteanv@gmail.com> > + if (dp->lag_dev) > + return dp->lag_dev; > + else if (dp->hsr_dev) > + return dp->hsr_dev; > + > + return dp->slave; > +} > + > typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid, > bool is_static, void *data); > struct dsa_switch_ops {
On 3/18/2021 7:15 AM, Tobias Waldekranz wrote: > In order for a driver to be able to query a bridge for information > about itself, e.g. reading out port flags, it has to use a netdev that > is known to the bridge. In the simple case, that is just the netdev > representing the port, e.g. swp0 or swp1 in this example: > > br0 > / \ > swp0 swp1 > > But in the case of an offloaded lag, this will be the bond or team > interface, e.g. bond0 in this example: > > br0 > / > bond0 > / \ > swp0 swp1 > > Add a helper that hides some of this complexity from the > drivers. Then, redefine dsa_port_offloads_bridge_port using the helper > to avoid double accounting of the set of possible offloaded uppers. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff --git a/include/net/dsa.h b/include/net/dsa.h index dac303edd33d..5c4340ecfeb2 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -493,6 +493,20 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp) return dp->vlan_filtering; } +static inline +struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) +{ + if (!dsa_is_user_port(dp->ds, dp->index)) + return NULL; + + if (dp->lag_dev) + return dp->lag_dev; + else if (dp->hsr_dev) + return dp->hsr_dev; + + return dp->slave; +} + typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid, bool is_static, void *data); struct dsa_switch_ops { diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 9d4b0e9b1aa1..4c43c5406834 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -233,19 +233,7 @@ extern const struct phylink_mac_ops dsa_port_phylink_mac_ops; static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp, struct net_device *dev) { - /* Switchdev offloading can be configured on: */ - - if (dev == dp->slave) - /* DSA ports directly connected to a bridge, and event - * was emitted for the ports themselves. - */ - return true; - - if (dp->lag_dev == dev) - /* DSA ports connected to a bridge via a LAG */ - return true; - - return false; + return dsa_port_to_bridge_port(dp) == dev; } static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
In order for a driver to be able to query a bridge for information about itself, e.g. reading out port flags, it has to use a netdev that is known to the bridge. In the simple case, that is just the netdev representing the port, e.g. swp0 or swp1 in this example: br0 / \ swp0 swp1 But in the case of an offloaded lag, this will be the bond or team interface, e.g. bond0 in this example: br0 / bond0 / \ swp0 swp1 Add a helper that hides some of this complexity from the drivers. Then, redefine dsa_port_offloads_bridge_port using the helper to avoid double accounting of the set of possible offloaded uppers. Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- include/net/dsa.h | 14 ++++++++++++++ net/dsa/dsa_priv.h | 14 +------------- 2 files changed, 15 insertions(+), 13 deletions(-)