| Message ID | 20220701205227.1337160-5-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: lan966x: Add lag support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 69 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, Jul 01, 2022 at 10:52:24PM +0200, Horatiu Vultur wrote: > Extend lan966x_foreign_bridging_check to check also if the upper > interface is a lag device. Don't allow a lan966x port to be part of a > lag if it has foreign interfaces. > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > .../microchip/lan966x/lan966x_switchdev.c | 32 ++++++++++++++----- > 1 file changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c > index d9b3ca5f6214..fe872edfcdca 100644 > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c > @@ -326,23 +326,25 @@ static int lan966x_port_prechangeupper(struct net_device *dev, > return NOTIFY_DONE; > } > > -static int lan966x_foreign_bridging_check(struct net_device *bridge, > +static int lan966x_foreign_bridging_check(struct net_device *upper, > + bool *has_foreign, > + bool *seen_lan966x, > struct netlink_ext_ack *extack) > { > struct lan966x *lan966x = NULL; > - bool has_foreign = false; > struct net_device *dev; > struct list_head *iter; > > - if (!netif_is_bridge_master(bridge)) > + if (!netif_is_bridge_master(upper) && > + !netif_is_lag_master(upper)) > return 0; > > - netdev_for_each_lower_dev(bridge, dev, iter) { > + netdev_for_each_lower_dev(upper, dev, iter) { > if (lan966x_netdevice_check(dev)) { > struct lan966x_port *port = netdev_priv(dev); > > if (lan966x) { > - /* Bridge already has at least one port of a > + /* Upper already has at least one port of a > * lan966x switch inside it, check that it's > * the same instance of the driver. > */ > @@ -353,15 +355,24 @@ static int lan966x_foreign_bridging_check(struct net_device *bridge, > } > } else { > /* This is the first lan966x port inside this > - * bridge > + * upper device > */ > lan966x = port->lan966x; > + *seen_lan966x = true; > } > + } else if (netif_is_lag_master(dev)) { > + /* Allow to have bond interfaces that have only lan966x > + * devices > + */ > + if (lan966x_foreign_bridging_check(dev, has_foreign, > + seen_lan966x, > + extack)) > + *has_foreign = true; Not clear why you set *has_foreign here and not just stop and return. The extack has presumably already been populated by the called function, there is absolutely no need to continue if an error has already been found. > } else { > - has_foreign = true; > + *has_foreign = true; > } > > - if (lan966x && has_foreign) { > + if (*seen_lan966x && *has_foreign) { > NL_SET_ERR_MSG_MOD(extack, > "Bridging lan966x ports with foreign interfaces disallowed"); > return -EINVAL; > @@ -374,7 +385,12 @@ static int lan966x_foreign_bridging_check(struct net_device *bridge, > static int lan966x_bridge_check(struct net_device *dev, > struct netdev_notifier_changeupper_info *info) > { > + bool has_foreign = false; > + bool seen_lan966x = false; > + > return lan966x_foreign_bridging_check(info->upper_dev, > + &has_foreign, > + &seen_lan966x, > info->info.extack); > } > > -- > 2.33.0 >
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c index d9b3ca5f6214..fe872edfcdca 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c @@ -326,23 +326,25 @@ static int lan966x_port_prechangeupper(struct net_device *dev, return NOTIFY_DONE; } -static int lan966x_foreign_bridging_check(struct net_device *bridge, +static int lan966x_foreign_bridging_check(struct net_device *upper, + bool *has_foreign, + bool *seen_lan966x, struct netlink_ext_ack *extack) { struct lan966x *lan966x = NULL; - bool has_foreign = false; struct net_device *dev; struct list_head *iter; - if (!netif_is_bridge_master(bridge)) + if (!netif_is_bridge_master(upper) && + !netif_is_lag_master(upper)) return 0; - netdev_for_each_lower_dev(bridge, dev, iter) { + netdev_for_each_lower_dev(upper, dev, iter) { if (lan966x_netdevice_check(dev)) { struct lan966x_port *port = netdev_priv(dev); if (lan966x) { - /* Bridge already has at least one port of a + /* Upper already has at least one port of a * lan966x switch inside it, check that it's * the same instance of the driver. */ @@ -353,15 +355,24 @@ static int lan966x_foreign_bridging_check(struct net_device *bridge, } } else { /* This is the first lan966x port inside this - * bridge + * upper device */ lan966x = port->lan966x; + *seen_lan966x = true; } + } else if (netif_is_lag_master(dev)) { + /* Allow to have bond interfaces that have only lan966x + * devices + */ + if (lan966x_foreign_bridging_check(dev, has_foreign, + seen_lan966x, + extack)) + *has_foreign = true; } else { - has_foreign = true; + *has_foreign = true; } - if (lan966x && has_foreign) { + if (*seen_lan966x && *has_foreign) { NL_SET_ERR_MSG_MOD(extack, "Bridging lan966x ports with foreign interfaces disallowed"); return -EINVAL; @@ -374,7 +385,12 @@ static int lan966x_foreign_bridging_check(struct net_device *bridge, static int lan966x_bridge_check(struct net_device *dev, struct netdev_notifier_changeupper_info *info) { + bool has_foreign = false; + bool seen_lan966x = false; + return lan966x_foreign_bridging_check(info->upper_dev, + &has_foreign, + &seen_lan966x, info->info.extack); }
Extend lan966x_foreign_bridging_check to check also if the upper interface is a lag device. Don't allow a lan966x port to be part of a lag if it has foreign interfaces. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- .../microchip/lan966x/lan966x_switchdev.c | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-)