On Thu, Jun 07, 2018 at 09:00:12AM +0200, Jiri Slaby wrote: > On 09/15/2017, 06:57 AM, David Miller wrote: > > Please queue up the following networking bug fixes for v4.9, v4.12, and > > v4.13 -stable, respectively. > > Hi, > > while walking through some fixes, I wonder, whether backports of > 25cc72a33835 (mlxsw: spectrum: Forbid linking to devices that have > uppers) to 4.9 and 4.12 are correct. > > Part of the original commit: > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > @@ -4139,6 +4139,8 @@ static int > mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, > return -EINVAL; > if (!info->linking) > break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > if (netif_is_lag_master(upper_dev) && > !mlxsw_sp_master_lag_check(mlxsw_sp, upper_dev, > info->upper_info)) > @@ -4258,6 +4260,10 @@ static int > mlxsw_sp_netdevice_port_vlan_event(struct net_device *vlan_dev, > upper_dev = info->upper_dev; > if (!netif_is_bridge_master(upper_dev)) > return -EINVAL; > + if (!info->linking) > + break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > break; > case NETDEV_CHANGEUPPER: > upper_dev = info->upper_dev; > > > > It changes mlxsw_sp_netdevice_port_upper_event and > mlxsw_sp_netdevice_port_vlan_event. > > > > > 4.9 backport (73ee5a73e75): > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > @@ -4172,6 +4172,8 @@ static int > mlxsw_sp_netdevice_port_upper_event(struct net_device *dev, > return -EINVAL; > if (!info->linking) > break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > /* HW limitation forbids to put ports to multiple > bridges. */ > if (netif_is_bridge_master(upper_dev) && > !mlxsw_sp_master_bridge_check(mlxsw_sp, upper_dev)) > @@ -4185,6 +4187,10 @@ static int > mlxsw_sp_netdevice_port_upper_event(struct net_device *dev, > if (netif_is_lag_port(dev) && is_vlan_dev(upper_dev) && > !netif_is_lag_master(vlan_dev_real_dev(upper_dev))) > return -EINVAL; > + if (!info->linking) > + break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > break; > case NETDEV_CHANGEUPPER: > upper_dev = info->upper_dev; > > > > > It changes mlxsw_sp_netdevice_port_upper_event *twice* instead of > mlxsw_sp_netdevice_port_vlan_event, which was named > mlxsw_sp_netdevice_vport_event in 4.9 yet. > > > > > > 4.12 backport (2f4232ba8001): > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c > @@ -4110,6 +4110,8 @@ static int > mlxsw_sp_netdevice_port_upper_event(struct net_device *dev, > return -EINVAL; > if (!info->linking) > break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > /* HW limitation forbids to put ports to multiple > bridges. */ G> if (netif_is_bridge_master(upper_dev) && > !mlxsw_sp_master_bridge_check(mlxsw_sp, upper_dev)) > @@ -4274,6 +4276,10 @@ static int mlxsw_sp_netdevice_bridge_event(struct > net_device *br_dev, > if (is_vlan_dev(upper_dev) && > br_dev != mlxsw_sp->master_bridge.dev) > return -EINVAL; > + if (!info->linking) > + break; > + if (netdev_has_any_upper_dev(upper_dev)) > + return -EINVAL; > break; > case NETDEV_CHANGEUPPER: > upper_dev = info->upper_dev; > > > > It changes mlxsw_sp_netdevice_port_upper_event (OK) and > mlxsw_sp_netdevice_bridge_event (not OK) instead of > mlxsw_sp_netdevice_vport_event. > > > Did I miss something or is this a mistake? Looks odd to me, want me to revert this from 4.9? Without the hardware, I doubt anyone has noticed this issue. thanks, greg k-h
