net: switchdev: pass flags and mask to both {PRE_,}BRIDGE_FLAGS attributes

This switchdev attribute offers a counterproductive API for a driver
writer, because although br_switchdev_set_port_flag gets passed a
"flags" and a "mask", those are passed piecemeal to the driver, so while
the PRE_BRIDGE_FLAGS listener knows what changed because it has the
"mask", the BRIDGE_FLAGS listener doesn't, because it only has the final
value. But certain drivers can offload only certain combinations of
settings, like for example they cannot change unicast flooding
independently of multicast flooding - they must be both on or both off.
The way the information is passed to switchdev makes drivers not
expressive enough, and unable to reject this request ahead of time, in
the PRE_BRIDGE_FLAGS notifier, so they are forced to reject it during
the deferred BRIDGE_FLAGS attribute, where the rejection is currently
ignored.

This patch also changes drivers to make use of the "mask" field for edge
detection when possible.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
index 2c1619715a4b443a559eed172803b4de6681931b..49e052273f305cd6867458cdb3e3aea3577d3b46 100644 (file)
--- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
@@ -581,7 +581,7 @@ int prestera_bridge_port_event(struct net_device *dev, unsigned long event,
 
 static int prestera_port_attr_br_flags_set(struct prestera_port *port,
                                           struct net_device *dev,
-                                          unsigned long flags)
+                                          struct switchdev_brport_flags flags)
 {
        struct prestera_bridge_port *br_port;
        int err;
@@ -590,15 +590,20 @@ static int prestera_port_attr_br_flags_set(struct prestera_port *port,
        if (!br_port)
                return 0;
 
-       err = prestera_hw_port_flood_set(port, flags & BR_FLOOD);
-       if (err)
-               return err;
+       if (flags.mask & BR_FLOOD) {
+               err = prestera_hw_port_flood_set(port, flags.val & BR_FLOOD);
+               if (err)
+                       return err;
+       }
 
-       err = prestera_hw_port_learning_set(port, flags & BR_LEARNING);
-       if (err)
-               return err;
+       if (flags.mask & BR_LEARNING) {
+               err = prestera_hw_port_learning_set(port,
+                                                   flags.val & BR_LEARNING);
+               if (err)
+                       return err;
+       }
 
-       memcpy(&br_port->flags, &flags, sizeof(flags));
+       memcpy(&br_port->flags, &flags.val, sizeof(flags.val));
 
        return 0;
 }
@@ -707,7 +712,7 @@ static int prestera_port_obj_attr_set(struct net_device *dev,
                                                       attr->u.stp_state);
                break;
        case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
-               if (attr->u.brport_flags &
+               if (attr->u.brport_flags.mask &
                    ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
                        err = -EINVAL;
                break;

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 18e4f1cd5587d96b22f7820016dea37bba63586a..23b7e8d6386bfa2293d03a855504bb9eb9ceab0d 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -653,11 +653,11 @@ err_port_bridge_vlan_learning_set:
        return err;
 }
 
-static int mlxsw_sp_port_attr_br_pre_flags_set(struct mlxsw_sp_port
-                                              *mlxsw_sp_port,
-                                              unsigned long brport_flags)
+static int
+mlxsw_sp_port_attr_br_pre_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
+                                   struct switchdev_brport_flags flags)
 {
-       if (brport_flags & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
+       if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
                return -EINVAL;
 
        return 0;
@@ -665,7 +665,7 @@ static int mlxsw_sp_port_attr_br_pre_flags_set(struct mlxsw_sp_port
 
 static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
                                           struct net_device *orig_dev,
-                                          unsigned long brport_flags)
+                                          struct switchdev_brport_flags flags)
 {
        struct mlxsw_sp_bridge_port *bridge_port;
        int err;
@@ -675,29 +675,37 @@ static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
        if (!bridge_port)
                return 0;
 
-       err = mlxsw_sp_bridge_port_flood_table_set(mlxsw_sp_port, bridge_port,
-                                                  MLXSW_SP_FLOOD_TYPE_UC,
-                                                  brport_flags & BR_FLOOD);
-       if (err)
-               return err;
+       if (flags.mask & BR_FLOOD) {
+               err = mlxsw_sp_bridge_port_flood_table_set(mlxsw_sp_port,
+                                                          bridge_port,
+                                                          MLXSW_SP_FLOOD_TYPE_UC,
+                                                          flags.val & BR_FLOOD);
+               if (err)
+                       return err;
+       }
 
-       err = mlxsw_sp_bridge_port_learning_set(mlxsw_sp_port, bridge_port,
-                                               brport_flags & BR_LEARNING);
-       if (err)
-               return err;
+       if (flags.mask & BR_LEARNING) {
+               err = mlxsw_sp_bridge_port_learning_set(mlxsw_sp_port,
+                                                       bridge_port,
+                                                       flags.val & BR_LEARNING);
+               if (err)
+                       return err;
+       }
 
        if (bridge_port->bridge_device->multicast_enabled)
                goto out;
 
-       err = mlxsw_sp_bridge_port_flood_table_set(mlxsw_sp_port, bridge_port,
-                                                  MLXSW_SP_FLOOD_TYPE_MC,
-                                                  brport_flags &
-                                                  BR_MCAST_FLOOD);
-       if (err)
-               return err;
+       if (flags.mask & BR_MCAST_FLOOD) {
+               err = mlxsw_sp_bridge_port_flood_table_set(mlxsw_sp_port,
+                                                          bridge_port,
+                                                          MLXSW_SP_FLOOD_TYPE_MC,
+                                                          flags.val & BR_MCAST_FLOOD);
+               if (err)
+                       return err;
+       }
 
 out:
-       memcpy(&bridge_port->flags, &brport_flags, sizeof(brport_flags));
+       memcpy(&bridge_port->flags, &flags.val, sizeof(flags.val));
        return 0;
 }
 

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 740a715c49c6d86cfa33989d61ccf08e0a4a6663..3473d296b2e2e70ddb788d7f2173f1b5c521cb12 100644 (file)
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -1576,7 +1576,7 @@ rocker_world_port_attr_bridge_flags_support_get(const struct rocker_port *
 
 static int
 rocker_world_port_attr_pre_bridge_flags_set(struct rocker_port *rocker_port,
-                                           unsigned long brport_flags)
+                                           struct switchdev_brport_flags flags)
 {
        struct rocker_world_ops *wops = rocker_port->rocker->wops;
        unsigned long brport_flags_s;
@@ -1590,7 +1590,7 @@ rocker_world_port_attr_pre_bridge_flags_set(struct rocker_port *rocker_port,
        if (err)
                return err;
 
-       if (brport_flags & ~brport_flags_s)
+       if (flags.mask & ~brport_flags_s)
                return -EINVAL;
 
        return 0;
@@ -1598,14 +1598,14 @@ rocker_world_port_attr_pre_bridge_flags_set(struct rocker_port *rocker_port,
 
 static int
 rocker_world_port_attr_bridge_flags_set(struct rocker_port *rocker_port,
-                                       unsigned long brport_flags)
+                                       struct switchdev_brport_flags flags)
 {
        struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
        if (!wops->port_attr_bridge_flags_set)
                return -EOPNOTSUPP;
 
-       return wops->port_attr_bridge_flags_set(rocker_port, brport_flags);
+       return wops->port_attr_bridge_flags_set(rocker_port, flags.val);
 }
 
 static int
@@ -2058,7 +2058,7 @@ static int rocker_port_attr_set(struct net_device *dev,
                break;
        case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
                err = rocker_world_port_attr_pre_bridge_flags_set(rocker_port,
-                                                             attr->u.brport_flags);
+                                                                 attr->u.brport_flags);
                break;
        case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
                err = rocker_world_port_attr_bridge_flags_set(rocker_port,

diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
index 314825acf0a0972914c9519e6f6eb128ccb90fe4..d93ffd8a08b0bd0aa008a301672afafa680d1969 100644 (file)
--- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
@@ -55,26 +55,30 @@ static int am65_cpsw_port_stp_state_set(struct am65_cpsw_port *port, u8 state)
 
 static int am65_cpsw_port_attr_br_flags_set(struct am65_cpsw_port *port,
                                            struct net_device *orig_dev,
-                                           unsigned long brport_flags)
+                                           struct switchdev_brport_flags flags)
 {
        struct am65_cpsw_common *cpsw = port->common;
-       bool unreg_mcast_add = false;
 
-       if (brport_flags & BR_MCAST_FLOOD)
-               unreg_mcast_add = true;
-       netdev_dbg(port->ndev, "BR_MCAST_FLOOD: %d port %u\n",
-                  unreg_mcast_add, port->port_id);
+       if (flags.mask & BR_MCAST_FLOOD) {
+               bool unreg_mcast_add = false;
 
-       cpsw_ale_set_unreg_mcast(cpsw->ale, BIT(port->port_id),
-                                unreg_mcast_add);
+               if (flags.val & BR_MCAST_FLOOD)
+                       unreg_mcast_add = true;
+
+               netdev_dbg(port->ndev, "BR_MCAST_FLOOD: %d port %u\n",
+                          unreg_mcast_add, port->port_id);
+
+               cpsw_ale_set_unreg_mcast(cpsw->ale, BIT(port->port_id),
+                                        unreg_mcast_add);
+       }
 
        return 0;
 }
 
 static int am65_cpsw_port_attr_br_flags_pre_set(struct net_device *netdev,
-                                               unsigned long flags)
+                                               struct switchdev_brport_flags flags)
 {
-       if (flags & ~(BR_LEARNING | BR_MCAST_FLOOD))
+       if (flags.mask & ~(BR_LEARNING | BR_MCAST_FLOOD))
                return -EINVAL;
 
        return 0;

diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
index 13524cbaa8b6a872df44a7987c42b1db39dc6972..a72bb570756fd073eae0d6286d43b3504ebe84cc 100644 (file)
--- a/drivers/net/ethernet/ti/cpsw_switchdev.c
+++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
@@ -57,26 +57,30 @@ static int cpsw_port_stp_state_set(struct cpsw_priv *priv, u8 state)
 
 static int cpsw_port_attr_br_flags_set(struct cpsw_priv *priv,
                                       struct net_device *orig_dev,
-                                      unsigned long brport_flags)
+                                      struct switchdev_brport_flags flags)
 {
        struct cpsw_common *cpsw = priv->cpsw;
-       bool unreg_mcast_add = false;
 
-       if (brport_flags & BR_MCAST_FLOOD)
-               unreg_mcast_add = true;
-       dev_dbg(priv->dev, "BR_MCAST_FLOOD: %d port %u\n",
-               unreg_mcast_add, priv->emac_port);
+       if (flags.mask & BR_MCAST_FLOOD) {
+               bool unreg_mcast_add = false;
 
-       cpsw_ale_set_unreg_mcast(cpsw->ale, BIT(priv->emac_port),
-                                unreg_mcast_add);
+               if (flags.val & BR_MCAST_FLOOD)
+                       unreg_mcast_add = true;
+
+               dev_dbg(priv->dev, "BR_MCAST_FLOOD: %d port %u\n",
+                       unreg_mcast_add, priv->emac_port);
+
+               cpsw_ale_set_unreg_mcast(cpsw->ale, BIT(priv->emac_port),
+                                        unreg_mcast_add);
+       }
 
        return 0;
 }
 
 static int cpsw_port_attr_br_flags_pre_set(struct net_device *netdev,
-                                          unsigned long flags)
+                                          struct switchdev_brport_flags flags)
 {
-       if (flags & ~(BR_LEARNING | BR_MCAST_FLOOD))
+       if (flags.mask & ~(BR_LEARNING | BR_MCAST_FLOOD))
                return -EINVAL;
 
        return 0;

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index ca3d07fe7f58ed964f9ced9b4a740e9c22eefc28..703055e063ffbcc6e001ab3c57f0c45ddabbab85 100644 (file)
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -908,31 +908,39 @@ static int dpaa2_switch_port_attr_stp_state_set(struct net_device *netdev,
        return dpaa2_switch_port_set_stp_state(port_priv, state);
 }
 
-static int dpaa2_switch_port_attr_br_flags_pre_set(struct net_device *netdev,
-                                                  unsigned long flags)
+static int
+dpaa2_switch_port_attr_br_flags_pre_set(struct net_device *netdev,
+                                       struct switchdev_brport_flags flags)
 {
-       if (flags & ~(BR_LEARNING | BR_FLOOD))
+       if (flags.mask & ~(BR_LEARNING | BR_FLOOD))
                return -EINVAL;
 
        return 0;
 }
 
-static int dpaa2_switch_port_attr_br_flags_set(struct net_device *netdev,
-                                              unsigned long flags)
+static int
+dpaa2_switch_port_attr_br_flags_set(struct net_device *netdev,
+                                   struct switchdev_brport_flags flags)
 {
        struct ethsw_port_priv *port_priv = netdev_priv(netdev);
        int err = 0;
 
-       /* Learning is enabled per switch */
-       err = dpaa2_switch_set_learning(port_priv->ethsw_data,
-                                       !!(flags & BR_LEARNING));
-       if (err)
-               goto exit;
+       if (flags.mask & BR_LEARNING) {
+               /* Learning is enabled per switch */
+               err = dpaa2_switch_set_learning(port_priv->ethsw_data,
+                                               !!(flags.val & BR_LEARNING));
+               if (err)
+                       return err;
+       }
 
-       err = dpaa2_switch_port_set_flood(port_priv, !!(flags & BR_FLOOD));
+       if (flags.mask & BR_FLOOD) {
+               err = dpaa2_switch_port_set_flood(port_priv,
+                                                 !!(flags.val & BR_FLOOD));
+               if (err)
+                       return err;
+       }
 
-exit:
-       return err;
+       return 0;
 }
 
 static int dpaa2_switch_port_attr_set(struct net_device *netdev,

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 9279d4245babee6510be34c25e22a3ec6f175685..25d9e45709344de850bdbb1c3b4992c58f70aa56 100644 (file)
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -32,6 +32,11 @@ enum switchdev_attr_id {
 #endif
 };
 
+struct switchdev_brport_flags {
+       unsigned long val;
+       unsigned long mask;
+};
+
 struct switchdev_attr {
        struct net_device *orig_dev;
        enum switchdev_attr_id id;
@@ -40,7 +45,7 @@ struct switchdev_attr {
        void (*complete)(struct net_device *dev, int err, void *priv);
        union {
                u8 stp_state;                           /* PORT_STP_STATE */
-               unsigned long brport_flags;             /* PORT_{PRE}_BRIDGE_FLAGS */
+               struct switchdev_brport_flags brport_flags; /* PORT_BRIDGE_FLAGS */
                bool mrouter;                           /* PORT_MROUTER */
                clock_t ageing_time;                    /* BRIDGE_AGEING_TIME */
                bool vlan_filtering;                    /* BRIDGE_VLAN_FILTERING */

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index bb21dd35ae85ee4953ea3ef075abc1b587af6f30..184cf4c8b06d93210727b4eebff29842fe87448c 100644 (file)
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -65,7 +65,6 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
 {
        struct switchdev_attr attr = {
                .orig_dev = p->dev,
-               .id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
        };
        struct switchdev_notifier_port_attr_info info = {
                .attr = &attr,
@@ -76,7 +75,9 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
        if (!mask)
                return 0;
 
-       attr.u.brport_flags = mask;
+       attr.id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS;
+       attr.u.brport_flags.val = flags;
+       attr.u.brport_flags.mask = mask;
 
        /* We run from atomic context here */
        err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
@@ -94,7 +95,6 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
 
        attr.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS;
        attr.flags = SWITCHDEV_F_DEFER;
-       attr.u.brport_flags = flags;
 
        err = switchdev_port_attr_set(p->dev, &attr);
        if (err) {

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7060f128386b704092e5c478a4bb074c4349fc28..bc835f3de2be088a81e3a6a5911938a3274ed2da 100644 (file)
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -183,8 +183,10 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
                     const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_mdb_del(const struct dsa_port *dp,
                     const struct switchdev_obj_port_mdb *mdb);
-int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags);
-int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags);
+int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
+                             struct switchdev_brport_flags flags);
+int dsa_port_bridge_flags(const struct dsa_port *dp,
+                         struct switchdev_brport_flags flags);
 int dsa_port_mrouter(struct dsa_port *dp, bool mrouter);
 int dsa_port_vlan_add(struct dsa_port *dp,
                      const struct switchdev_obj_port_vlan *vlan);

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1f877bf21bb491a4aae2770acb52b6d5d4a5f73e..368064dfd93e9a3520cec5c7643f57a3d729c861 100644 (file)
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -125,21 +125,22 @@ void dsa_port_disable(struct dsa_port *dp)
 static void dsa_port_change_brport_flags(struct dsa_port *dp,
                                         bool bridge_offload)
 {
-       unsigned long mask, flags;
-       int flag, err;
+       struct switchdev_brport_flags flags;
+       int flag;
 
-       mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD;
+       flags.mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD;
        if (bridge_offload)
-               flags = mask;
+               flags.val = flags.mask;
        else
-               flags = mask & ~BR_LEARNING;
+               flags.val = flags.mask & ~BR_LEARNING;
 
-       for_each_set_bit(flag, &mask, 32) {
-               err = dsa_port_pre_bridge_flags(dp, BIT(flag));
-               if (err)
-                       continue;
+       for_each_set_bit(flag, &flags.mask, 32) {
+               struct switchdev_brport_flags tmp;
+
+               tmp.val = flags.val & BIT(flag);
+               tmp.mask = BIT(flag);
 
-               dsa_port_bridge_flags(dp, flags & BIT(flag));
+               dsa_port_bridge_flags(dp, tmp);
        }
 }
 
@@ -423,26 +424,29 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
        return 0;
 }
 
-int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags)
+int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
+                             struct switchdev_brport_flags flags)
 {
        struct dsa_switch *ds = dp->ds;
 
        if (!ds->ops->port_egress_floods ||
-           (flags & ~(BR_FLOOD | BR_MCAST_FLOOD)))
+           (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD)))
                return -EINVAL;
 
        return 0;
 }
 
-int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags)
+int dsa_port_bridge_flags(const struct dsa_port *dp,
+                         struct switchdev_brport_flags flags)
 {
        struct dsa_switch *ds = dp->ds;
        int port = dp->index;
        int err = 0;
 
        if (ds->ops->port_egress_floods)
-               err = ds->ops->port_egress_floods(ds, port, flags & BR_FLOOD,
-                                                 flags & BR_MCAST_FLOOD);
+               err = ds->ops->port_egress_floods(ds, port,
+                                                 flags.val & BR_FLOOD,
+                                                 flags.val & BR_MCAST_FLOOD);
 
        return err;
 }