Re: [PATCHES] Networking

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