| Message ID | 20201208120802.1268708-4-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | LAG offload for Ocelot DSA switches | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | fail | Errors and warnings before: 4 this patch: 10 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 4 this patch: 10 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
Hi, On 08/12/2020 14:07:49+0200, Vladimir Oltean wrote: > -static int ocelot_netdevice_port_event(struct net_device *dev, > - unsigned long event, > - struct netdev_notifier_changeupper_info *info) > +static int ocelot_netdevice_changeupper(struct net_device *dev, > + struct netdev_notifier_changeupper_info *info) [...] > - netdev_for_each_lower_dev(dev, slave, iter) { > - ret = ocelot_netdevice_port_event(slave, event, info); > - if (ret) > - goto notify; > + netdev_for_each_lower_dev(dev, slave, iter) { > + ret = ocelot_netdevice_changeupper(slave, event, info); > + if (ret) > + goto notify; > + } > + } else { > + ret = ocelot_netdevice_changeupper(dev, event, info); Does that compile? Shouldn't event be dropped?
On Tue, Dec 15, 2020 at 04:01:32PM +0100, Alexandre Belloni wrote: > Hi, > > On 08/12/2020 14:07:49+0200, Vladimir Oltean wrote: > > -static int ocelot_netdevice_port_event(struct net_device *dev, > > - unsigned long event, > > - struct netdev_notifier_changeupper_info *info) > > +static int ocelot_netdevice_changeupper(struct net_device *dev, > > + struct netdev_notifier_changeupper_info *info) > > [...] > > > - netdev_for_each_lower_dev(dev, slave, iter) { > > - ret = ocelot_netdevice_port_event(slave, event, info); > > - if (ret) > > - goto notify; > > + netdev_for_each_lower_dev(dev, slave, iter) { > > + ret = ocelot_netdevice_changeupper(slave, event, info); > > + if (ret) > > + goto notify; > > + } > > + } else { > > + ret = ocelot_netdevice_changeupper(dev, event, info); > > Does that compile? No it doesn't. > Shouldn't event be dropped? It is, but in the next patch. I'll fix it, thanks.
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c index 6fb2a813e694..50765a3b1c44 100644 --- a/drivers/net/ethernet/mscc/ocelot_net.c +++ b/drivers/net/ethernet/mscc/ocelot_net.c @@ -1003,9 +1003,8 @@ static int ocelot_port_obj_del(struct net_device *dev, return ret; } -static int ocelot_netdevice_port_event(struct net_device *dev, - unsigned long event, - struct netdev_notifier_changeupper_info *info) +static int ocelot_netdevice_changeupper(struct net_device *dev, + struct netdev_notifier_changeupper_info *info) { struct ocelot_port_private *priv = netdev_priv(dev); struct ocelot_port *ocelot_port = &priv->port; @@ -1013,28 +1012,22 @@ static int ocelot_netdevice_port_event(struct net_device *dev, int port = priv->chip_port; int err = 0; - switch (event) { - case NETDEV_CHANGEUPPER: - if (netif_is_bridge_master(info->upper_dev)) { - if (info->linking) { - err = ocelot_port_bridge_join(ocelot, port, - info->upper_dev); - } else { - err = ocelot_port_bridge_leave(ocelot, port, - info->upper_dev); - } - } - if (netif_is_lag_master(info->upper_dev)) { - if (info->linking) - err = ocelot_port_lag_join(ocelot, port, - info->upper_dev); - else - ocelot_port_lag_leave(ocelot, port, + if (netif_is_bridge_master(info->upper_dev)) { + if (info->linking) { + err = ocelot_port_bridge_join(ocelot, port, info->upper_dev); + } else { + err = ocelot_port_bridge_leave(ocelot, port, + info->upper_dev); } - break; - default: - break; + } + if (netif_is_lag_master(info->upper_dev)) { + if (info->linking) + err = ocelot_port_lag_join(ocelot, port, + info->upper_dev); + else + ocelot_port_lag_leave(ocelot, port, + info->upper_dev); } return err; @@ -1063,17 +1056,19 @@ static int ocelot_netdevice_event(struct notifier_block *unused, } } - if (netif_is_lag_master(dev)) { - struct net_device *slave; - struct list_head *iter; + if (event == NETDEV_CHANGEUPPER) { + if (netif_is_lag_master(dev)) { + struct net_device *slave; + struct list_head *iter; - netdev_for_each_lower_dev(dev, slave, iter) { - ret = ocelot_netdevice_port_event(slave, event, info); - if (ret) - goto notify; + netdev_for_each_lower_dev(dev, slave, iter) { + ret = ocelot_netdevice_changeupper(slave, event, info); + if (ret) + goto notify; + } + } else { + ret = ocelot_netdevice_changeupper(dev, event, info); } - } else { - ret = ocelot_netdevice_port_event(dev, event, info); } notify:
ocelot_netdevice_port_event treats a single event, NETDEV_CHANGEUPPER. So we can remove the check for the type of event, and rename the function to be more suggestive, since there already is a function with a very similar name of ocelot_netdevice_event. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/ethernet/mscc/ocelot_net.c | 59 ++++++++++++-------------- 1 file changed, 27 insertions(+), 32 deletions(-)