mlxsw: spectrum_switchdev: Manage RIFs on PVID change

Currently, mlxsw has several shortcomings with regards to RIF handling due
to PVID changes:

- In order to cause RIF for a bridge device to be created, the user is
  expected first to set PVID, then to add an IP address. The reverse
  ordering is disallowed, which is not very user-friendly.

- When such bridge gets a VLAN upper whose VID was the same as the existing
  PVID, and this VLAN netdevice gets an IP address, a RIF is created for
  this netdevice. The new RIF is then assigned to the 802.1Q FID for the
  given VID. This results in a working configuration. However, then, when
  the VLAN netdevice is removed again, the RIF for the bridge itself is
  never reassociated to the VLAN.

- PVID cannot be changed once the bridge has uppers. Presumably this is
  because the driver does not manage RIFs properly in face of PVID changes.
  However, as the previous point shows, it is still possible to get into
  invalid configurations.

In this patch, add the logic necessary for creation of a RIF as a result of
PVID change. Moreover, when a VLAN upper is created whose VID matches lower
PVID, do not create RIF for this netdevice.

These changes obviate the need for ordering of IP address additions and
PVID configuration, so stop forbidding addition of an IP address to a
PVID-less bridge. Instead, bail out quietly. Also stop preventing PVID
changes when the bridge has uppers.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Danielle Ratson <danieller@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 63f40d16be3bf41ab65ff14e7c4be30a4aa0e362..109ac2db0d65446e3db1de3b876a75679a79cad0 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -8411,6 +8411,110 @@ out:
        mutex_unlock(&mlxsw_sp->router->lock);
 }
 
        mutex_unlock(&mlxsw_sp->router->lock);
 }
 

+static void mlxsw_sp_rif_destroy_vlan_upper(struct mlxsw_sp *mlxsw_sp,
+                                           struct net_device *br_dev,
+                                           u16 vid)
+{
+       struct net_device *upper_dev;
+       struct mlxsw_sp_crif *crif;
+
+       rcu_read_lock();
+       upper_dev = __vlan_find_dev_deep_rcu(br_dev, htons(ETH_P_8021Q), vid);
+       rcu_read_unlock();
+
+       if (!upper_dev)
+               return;
+
+       crif = mlxsw_sp_crif_lookup(mlxsw_sp->router, upper_dev);
+       if (!crif || !crif->rif)
+               return;
+
+       mlxsw_sp_rif_destroy(crif->rif);
+}
+
+static int mlxsw_sp_inetaddr_bridge_event(struct mlxsw_sp *mlxsw_sp,
+                                         struct net_device *l3_dev,
+                                         int lower_pvid,
+                                         unsigned long event,
+                                         struct netlink_ext_ack *extack);
+
+int mlxsw_sp_router_bridge_vlan_add(struct mlxsw_sp *mlxsw_sp,
+                                   struct net_device *br_dev,
+                                   u16 new_vid, bool is_pvid,
+                                   struct netlink_ext_ack *extack)
+{
+       struct mlxsw_sp_rif *old_rif;
+       struct mlxsw_sp_rif *new_rif;
+       struct net_device *upper_dev;
+       u16 old_pvid = 0;
+       u16 new_pvid;
+       int err = 0;
+
+       mutex_lock(&mlxsw_sp->router->lock);
+       old_rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, br_dev);
+       if (old_rif) {
+               /* If the RIF on the bridge is not a VLAN RIF, we shouldn't have
+                * gotten a PVID notification.
+                */
+               if (WARN_ON(old_rif->ops->type != MLXSW_SP_RIF_TYPE_VLAN))
+                       old_rif = NULL;
+               else
+                       old_pvid = mlxsw_sp_fid_8021q_vid(old_rif->fid);
+       }
+
+       if (is_pvid)
+               new_pvid = new_vid;
+       else if (old_pvid == new_vid)
+               new_pvid = 0;
+       else
+               goto out;
+
+       if (old_pvid == new_pvid)
+               goto out;
+
+       if (new_pvid) {
+               struct mlxsw_sp_rif_params params = {
+                       .dev = br_dev,
+                       .vid = new_pvid,
+               };
+
+               /* If there is a VLAN upper with the same VID as the new PVID,
+                * kill its RIF, if there is one.
+                */
+               mlxsw_sp_rif_destroy_vlan_upper(mlxsw_sp, br_dev, new_pvid);
+
+               if (mlxsw_sp_dev_addr_list_empty(br_dev))
+                       goto out;
+               new_rif = mlxsw_sp_rif_create(mlxsw_sp, &params, extack);
+               if (IS_ERR(new_rif)) {
+                       err = PTR_ERR(new_rif);
+                       goto out;
+               }
+
+               if (old_pvid)
+                       mlxsw_sp_rif_migrate_destroy(mlxsw_sp, old_rif, new_rif,
+                                                    true);
+       } else {
+               mlxsw_sp_rif_destroy(old_rif);
+       }
+
+       if (old_pvid) {
+               rcu_read_lock();
+               upper_dev = __vlan_find_dev_deep_rcu(br_dev, htons(ETH_P_8021Q),
+                                                    old_pvid);
+               rcu_read_unlock();
+               if (upper_dev)
+                       err = mlxsw_sp_inetaddr_bridge_event(mlxsw_sp,
+                                                            upper_dev,
+                                                            new_pvid,
+                                                            NETDEV_UP, extack);
+       }
+
+out:
+       mutex_unlock(&mlxsw_sp->router->lock);
+       return err;
+}
+

 static void
 mlxsw_sp_rif_subport_params_init(struct mlxsw_sp_rif_params *params,
                                 struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
 static void
 mlxsw_sp_rif_subport_params_init(struct mlxsw_sp_rif_params *params,
                                 struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)


@@ -8847,13 +8951,20 @@ static int mlxsw_sp_inetaddr_bridge_event(struct mlxsw_sp *mlxsw_sp,
                                return -EOPNOTSUPP;
                        }
                        err = br_vlan_get_pvid(l3_dev, &params.vid);
                                return -EOPNOTSUPP;
                        }
                        err = br_vlan_get_pvid(l3_dev, &params.vid);

-                       if (err < 0 || !params.vid) {
-                               NL_SET_ERR_MSG_MOD(extack, "Couldn't determine bridge PVID");
-                               return -EINVAL;
-                       }
+                       if (err)
+                               return err;
+                       if (!params.vid)
+                               return 0;

                } else if (is_vlan_dev(l3_dev)) {
                        params.vid = vlan_dev_vlan_id(l3_dev);
                } else if (is_vlan_dev(l3_dev)) {
                        params.vid = vlan_dev_vlan_id(l3_dev);

+
+                       /* If the VID matches PVID of the bridge below, the
+                        * bridge owns the RIF for this VLAN. Don't do anything.
+                        */
+                       if ((int)params.vid == lower_pvid)
+                               return 0;

                }
                }

+

                rif = mlxsw_sp_rif_create(mlxsw_sp, &params, extack);
                if (IS_ERR(rif))
                        return PTR_ERR(rif);
                rif = mlxsw_sp_rif_create(mlxsw_sp, &params, extack);
                if (IS_ERR(rif))
                        return PTR_ERR(rif);

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index 9a2669a084800b2631eec89f326c6d4d5b145116..74242220a0cf0beffe4ad940c0e1aa2ae9e991d4 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -171,6 +171,10 @@ int mlxsw_sp_ipip_ecn_encap_init(struct mlxsw_sp *mlxsw_sp);
 int mlxsw_sp_ipip_ecn_decap_init(struct mlxsw_sp *mlxsw_sp);
 struct net_device *
 mlxsw_sp_ipip_netdev_ul_dev_get(const struct net_device *ol_dev);
 int mlxsw_sp_ipip_ecn_decap_init(struct mlxsw_sp *mlxsw_sp);
 struct net_device *
 mlxsw_sp_ipip_netdev_ul_dev_get(const struct net_device *ol_dev);

+int mlxsw_sp_router_bridge_vlan_add(struct mlxsw_sp *mlxsw_sp,
+                                   struct net_device *dev,
+                                   u16 new_vid, bool is_pvid,
+                                   struct netlink_ext_ack *extack);

 int mlxsw_sp_router_port_join_lag(struct mlxsw_sp_port *mlxsw_sp_port,
                                  struct net_device *lag_dev,
                                  struct netlink_ext_ack *extack);
 int mlxsw_sp_router_port_join_lag(struct mlxsw_sp_port *mlxsw_sp_port,
                                  struct net_device *lag_dev,
                                  struct netlink_ext_ack *extack);

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index a3365f7437d66cd806dd09d8767e7c23e1a19ad0..79d45c6c6edf6746665d2561cf214b1862ab8d61 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1479,30 +1479,15 @@ err_port_vlan_set:
 }
 
 static int
 }
 
 static int

-mlxsw_sp_br_ban_rif_pvid_change(struct mlxsw_sp *mlxsw_sp,
-                               const struct net_device *br_dev,
-                               const struct switchdev_obj_port_vlan *vlan,
-                               struct netlink_ext_ack *extack)
+mlxsw_sp_br_rif_pvid_change(struct mlxsw_sp *mlxsw_sp,
+                           struct net_device *br_dev,
+                           const struct switchdev_obj_port_vlan *vlan,
+                           struct netlink_ext_ack *extack)

 {
 {

-       u16 pvid;
-
-       pvid = mlxsw_sp_rif_vid(mlxsw_sp, br_dev);
-       if (!pvid)
-               return 0;
-
-       if (vlan->flags & BRIDGE_VLAN_INFO_PVID) {
-               if (vlan->vid != pvid) {
-                       NL_SET_ERR_MSG_MOD(extack, "Can't change PVID, it's used by router interface");
-                       return -EBUSY;
-               }
-       } else {
-               if (vlan->vid == pvid) {
-                       NL_SET_ERR_MSG_MOD(extack, "Can't remove PVID, it's used by router interface");
-                       return -EBUSY;
-               }
-       }
+       bool flag_pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;

 
 

-       return 0;
+       return mlxsw_sp_router_bridge_vlan_add(mlxsw_sp, br_dev, vlan->vid,
+                                              flag_pvid, extack);

 }
 
 static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,


@@ -1519,9 +1504,8 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
                int err = 0;
 
                if (br_vlan_enabled(orig_dev))
                int err = 0;
 
                if (br_vlan_enabled(orig_dev))

-                       err = mlxsw_sp_br_ban_rif_pvid_change(mlxsw_sp,
-                                                             orig_dev, vlan,
-                                                             extack);
+                       err = mlxsw_sp_br_rif_pvid_change(mlxsw_sp, orig_dev,
+                                                         vlan, extack);

                if (!err)
                        err = -EOPNOTSUPP;
                return err;
                if (!err)
                        err = -EOPNOTSUPP;
                return err;