Hi all,
This patch series splits the removal of the switchdev_ops that was
proposed a few times before and first tackles the easy part which is the
removal of the single call to switchdev_port_attr_get() within the
bridge code.
As suggestd by Ido, this patch series adds a
SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS which is used in the same
context as the caller of switchdev_port_attr_set(), so not deferred, and
then the operation is carried out in deferred context with setting a
support bridge port flag.
Follow-up patches will do the switchdev_ops removal after introducing
the proper helpers for the switchdev blocking notifier to work across
stacked devices (unlike the previous submissions).
Florian Fainelli (9):
Documentation: networking: switchdev: Update port parent ID section
net: switchdev: Add PORT_PRE_BRIDGE_FLAGS
mlxsw: spectrum: Check bridge flags during prepare phase
staging: fsl-dpaa2: ethsw: Check bridge port flags during prepare
net: dsa: Add setter for SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
rocker: Check bridge flags during prepare phase
net: bridge: Stop calling switchdev_port_attr_get()
net: Remove SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
net: Get rid of switchdev_port_attr_get()
Documentation/networking/switchdev.txt | 15 ++---
.../mellanox/mlxsw/spectrum_switchdev.c | 30 +++------
drivers/net/ethernet/rocker/rocker_main.c | 67 +++++++++----------
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 27 +++-----
include/net/switchdev.h | 13 +---
net/bridge/br_switchdev.c | 16 ++---
net/dsa/dsa_priv.h | 3 +
net/dsa/port.c | 11 +++
net/dsa/slave.c | 22 ++----
9 files changed, 88 insertions(+), 116 deletions(-)
--
2.17.1
Update the section about switchdev drivers having to implement a
switchdev_port_attr_get() function to return
SWITCHDEV_ATTR_ID_PORT_PARENT_ID since that is no longer valid after
commit bccb30254a4a ("net: Get rid of
SWITCHDEV_ATTR_ID_PORT_PARENT_ID").
Fixes: bccb30254a4a ("net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID")
Acked-by: Jiri Pirko <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/networking/switchdev.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index f3244d87512a..ea90243340a9 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -92,11 +92,11 @@ device.
Switch ID
^^^^^^^^^
-The switchdev driver must implement the switchdev op switchdev_port_attr_get
-for SWITCHDEV_ATTR_ID_PORT_PARENT_ID for each port netdev, returning the same
-physical ID for each port of a switch. The ID must be unique between switches
-on the same system. The ID does not need to be unique between switches on
-different systems.
+The switchdev driver must implement the net_device operation
+ndo_get_port_parent_id for each port netdev, returning the same physical ID for
+each port of a switch. The ID must be unique between switches on the same
+system. The ID does not need to be unique between switches on different
+systems.
The switch ID is used to locate ports on a switch and to know if aggregated
ports belong to the same switch.
--
2.17.1
In preparation for removing switchdev_port_attr_get(), introduce
PORT_PRE_BRIDGE_FLAGS which will be called through
switchdev_port_attr_set(), in the caller's context (possibly atomic) and
which must be checked by the switchdev driver in order to return whether
the operation is supported or not.
This is entirely analoguous to how the BRIDGE_FLAGS_SUPPORT works,
except it goes through a set() instead of get().
Signed-off-by: Florian Fainelli <[email protected]>
---
include/net/switchdev.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 5e87b54c5dc5..de72b0a3867f 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -46,6 +46,7 @@ enum switchdev_attr_id {
SWITCHDEV_ATTR_ID_PORT_STP_STATE,
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
+ SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
SWITCHDEV_ATTR_ID_PORT_MROUTER,
SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME,
SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
@@ -61,7 +62,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_BRIDGE_FLAGS */
+ unsigned long brport_flags; /* PORT_{PRE}_BRIDGE_FLAGS */
unsigned long brport_flags_support; /* PORT_BRIDGE_FLAGS_SUPPORT */
bool mrouter; /* PORT_MROUTER */
clock_t ageing_time; /* BRIDGE_AGEING_TIME */
--
2.17.1
In preparation for getting rid of switchdev_port_attr_get(), have mlxsw
check for the bridge flags being set through switchdev_port_attr_set()
when the SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier is
used.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 1f492b7dbea8..7616eab50035 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -598,13 +598,17 @@ mlxsw_sp_bridge_port_learning_set(struct mlxsw_sp_port *mlxsw_sp_port,
static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct switchdev_trans *trans,
struct net_device *orig_dev,
- unsigned long brport_flags)
+ unsigned long brport_flags,
+ bool pre_set)
{
struct mlxsw_sp_bridge_port *bridge_port;
int err;
- if (switchdev_trans_ph_prepare(trans))
+ if (switchdev_trans_ph_prepare(trans) && pre_set) {
+ if (brport_flags & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
+ return -EOPNOTSUPP;
return 0;
+ }
bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
orig_dev);
@@ -833,6 +837,7 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
struct switchdev_trans *trans)
{
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
+ bool pre_set = false;
int err;
switch (attr->id) {
@@ -841,10 +846,13 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
attr->orig_dev,
attr->u.stp_state);
break;
+ case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
+ pre_set = true; /* fall through */
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
err = mlxsw_sp_port_attr_br_flags_set(mlxsw_sp_port, trans,
attr->orig_dev,
- attr->u.brport_flags);
+ attr->u.brport_flags,
+ pre_set);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
err = mlxsw_sp_port_attr_br_ageing_set(mlxsw_sp_port, trans,
--
2.17.1
In preparation for removing SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
add support for a function that processes the
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS and
SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attributes and returns not
supported for any flag set, since DSA does not currently support
toggling those bridge port attributes (yet).
Signed-off-by: Florian Fainelli <[email protected]>
---
net/dsa/dsa_priv.h | 3 +++
net/dsa/port.c | 11 +++++++++++
net/dsa/slave.c | 7 +++++++
3 files changed, 21 insertions(+)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1f4972dab9f2..50d73698dfb3 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -150,6 +150,9 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
struct switchdev_trans *trans);
int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
struct switchdev_trans *trans);
+int dsa_port_bridge_port_flags_set(struct dsa_port *dp,
+ unsigned long brport_flags,
+ struct switchdev_trans *trans, bool pre_set);
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..b0c4cfd18da9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -177,6 +177,17 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
}
+int dsa_port_bridge_port_flags_set(struct dsa_port *dp,
+ unsigned long brport_flags,
+ struct switchdev_trans *trans,
+ bool pre_set)
+{
+ if (brport_flags)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid)
{
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..417388c9f1fa 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -282,6 +282,7 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
struct switchdev_trans *trans)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
+ bool pre_set = false;
int ret;
switch (attr->id) {
@@ -295,6 +296,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
break;
+ case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
+ pre_set = true; /* fall through */
+ case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
+ ret = dsa_port_bridge_port_flags_set(dp, attr->u.brport_flags,
+ trans, pre_set);
+ break;
default:
ret = -EOPNOTSUPP;
break;
--
2.17.1
In preparation for removing SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
have ethsw check that the bridge port flags that are being set are
supported when SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS is specified.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 1b3943b71254..f788a9458b89 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -668,13 +668,16 @@ static int port_attr_stp_state_set(struct net_device *netdev,
static int port_attr_br_flags_set(struct net_device *netdev,
struct switchdev_trans *trans,
- unsigned long flags)
+ unsigned long flags, bool pre_set)
{
struct ethsw_port_priv *port_priv = netdev_priv(netdev);
int err = 0;
- if (switchdev_trans_ph_prepare(trans))
+ if (switchdev_trans_ph_prepare(trans) && pre_set) {
+ if (flags & ~(BR_LEARNING | BR_FLOOD))
+ return -EOPNOTSUPP;
return 0;
+ }
/* Learning is enabled per switch */
err = ethsw_set_learning(port_priv->ethsw_data, flags & BR_LEARNING);
@@ -691,6 +694,7 @@ static int swdev_port_attr_set(struct net_device *netdev,
const struct switchdev_attr *attr,
struct switchdev_trans *trans)
{
+ bool pre_set = false;
int err = 0;
switch (attr->id) {
@@ -698,9 +702,11 @@ static int swdev_port_attr_set(struct net_device *netdev,
err = port_attr_stp_state_set(netdev, trans,
attr->u.stp_state);
break;
+ case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
+ pre_set = true; /* fall through */
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
err = port_attr_br_flags_set(netdev, trans,
- attr->u.brport_flags);
+ attr->u.brport_flags, pre_set);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
/* VLANs are supported by default */
--
2.17.1
In preparation for getting rid of switchdev_port_attr_get(), have rocker
check for the bridge flags being set through switchdev_port_attr_set()
with the SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/net/ethernet/rocker/rocker_main.c | 48 +++++++++++++++--------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 5ce8d5aba603..863a8b32e6e9 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -1565,37 +1565,48 @@ static int rocker_world_port_attr_stp_state_set(struct rocker_port *rocker_port,
return wops->port_attr_stp_state_set(rocker_port, state);
}
+static int
+rocker_world_port_attr_bridge_flags_support_get(const struct rocker_port *
+ rocker_port,
+ unsigned long *
+ p_brport_flags_support)
+{
+ struct rocker_world_ops *wops = rocker_port->rocker->wops;
+
+ if (!wops->port_attr_bridge_flags_support_get)
+ return -EOPNOTSUPP;
+ return wops->port_attr_bridge_flags_support_get(rocker_port,
+ p_brport_flags_support);
+}
+
static int
rocker_world_port_attr_bridge_flags_set(struct rocker_port *rocker_port,
unsigned long brport_flags,
- struct switchdev_trans *trans)
+ struct switchdev_trans *trans,
+ bool pre_set)
{
struct rocker_world_ops *wops = rocker_port->rocker->wops;
+ unsigned long brport_flags_s;
+ int err;
if (!wops->port_attr_bridge_flags_set)
return -EOPNOTSUPP;
- if (switchdev_trans_ph_prepare(trans))
+ if (switchdev_trans_ph_prepare(trans) && pre_set) {
+ err = rocker_world_port_attr_bridge_flags_support_get(rocker_port,
+ &brport_flags_s);
+ if (err)
+ return err;
+
+ if (brport_flags & ~brport_flags_s)
+ return -EOPNOTSUPP;
return 0;
+ }
return wops->port_attr_bridge_flags_set(rocker_port, brport_flags,
trans);
}
-static int
-rocker_world_port_attr_bridge_flags_support_get(const struct rocker_port *
- rocker_port,
- unsigned long *
- p_brport_flags_support)
-{
- struct rocker_world_ops *wops = rocker_port->rocker->wops;
-
- if (!wops->port_attr_bridge_flags_support_get)
- return -EOPNOTSUPP;
- return wops->port_attr_bridge_flags_support_get(rocker_port,
- p_brport_flags_support);
-}
-
static int
rocker_world_port_attr_bridge_ageing_time_set(struct rocker_port *rocker_port,
u32 ageing_time,
@@ -2066,6 +2077,7 @@ static int rocker_port_attr_set(struct net_device *dev,
struct switchdev_trans *trans)
{
struct rocker_port *rocker_port = netdev_priv(dev);
+ bool pre_set = false;
int err = 0;
switch (attr->id) {
@@ -2074,10 +2086,12 @@ static int rocker_port_attr_set(struct net_device *dev,
attr->u.stp_state,
trans);
break;
+ case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
+ pre_set = true; /* fall through */
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
err = rocker_world_port_attr_bridge_flags_set(rocker_port,
attr->u.brport_flags,
- trans);
+ trans, pre_set);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
err = rocker_world_port_attr_bridge_ageing_time_set(rocker_port,
--
2.17.1
Now that all switchdev drivers have been converted to checking the
bridge port flags during the prepare phase of the
switchdev_port_attr_set() when the process
SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS, we can avoid calling
switchdev_port_attr_get() with
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT.
Signed-off-by: Florian Fainelli <[email protected]>
---
net/bridge/br_switchdev.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index db9e8ab96d48..8f88f8a1a7fa 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -64,29 +64,27 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
{
struct switchdev_attr attr = {
.orig_dev = p->dev,
- .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
+ .id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
+ .u.brport_flags = flags,
};
int err;
if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
return 0;
- err = switchdev_port_attr_get(p->dev, &attr);
- if (err == -EOPNOTSUPP)
- return 0;
- if (err)
+ err = switchdev_port_attr_set(p->dev, &attr);
+ if (err && err != -EOPNOTSUPP)
return err;
- /* Check if specific bridge flag attribute offload is supported */
- if (!(attr.u.brport_flags_support & mask)) {
+ if (err == -EOPNOTSUPP) {
br_warn(p->br, "bridge flag offload is not supported %u(%s)\n",
(unsigned int)p->port_no, p->dev->name);
- return -EOPNOTSUPP;
+ return err;
}
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) {
br_warn(p->br, "error setting offload flag on port %u(%s)\n",
--
2.17.1
Now that we have converted the bridge code and the drivers to check for
bridge port(s) flags at the time we try to set them, there is no need
for a get() -> set() sequence anymore and
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT therefore becomes unused.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 11 +----------
drivers/net/ethernet/rocker/rocker_main.c | 14 +-------------
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 10 +---------
include/net/switchdev.h | 2 --
net/dsa/slave.c | 10 +---------
5 files changed, 4 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 7616eab50035..c11cf7fa4863 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -434,16 +434,7 @@ static void mlxsw_sp_bridge_vlan_put(struct mlxsw_sp_bridge_vlan *bridge_vlan)
static int mlxsw_sp_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
- switch (attr->id) {
- case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
- attr->u.brport_flags_support = BR_LEARNING | BR_FLOOD |
- BR_MCAST_FLOOD;
- break;
- default:
- return -EOPNOTSUPP;
- }
-
- return 0;
+ return -EOPNOTSUPP;
}
static int
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 863a8b32e6e9..8e80301eae7b 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2057,19 +2057,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
static int rocker_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
- const struct rocker_port *rocker_port = netdev_priv(dev);
- int err = 0;
-
- switch (attr->id) {
- case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
- err = rocker_world_port_attr_bridge_flags_support_get(rocker_port,
- &attr->u.brport_flags_support);
- break;
- default:
- return -EOPNOTSUPP;
- }
-
- return err;
+ return -EOPNOTSUPP;
}
static int rocker_port_attr_set(struct net_device *dev,
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index f788a9458b89..5f58c7df67bb 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -643,15 +643,7 @@ static void ethsw_teardown_irqs(struct fsl_mc_device *sw_dev)
static int swdev_port_attr_get(struct net_device *netdev,
struct switchdev_attr *attr)
{
- switch (attr->id) {
- case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
- attr->u.brport_flags_support = BR_LEARNING | BR_FLOOD;
- break;
- default:
- return -EOPNOTSUPP;
- }
-
- return 0;
+ return -EOPNOTSUPP;
}
static int port_attr_stp_state_set(struct net_device *netdev,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index de72b0a3867f..0f352019ef99 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -45,7 +45,6 @@ enum switchdev_attr_id {
SWITCHDEV_ATTR_ID_UNDEFINED,
SWITCHDEV_ATTR_ID_PORT_STP_STATE,
SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
- SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
SWITCHDEV_ATTR_ID_PORT_MROUTER,
SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME,
@@ -63,7 +62,6 @@ struct switchdev_attr {
union {
u8 stp_state; /* PORT_STP_STATE */
unsigned long brport_flags; /* PORT_{PRE}_BRIDGE_FLAGS */
- unsigned long brport_flags_support; /* PORT_BRIDGE_FLAGS_SUPPORT */
bool mrouter; /* PORT_MROUTER */
clock_t ageing_time; /* BRIDGE_AGEING_TIME */
bool vlan_filtering; /* BRIDGE_VLAN_FILTERING */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 417388c9f1fa..a176d3ba3b7a 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -388,15 +388,7 @@ static int dsa_slave_get_port_parent_id(struct net_device *dev,
static int dsa_slave_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
- switch (attr->id) {
- case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
- attr->u.brport_flags_support = 0;
- break;
- default:
- return -EOPNOTSUPP;
- }
-
- return 0;
+ return -EOPNOTSUPP;
}
static inline netdev_tx_t dsa_slave_netpoll_send_skb(struct net_device *dev,
--
2.17.1
With the bridge no longer calling switchdev_port_attr_get() to obtain
the supported bridge port flags from a driver but instead trying to set
the bridge port flags directly and relying on driver to reject
unsupported configurations, we can effectively get rid of
switchdev_port_attr_get() entirely since this was the only place where
it was called.
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/networking/switchdev.txt | 5 ++---
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 7 -------
drivers/net/ethernet/rocker/rocker_main.c | 7 -------
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 7 -------
include/net/switchdev.h | 8 --------
net/dsa/slave.c | 7 -------
6 files changed, 2 insertions(+), 39 deletions(-)
diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index ea90243340a9..327afe754230 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -233,9 +233,8 @@ the bridge's FDB. It's possible, but not optimal, to enable learning on the
device port and on the bridge port, and disable learning_sync.
To support learning and learning_sync port attributes, the driver implements
-switchdev op switchdev_port_attr_get/set for
-SWITCHDEV_ATTR_PORT_ID_BRIDGE_FLAGS. The driver should initialize the attributes
-to the hardware defaults.
+switchdev op switchdev_port_attr_set for SWITCHDEV_ATTR_PORT_ID_BRIDGE_FLAGS.
+The driver should initialize the attributes to the hardware defaults.
FDB Ageing
^^^^^^^^^^
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c11cf7fa4863..5263abe6224e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -431,12 +431,6 @@ static void mlxsw_sp_bridge_vlan_put(struct mlxsw_sp_bridge_vlan *bridge_vlan)
mlxsw_sp_bridge_vlan_destroy(bridge_vlan);
}
-static int mlxsw_sp_port_attr_get(struct net_device *dev,
- struct switchdev_attr *attr)
-{
- return -EOPNOTSUPP;
-}
-
static int
mlxsw_sp_port_bridge_vlan_stp_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp_bridge_vlan *bridge_vlan,
@@ -1937,7 +1931,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct mlxsw_sp *mlxsw_sp,
}
static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
- .switchdev_port_attr_get = mlxsw_sp_port_attr_get,
.switchdev_port_attr_set = mlxsw_sp_port_attr_set,
};
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 8e80301eae7b..5d06c7edf696 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2054,12 +2054,6 @@ static const struct net_device_ops rocker_port_netdev_ops = {
* swdev interface
********************/
-static int rocker_port_attr_get(struct net_device *dev,
- struct switchdev_attr *attr)
-{
- return -EOPNOTSUPP;
-}
-
static int rocker_port_attr_set(struct net_device *dev,
const struct switchdev_attr *attr,
struct switchdev_trans *trans)
@@ -2135,7 +2129,6 @@ static int rocker_port_obj_del(struct net_device *dev,
}
static const struct switchdev_ops rocker_port_switchdev_ops = {
- .switchdev_port_attr_get = rocker_port_attr_get,
.switchdev_port_attr_set = rocker_port_attr_set,
};
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 5f58c7df67bb..005c12c08df4 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -640,12 +640,6 @@ static void ethsw_teardown_irqs(struct fsl_mc_device *sw_dev)
fsl_mc_free_irqs(sw_dev);
}
-static int swdev_port_attr_get(struct net_device *netdev,
- struct switchdev_attr *attr)
-{
- return -EOPNOTSUPP;
-}
-
static int port_attr_stp_state_set(struct net_device *netdev,
struct switchdev_trans *trans,
u8 state)
@@ -924,7 +918,6 @@ static int swdev_port_obj_del(struct net_device *netdev,
}
static const struct switchdev_ops ethsw_port_switchdev_ops = {
- .switchdev_port_attr_get = swdev_port_attr_get,
.switchdev_port_attr_set = swdev_port_attr_set,
};
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 0f352019ef99..45310ddf2d7e 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -179,8 +179,6 @@ switchdev_notifier_info_to_extack(const struct switchdev_notifier_info *info)
#ifdef CONFIG_NET_SWITCHDEV
void switchdev_deferred_process(void);
-int switchdev_port_attr_get(struct net_device *dev,
- struct switchdev_attr *attr);
int switchdev_port_attr_set(struct net_device *dev,
const struct switchdev_attr *attr);
int switchdev_port_obj_add(struct net_device *dev,
@@ -225,12 +223,6 @@ static inline void switchdev_deferred_process(void)
{
}
-static inline int switchdev_port_attr_get(struct net_device *dev,
- struct switchdev_attr *attr)
-{
- return -EOPNOTSUPP;
-}
-
static inline int switchdev_port_attr_set(struct net_device *dev,
const struct switchdev_attr *attr)
{
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index a176d3ba3b7a..1258a0b7a158 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -385,12 +385,6 @@ static int dsa_slave_get_port_parent_id(struct net_device *dev,
return 0;
}
-static int dsa_slave_port_attr_get(struct net_device *dev,
- struct switchdev_attr *attr)
-{
- return -EOPNOTSUPP;
-}
-
static inline netdev_tx_t dsa_slave_netpoll_send_skb(struct net_device *dev,
struct sk_buff *skb)
{
@@ -1057,7 +1051,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
};
static const struct switchdev_ops dsa_slave_switchdev_ops = {
- .switchdev_port_attr_get = dsa_slave_port_attr_get,
.switchdev_port_attr_set = dsa_slave_port_attr_set,
};
--
2.17.1
On Wed, Feb 13, 2019 at 02:06:30PM -0800, Florian Fainelli wrote:
> Update the section about switchdev drivers having to implement a
> switchdev_port_attr_get() function to return
> SWITCHDEV_ATTR_ID_PORT_PARENT_ID since that is no longer valid after
> commit bccb30254a4a ("net: Get rid of
> SWITCHDEV_ATTR_ID_PORT_PARENT_ID").
>
> Fixes: bccb30254a4a ("net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID")
> Acked-by: Jiri Pirko <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
On Wed, Feb 13, 2019 at 02:06:36PM -0800, Florian Fainelli wrote:
> Now that all switchdev drivers have been converted to checking the
> bridge port flags during the prepare phase of the
> switchdev_port_attr_set() when the process
> SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS, we can avoid calling
> switchdev_port_attr_get() with
> SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> net/bridge/br_switchdev.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index db9e8ab96d48..8f88f8a1a7fa 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -64,29 +64,27 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
> {
> struct switchdev_attr attr = {
> .orig_dev = p->dev,
> - .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> + .id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
> + .u.brport_flags = flags,
> };
> int err;
>
> if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
> return 0;
>
> - err = switchdev_port_attr_get(p->dev, &attr);
> - if (err == -EOPNOTSUPP)
> - return 0;
> - if (err)
> + err = switchdev_port_attr_set(p->dev, &attr);
> + if (err && err != -EOPNOTSUPP)
> return err;
>
> - /* Check if specific bridge flag attribute offload is supported */
> - if (!(attr.u.brport_flags_support & mask)) {
> + if (err == -EOPNOTSUPP) {
> br_warn(p->br, "bridge flag offload is not supported %u(%s)\n",
> (unsigned int)p->port_no, p->dev->name);
> - return -EOPNOTSUPP;
> + return err;
> }
I see that you return -EOPNOTSUPP from drivers in case of unsupported
flags. I believe this is problematic (I'll test soon). The same return
code is used by:
1. Switch drivers to indicate unsupported flags
2. switchdev code to indicate unsupported netdev (no switchdev ops)
I guess that with this patch any attempt to set bridge port flags on
veth/dummy device will result in an error.
>
> 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) {
> br_warn(p->br, "error setting offload flag on port %u(%s)\n",
> --
> 2.17.1
>
On Wed, Feb 13, 2019 at 02:06:37PM -0800, Florian Fainelli wrote:
> Now that we have converted the bridge code and the drivers to check for
> bridge port(s) flags at the time we try to set them, there is no need
> for a get() -> set() sequence anymore and
> SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT therefore becomes unused.
>
> Signed-off-by: Florian Fainelli <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
On Wed, Feb 13, 2019 at 02:06:38PM -0800, Florian Fainelli wrote:
> With the bridge no longer calling switchdev_port_attr_get() to obtain
> the supported bridge port flags from a driver but instead trying to set
> the bridge port flags directly and relying on driver to reject
> unsupported configurations, we can effectively get rid of
> switchdev_port_attr_get() entirely since this was the only place where
> it was called.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> Documentation/networking/switchdev.txt | 5 ++---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 7 -------
> drivers/net/ethernet/rocker/rocker_main.c | 7 -------
> drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 7 -------
> include/net/switchdev.h | 8 --------
> net/dsa/slave.c | 7 -------
> 6 files changed, 2 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
> index ea90243340a9..327afe754230 100644
> --- a/Documentation/networking/switchdev.txt
> +++ b/Documentation/networking/switchdev.txt
> @@ -233,9 +233,8 @@ the bridge's FDB. It's possible, but not optimal, to enable learning on the
> device port and on the bridge port, and disable learning_sync.
>
> To support learning and learning_sync port attributes, the driver implements
> -switchdev op switchdev_port_attr_get/set for
> -SWITCHDEV_ATTR_PORT_ID_BRIDGE_FLAGS. The driver should initialize the attributes
> -to the hardware defaults.
> +switchdev op switchdev_port_attr_set for SWITCHDEV_ATTR_PORT_ID_BRIDGE_FLAGS.
> +The driver should initialize the attributes to the hardware defaults.
Last sentence is not relevant anymore. learning_sync can also be dropped
On Thu, Feb 14, 2019 at 01:20:02PM +0200, Ido Schimmel wrote:
> On Wed, Feb 13, 2019 at 02:06:36PM -0800, Florian Fainelli wrote:
> > Now that all switchdev drivers have been converted to checking the
> > bridge port flags during the prepare phase of the
> > switchdev_port_attr_set() when the process
> > SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS, we can avoid calling
> > switchdev_port_attr_get() with
> > SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT.
> >
> > Signed-off-by: Florian Fainelli <[email protected]>
> > ---
> > net/bridge/br_switchdev.c | 16 +++++++---------
> > 1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> > index db9e8ab96d48..8f88f8a1a7fa 100644
> > --- a/net/bridge/br_switchdev.c
> > +++ b/net/bridge/br_switchdev.c
> > @@ -64,29 +64,27 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
> > {
> > struct switchdev_attr attr = {
> > .orig_dev = p->dev,
> > - .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> > + .id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
> > + .u.brport_flags = flags,
> > };
> > int err;
> >
> > if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
> > return 0;
> >
> > - err = switchdev_port_attr_get(p->dev, &attr);
> > - if (err == -EOPNOTSUPP)
> > - return 0;
> > - if (err)
> > + err = switchdev_port_attr_set(p->dev, &attr);
> > + if (err && err != -EOPNOTSUPP)
> > return err;
> >
> > - /* Check if specific bridge flag attribute offload is supported */
> > - if (!(attr.u.brport_flags_support & mask)) {
> > + if (err == -EOPNOTSUPP) {
> > br_warn(p->br, "bridge flag offload is not supported %u(%s)\n",
> > (unsigned int)p->port_no, p->dev->name);
> > - return -EOPNOTSUPP;
> > + return err;
> > }
>
> I see that you return -EOPNOTSUPP from drivers in case of unsupported
> flags. I believe this is problematic (I'll test soon). The same return
> code is used by:
>
> 1. Switch drivers to indicate unsupported flags
> 2. switchdev code to indicate unsupported netdev (no switchdev ops)
>
> I guess that with this patch any attempt to set bridge port flags on
> veth/dummy device will result in an error.
Yea, that's the case. You can test with
tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh and other
bridge-related tests we have there.
Another problem is that during PORT_PRE_BRIDGE_FLAGS you pass 'flags'
and not 'mask'. This breaks mlxsw (and probably others as well) given
BR_BCAST_FLOOD is set by default.
>
> >
> > 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) {
> > br_warn(p->br, "error setting offload flag on port %u(%s)\n",
> > --
> > 2.17.1
> >
On Wed, Feb 13, 2019 at 02:06:32PM -0800, Florian Fainelli wrote:
> In preparation for getting rid of switchdev_port_attr_get(), have mlxsw
> check for the bridge flags being set through switchdev_port_attr_set()
> when the SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier is
> used.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index 1f492b7dbea8..7616eab50035 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -598,13 +598,17 @@ mlxsw_sp_bridge_port_learning_set(struct mlxsw_sp_port *mlxsw_sp_port,
> static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
> struct switchdev_trans *trans,
> struct net_device *orig_dev,
> - unsigned long brport_flags)
> + unsigned long brport_flags,
> + bool pre_set)
> {
> struct mlxsw_sp_bridge_port *bridge_port;
> int err;
>
> - if (switchdev_trans_ph_prepare(trans))
> + if (switchdev_trans_ph_prepare(trans) && pre_set) {
> + if (brport_flags & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
> + return -EOPNOTSUPP;
> return 0;
> + }
When we get SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS we only want to
perform a check. With this code in case it's not prepare phase, then we
continue to set the flags. Better do:
if (pre_set) {
if (switchdev_trans_ph_commit(trans))
return 0;
// perform check here
}
>
> bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
> orig_dev);
> @@ -833,6 +837,7 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
> struct switchdev_trans *trans)
> {
> struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
> + bool pre_set = false;
> int err;
>
> switch (attr->id) {
> @@ -841,10 +846,13 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
> attr->orig_dev,
> attr->u.stp_state);
> break;
> + case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
> + pre_set = true; /* fall through */
> case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> err = mlxsw_sp_port_attr_br_flags_set(mlxsw_sp_port, trans,
> attr->orig_dev,
> - attr->u.brport_flags);
> + attr->u.brport_flags,
> + pre_set);
> break;
> case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
> err = mlxsw_sp_port_attr_br_ageing_set(mlxsw_sp_port, trans,
> --
> 2.17.1
>