* [PATCH v2 net-next 1/7] net: dsa: make dp->bridge_num one-based
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 2/7] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
I have seen too many bugs already due to the fact that we must encode an
invalid dp->bridge_num as a negative value, because the natural tendency
is to check that invalid value using (!dp->bridge_num). Latest example
can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
getting cleared after ports leaving the bridge").
Convert the existing users to assume that dp->bridge_num == 0 is the
encoding for invalid, and valid bridge numbers start from 1.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v1->v2: none
drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++------
include/linux/dsa/8021q.h | 6 +++---
include/net/dsa.h | 6 +++---
net/dsa/dsa2.c | 24 ++++++++++++------------
net/dsa/dsa_priv.h | 5 +++--
net/dsa/port.c | 11 ++++++-----
net/dsa/tag_8021q.c | 12 +++++++-----
net/dsa/tag_dsa.c | 2 +-
8 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f00cbf5753b9..de3401b2c86c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1250,10 +1250,10 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
/* dev is a virtual bridge */
} else {
list_for_each_entry(dp, &dst->ports, list) {
- if (dp->bridge_num < 0)
+ if (!dp->bridge_num)
continue;
- if (dp->bridge_num + 1 + dst->last_switch != dev)
+ if (dp->bridge_num + dst->last_switch != dev)
continue;
br = dp->bridge_dev;
@@ -2527,9 +2527,9 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
* physical switches, so start from beyond that range.
*/
static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
- int bridge_num)
+ unsigned int bridge_num)
{
- u8 dev = bridge_num + ds->dst->last_switch + 1;
+ u8 dev = bridge_num + ds->dst->last_switch;
struct mv88e6xxx_chip *chip = ds->priv;
int err;
@@ -2542,14 +2542,14 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
}
static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
int err;
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 254b165f2b44..0af4371fbebb 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -38,13 +38,13 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num);
+ unsigned int bridge_num);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num);
+ unsigned int bridge_num);
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
u16 dsa_tag_8021q_tx_vid(const struct dsa_port *dp);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8ca9d50cbbc2..a23cfbaa09d6 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -257,7 +257,7 @@ struct dsa_port {
bool learning;
u8 stp_state;
struct net_device *bridge_dev;
- int bridge_num;
+ unsigned int bridge_num;
struct devlink_port devlink_port;
bool devlink_port_setup;
struct phylink *pl;
@@ -754,11 +754,11 @@ struct dsa_switch_ops {
/* Called right after .port_bridge_join() */
int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
struct net_device *bridge,
- int bridge_num);
+ unsigned int bridge_num);
/* Called right before .port_bridge_leave() */
void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
struct net_device *bridge,
- int bridge_num);
+ unsigned int bridge_num);
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
void (*port_fast_age)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 826957b6442b..9606e56710a5 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -141,23 +141,23 @@ static int dsa_bridge_num_find(const struct net_device *bridge_dev)
*/
list_for_each_entry(dst, &dsa_tree_list, list)
list_for_each_entry(dp, &dst->ports, list)
- if (dp->bridge_dev == bridge_dev &&
- dp->bridge_num != -1)
+ if (dp->bridge_dev == bridge_dev && dp->bridge_num)
return dp->bridge_num;
- return -1;
+ return 0;
}
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
{
- int bridge_num = dsa_bridge_num_find(bridge_dev);
+ unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
- if (bridge_num < 0) {
+ if (!bridge_num) {
/* First port that offloads TX forwarding for this bridge */
- bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
- DSA_MAX_NUM_OFFLOADING_BRIDGES);
+ bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
+ DSA_MAX_NUM_OFFLOADING_BRIDGES,
+ 1);
if (bridge_num >= max)
- return -1;
+ return 0;
set_bit(bridge_num, &dsa_fwd_offloading_bridges);
}
@@ -165,12 +165,13 @@ int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
return bridge_num;
}
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num)
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+ unsigned int bridge_num)
{
/* Check if the bridge is still in use, otherwise it is time
* to clean it up so we can reuse this bridge_num later.
*/
- if (dsa_bridge_num_find(bridge_dev) < 0)
+ if (!dsa_bridge_num_find(bridge_dev))
clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
}
@@ -1184,7 +1185,6 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
dp->ds = ds;
dp->index = index;
- dp->bridge_num = -1;
INIT_LIST_HEAD(&dp->list);
list_add_tail(&dp->list, &dst->ports);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 3fb2c37c9b88..70c4a5b36a8b 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -546,8 +546,9 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
struct net_device *master,
const struct dsa_device_ops *tag_ops,
const struct dsa_device_ops *old_tag_ops);
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num);
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+ unsigned int bridge_num);
/* tag_8021q.c */
int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6d5ebe61280b..9a77bd1373e2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -273,14 +273,14 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
struct net_device *bridge_dev)
{
- int bridge_num = dp->bridge_num;
+ unsigned int bridge_num = dp->bridge_num;
struct dsa_switch *ds = dp->ds;
/* No bridge TX forwarding offload => do nothing */
- if (!ds->ops->port_bridge_tx_fwd_unoffload || dp->bridge_num == -1)
+ if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
return;
- dp->bridge_num = -1;
+ dp->bridge_num = 0;
dsa_bridge_num_put(bridge_dev, bridge_num);
@@ -295,14 +295,15 @@ static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
struct net_device *bridge_dev)
{
struct dsa_switch *ds = dp->ds;
- int bridge_num, err;
+ unsigned int bridge_num;
+ int err;
if (!ds->ops->port_bridge_tx_fwd_offload)
return false;
bridge_num = dsa_bridge_num_get(bridge_dev,
ds->num_fwd_offloading_bridges);
- if (bridge_num < 0)
+ if (!bridge_num)
return false;
dp->bridge_num = bridge_num;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 72cac2c0af7b..df59f16436a5 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -67,10 +67,12 @@
#define DSA_8021Q_PORT(x) (((x) << DSA_8021Q_PORT_SHIFT) & \
DSA_8021Q_PORT_MASK)
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num)
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
{
- /* The VBID value of 0 is reserved for precise TX */
- return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1);
+ /* The VBID value of 0 is reserved for precise TX, but it is also
+ * reserved/invalid for the bridge_num, so all is well.
+ */
+ return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num);
}
EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
@@ -409,7 +411,7 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
@@ -420,7 +422,7 @@ EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index b3da4b2ea11c..a7d70ae7cc97 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -140,7 +140,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
* packets on behalf of a virtual switch device with an index
* past the physical switches.
*/
- tag_dev = dst->last_switch + 1 + dp->bridge_num;
+ tag_dev = dst->last_switch + dp->bridge_num;
tag_port = 0;
} else {
cmd = DSA_CMD_FROM_CPU;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 net-next 2/7] net: dsa: assign a bridge number even without TX forwarding offload
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 1/7] net: dsa: make dp->bridge_num one-based Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 3/7] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
The service where DSA assigns a unique bridge number for each forwarding
domain is useful even for drivers which do not implement the TX
forwarding offload feature.
For example, drivers might use the dp->bridge_num for FDB isolation.
So rename ds->num_fwd_offloading_bridges to ds->max_num_bridges, and
calculate a unique bridge_num for all drivers that set this value.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v1->v2: update the comment in dsa_bridge_num_get() about bridge_num's
new role as per Alvin's suggestion
drivers/net/dsa/mv88e6xxx/chip.c | 4 +-
drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
include/net/dsa.h | 10 ++--
net/dsa/dsa2.c | 4 +-
net/dsa/port.c | 81 +++++++++++++++++---------
5 files changed, 66 insertions(+), 35 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index de3401b2c86c..a818df35b239 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3186,8 +3186,8 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
* time.
*/
if (mv88e6xxx_has_pvt(chip))
- ds->num_fwd_offloading_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
- ds->dst->last_switch - 1;
+ ds->max_num_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
+ ds->dst->last_switch - 1;
mv88e6xxx_reg_lock(chip);
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index c343effe2e96..355b56cf94d8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3139,7 +3139,7 @@ static int sja1105_setup(struct dsa_switch *ds)
ds->vlan_filtering_is_global = true;
ds->untag_bridge_pvid = true;
/* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
- ds->num_fwd_offloading_bridges = 7;
+ ds->max_num_bridges = 7;
/* Advertise the 8 egress queues */
ds->num_tx_queues = SJA1105_NUM_TC;
diff --git a/include/net/dsa.h b/include/net/dsa.h
index a23cfbaa09d6..00fbd87ae4ff 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -413,12 +413,12 @@ struct dsa_switch {
*/
unsigned int num_lag_ids;
- /* Drivers that support bridge forwarding offload should set this to
- * the maximum number of bridges spanning the same switch tree (or all
- * trees, in the case of cross-tree bridging support) that can be
- * offloaded.
+ /* Drivers that support bridge forwarding offload or FDB isolation
+ * should set this to the maximum number of bridges spanning the same
+ * switch tree (or all trees, in the case of cross-tree bridging
+ * support) that can be offloaded.
*/
- unsigned int num_fwd_offloading_bridges;
+ unsigned int max_num_bridges;
size_t num_ports;
};
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 9606e56710a5..4901cdc264ee 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -152,7 +152,9 @@ unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
if (!bridge_num) {
- /* First port that offloads TX forwarding for this bridge */
+ /* First port that requests FDB isolation or TX forwarding
+ * offload for this bridge
+ */
bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
DSA_MAX_NUM_OFFLOADING_BRIDGES,
1);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 9a77bd1373e2..199a56faf460 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -271,19 +271,15 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
}
static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
- struct net_device *bridge_dev)
+ struct net_device *bridge_dev,
+ unsigned int bridge_num)
{
- unsigned int bridge_num = dp->bridge_num;
struct dsa_switch *ds = dp->ds;
/* No bridge TX forwarding offload => do nothing */
- if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
+ if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
return;
- dp->bridge_num = 0;
-
- dsa_bridge_num_put(bridge_dev, bridge_num);
-
/* Notify the chips only once the offload has been deactivated, so
* that they can update their configuration accordingly.
*/
@@ -292,31 +288,60 @@ static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
}
static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
- struct net_device *bridge_dev)
+ struct net_device *bridge_dev,
+ unsigned int bridge_num)
{
struct dsa_switch *ds = dp->ds;
- unsigned int bridge_num;
int err;
- if (!ds->ops->port_bridge_tx_fwd_offload)
- return false;
-
- bridge_num = dsa_bridge_num_get(bridge_dev,
- ds->num_fwd_offloading_bridges);
- if (!bridge_num)
+ /* FDB isolation is required for TX forwarding offload */
+ if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
return false;
- dp->bridge_num = bridge_num;
-
/* Notify the driver */
err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
bridge_num);
- if (err) {
- dsa_port_bridge_tx_fwd_unoffload(dp, bridge_dev);
- return false;
+
+ return err ? false : true;
+}
+
+static int dsa_port_bridge_create(struct dsa_port *dp,
+ struct net_device *br,
+ struct netlink_ext_ack *extack)
+{
+ struct dsa_switch *ds = dp->ds;
+ unsigned int bridge_num;
+
+ dp->bridge_dev = br;
+
+ if (!ds->max_num_bridges)
+ return 0;
+
+ bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
+ if (!bridge_num) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Range of offloadable bridges exceeded");
+ return -EOPNOTSUPP;
}
- return true;
+ dp->bridge_num = bridge_num;
+
+ return 0;
+}
+
+static void dsa_port_bridge_destroy(struct dsa_port *dp,
+ const struct net_device *br)
+{
+ struct dsa_switch *ds = dp->ds;
+
+ dp->bridge_dev = NULL;
+
+ if (ds->max_num_bridges) {
+ int bridge_num = dp->bridge_num;
+
+ dp->bridge_num = 0;
+ dsa_bridge_num_put(br, bridge_num);
+ }
}
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
@@ -336,7 +361,9 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
/* Here the interface is already bridged. Reflect the current
* configuration so that drivers can program their chips accordingly.
*/
- dp->bridge_dev = br;
+ err = dsa_port_bridge_create(dp, br, extack);
+ if (err)
+ return err;
brport_dev = dsa_port_to_bridge_port(dp);
@@ -344,7 +371,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
if (err)
goto out_rollback;
- tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br);
+ tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
+ dp->bridge_num);
err = switchdev_bridge_port_offload(brport_dev, dev, dp,
&dsa_slave_switchdev_notifier,
@@ -366,7 +394,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
out_rollback_unbridge:
dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
out_rollback:
- dp->bridge_dev = NULL;
+ dsa_port_bridge_destroy(dp, br);
return err;
}
@@ -393,14 +421,15 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
.port = dp->index,
.br = br,
};
+ int bridge_num = dp->bridge_num;
int err;
/* Here the port is already unbridged. Reflect the current configuration
* so that drivers can program their chips accordingly.
*/
- dp->bridge_dev = NULL;
+ dsa_port_bridge_destroy(dp, br);
- dsa_port_bridge_tx_fwd_unoffload(dp, br);
+ dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
if (err)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 net-next 3/7] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 1/7] net: dsa: make dp->bridge_num one-based Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 2/7] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-04 23:27 ` Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 4/7] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
The location of the bridge device pointer and number is going to change.
It is not going to be kept individually per port, but in a common
structure allocated dynamically and which will have lockdep validation.
Create helpers to access these elements so that we have a migration path
to the new organization.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2:
- convert ksz_common.c too, after Oleksij's commit b3612ccdf284
("net: dsa: microchip: implement multi-bridge support").
- make dsa_port_bridge_dev_get take const struct dsa_port * arguments.
- make dsa_port_bridge_same return false for two standalone ports.
drivers/net/dsa/b53/b53_common.c | 4 +-
drivers/net/dsa/lan9303-core.c | 2 +-
drivers/net/dsa/lantiq_gswip.c | 10 ++---
drivers/net/dsa/microchip/ksz_common.c | 2 +-
drivers/net/dsa/mt7530.c | 14 ++++---
drivers/net/dsa/mv88e6xxx/chip.c | 56 ++++++++++++++------------
drivers/net/dsa/qca8k.c | 4 +-
drivers/net/dsa/rtl8366rb.c | 4 +-
drivers/net/dsa/sja1105/sja1105_main.c | 10 +++--
drivers/net/dsa/xrs700x/xrs700x.c | 4 +-
include/net/dsa.h | 21 ++++++++++
net/dsa/dsa_priv.h | 4 +-
net/dsa/port.c | 35 ++++++++--------
net/dsa/slave.c | 14 ++++---
net/dsa/switch.c | 6 +--
net/dsa/tag_8021q.c | 2 +-
net/dsa/tag_dsa.c | 5 ++-
net/dsa/tag_ocelot.c | 2 +-
net/dsa/tag_sja1105.c | 11 +++--
19 files changed, 123 insertions(+), 87 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index af4761968733..d5e78f51f42d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1887,7 +1887,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
b53_for_each_port(dev, i) {
- if (dsa_to_port(ds, i)->bridge_dev != br)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
continue;
/* Add this local port to the remote port VLAN control
@@ -1923,7 +1923,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
b53_for_each_port(dev, i) {
/* Don't touch the remaining ports */
- if (dsa_to_port(ds, i)->bridge_dev != br)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
continue;
b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), ®);
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 89f920289ae2..1c2bdcde6979 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1108,7 +1108,7 @@ static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
struct lan9303 *chip = ds->priv;
dev_dbg(chip->dev, "%s(port %d)\n", __func__, port);
- if (dsa_to_port(ds, 1)->bridge_dev == dsa_to_port(ds, 2)->bridge_dev) {
+ if (dsa_port_bridge_same(dsa_to_port(ds, 1), dsa_to_port(ds, 2))) {
lan9303_bridge_ports(chip);
chip->is_bridged = true; /* unleash stp_state_set() */
}
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 583af774e1bd..6317d0ae42d0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -759,7 +759,7 @@ static int gswip_port_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct netlink_ext_ack *extack)
{
- struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
+ struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
/* Do not allow changing the VLAN filtering options while in bridge */
@@ -1183,8 +1183,8 @@ static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan,
struct netlink_ext_ack *extack)
{
+ struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
- struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
unsigned int max_ports = priv->hw_info->max_ports;
int pos = max_ports;
int i, idx = -1;
@@ -1229,8 +1229,8 @@ static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan,
struct netlink_ext_ack *extack)
{
+ struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
- struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
int err;
@@ -1254,8 +1254,8 @@ static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
static int gswip_port_vlan_del(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan)
{
+ struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
- struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
/* We have to receive all packets on the CPU port and should not
@@ -1340,8 +1340,8 @@ static void gswip_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
static int gswip_port_fdb(struct dsa_switch *ds, int port,
const unsigned char *addr, u16 vid, bool add)
{
+ struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
- struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
struct gswip_pce_table_entry mac_bridge = {0,};
unsigned int cpu_port = priv->hw_info->cpu_port;
int fid = -1;
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8a04302018dc..cebcb73cda76 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -43,7 +43,7 @@ void ksz_update_port_member(struct ksz_device *dev, int port)
continue;
if (port == i)
continue;
- if (!dp->bridge_dev || dp->bridge_dev != other_dp->bridge_dev)
+ if (!dsa_port_bridge_same(dp, other_dp))
continue;
if (other_p->stp_state == BR_STATE_FORWARDING &&
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 9890672a206d..79bde263b06f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1195,12 +1195,14 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
mutex_lock(&priv->reg_mutex);
for (i = 0; i < MT7530_NUM_PORTS; i++) {
+ struct dsa_port *other_dp = dsa_to_port(ds, port);
+
/* Add this port to the port matrix of the other ports in the
* same bridge. If the port is disabled, port matrix is kept
* and not being setup until the port becomes enabled.
*/
- if (dsa_is_user_port(ds, i) && i != port) {
- if (dsa_to_port(ds, i)->bridge_dev != bridge)
+ if (dsa_port_is_user(other_dp) && i != port) {
+ if (dsa_port_bridge_dev_get(other_dp) != bridge)
continue;
if (priv->ports[i].enable)
mt7530_set(priv, MT7530_PCR_P(i),
@@ -1236,7 +1238,7 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
/* This is called after .port_bridge_leave when leaving a VLAN-aware
* bridge. Don't set standalone ports to fallback mode.
*/
- if (dsa_to_port(ds, port)->bridge_dev)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, port)))
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
MT7530_PORT_FALLBACK_MODE);
@@ -1307,12 +1309,14 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
mutex_lock(&priv->reg_mutex);
for (i = 0; i < MT7530_NUM_PORTS; i++) {
+ struct dsa_port *other_dp = dsa_to_port(ds, i);
+
/* Remove this port from the port matrix of the other ports
* in the same bridge. If the port is disabled, port matrix
* is kept and not being setup until the port becomes enabled.
*/
- if (dsa_is_user_port(ds, i) && i != port) {
- if (dsa_to_port(ds, i)->bridge_dev != bridge)
+ if (dsa_port_is_user(other_dp) && i != port) {
+ if (dsa_port_bridge_dev_get(other_dp) != bridge)
continue;
if (priv->ports[i].enable)
mt7530_clear(priv, MT7530_PCR_P(i),
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index a818df35b239..c583ece83b24 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1228,8 +1228,7 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
{
struct dsa_switch *ds = chip->ds;
struct dsa_switch_tree *dst = ds->dst;
- struct net_device *br;
- struct dsa_port *dp;
+ struct dsa_port *dp, *other_dp;
bool found = false;
u16 pvlan;
@@ -1238,11 +1237,9 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
list_for_each_entry(dp, &dst->ports, list) {
if (dp->ds->index == dev && dp->index == port) {
/* dp might be a DSA link or a user port, so it
- * might or might not have a bridge_dev
- * pointer. Use the "found" variable for both
- * cases.
+ * might or might not have a bridge.
+ * Use the "found" variable for both cases.
*/
- br = dp->bridge_dev;
found = true;
break;
}
@@ -1250,13 +1247,14 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
/* dev is a virtual bridge */
} else {
list_for_each_entry(dp, &dst->ports, list) {
- if (!dp->bridge_num)
+ unsigned int bridge_num = dsa_port_bridge_num_get(dp);
+
+ if (!bridge_num)
continue;
- if (dp->bridge_num + dst->last_switch != dev)
+ if (bridge_num + dst->last_switch != dev)
continue;
- br = dp->bridge_dev;
found = true;
break;
}
@@ -1275,12 +1273,12 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
/* Frames from user ports can egress any local DSA links and CPU ports,
* as well as any local member of their bridge group.
*/
- list_for_each_entry(dp, &dst->ports, list)
- if (dp->ds == ds &&
- (dp->type == DSA_PORT_TYPE_CPU ||
- dp->type == DSA_PORT_TYPE_DSA ||
- (br && dp->bridge_dev == br)))
- pvlan |= BIT(dp->index);
+ list_for_each_entry(other_dp, &dst->ports, list)
+ if (other_dp->ds == ds &&
+ (other_dp->type == DSA_PORT_TYPE_CPU ||
+ other_dp->type == DSA_PORT_TYPE_DSA ||
+ dsa_port_bridge_same(dp, other_dp)))
+ pvlan |= BIT(other_dp->index);
return pvlan;
}
@@ -1647,8 +1645,10 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
u16 vid)
{
+ struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
struct mv88e6xxx_chip *chip = ds->priv;
struct mv88e6xxx_vtu_entry vlan;
+ struct net_device *other_br;
int i, err;
/* DSA and CPU ports have to be members of multiple vlans */
@@ -1662,27 +1662,30 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
if (!vlan.valid)
return 0;
- for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
- if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
+ list_for_each_entry(other_dp, &ds->dst->ports, list) {
+ if (other_dp->ds != ds)
continue;
- if (!dsa_to_port(ds, i)->slave)
+ if (dsa_port_is_dsa(other_dp) || dsa_port_is_cpu(other_dp))
+ continue;
+
+ if (!other_dp->slave)
continue;
if (vlan.member[i] ==
MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
continue;
- if (dsa_to_port(ds, i)->bridge_dev ==
- dsa_to_port(ds, port)->bridge_dev)
+ other_br = dsa_port_bridge_dev_get(other_dp);
+
+ if (dsa_port_bridge_same(dp, other_dp))
break; /* same bridge, check next VLAN */
- if (!dsa_to_port(ds, i)->bridge_dev)
+ if (!other_br)
continue;
dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n",
- port, vlan.vid, i,
- netdev_name(dsa_to_port(ds, i)->bridge_dev));
+ port, vlan.vid, i, netdev_name(other_br));
return -EOPNOTSUPP;
}
@@ -1692,13 +1695,14 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
static int mv88e6xxx_port_commit_pvid(struct mv88e6xxx_chip *chip, int port)
{
struct dsa_port *dp = dsa_to_port(chip->ds, port);
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct mv88e6xxx_port *p = &chip->ports[port];
u16 pvid = MV88E6XXX_VID_STANDALONE;
bool drop_untagged = false;
int err;
- if (dp->bridge_dev) {
- if (br_vlan_enabled(dp->bridge_dev)) {
+ if (br) {
+ if (br_vlan_enabled(br)) {
pvid = p->bridge_pvid.vid;
drop_untagged = !p->bridge_pvid.valid;
} else {
@@ -2424,7 +2428,7 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
int err;
list_for_each_entry(dp, &dst->ports, list) {
- if (dp->bridge_dev == br) {
+ if (dsa_port_bridge_dev_get(dp) == br) {
if (dp->ds == ds) {
/* This is a local bridge group member,
* remap its Port VLAN Map.
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 96a7fbf8700c..7053a3510d71 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1823,7 +1823,7 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
for (i = 0; i < QCA8K_NUM_PORTS; i++) {
if (dsa_is_cpu_port(ds, i))
continue;
- if (dsa_to_port(ds, i)->bridge_dev != br)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
continue;
/* Add this port to the portvlan mask of the other ports
* in the bridge
@@ -1855,7 +1855,7 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
for (i = 0; i < QCA8K_NUM_PORTS; i++) {
if (dsa_is_cpu_port(ds, i))
continue;
- if (dsa_to_port(ds, i)->bridge_dev != br)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
continue;
/* Remove this port to the portvlan mask of the other ports
* in the bridge
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index 03deacd83e61..b6f277a04989 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1198,7 +1198,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
if (i == port)
continue;
/* Not on this bridge */
- if (dsa_to_port(ds, i)->bridge_dev != bridge)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
continue;
/* Join this port to each other port on the bridge */
ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
@@ -1230,7 +1230,7 @@ rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
if (i == port)
continue;
/* Not on this bridge */
- if (dsa_to_port(ds, i)->bridge_dev != bridge)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
continue;
/* Remove this port from any other port on the bridge */
ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 355b56cf94d8..5e03eda4c16f 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -118,13 +118,14 @@ static int sja1105_pvid_apply(struct sja1105_private *priv, int port, u16 pvid)
static int sja1105_commit_pvid(struct dsa_switch *ds, int port)
{
struct dsa_port *dp = dsa_to_port(ds, port);
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct sja1105_private *priv = ds->priv;
struct sja1105_vlan_lookup_entry *vlan;
bool drop_untagged = false;
int match, rc;
u16 pvid;
- if (dp->bridge_dev && br_vlan_enabled(dp->bridge_dev))
+ if (br && br_vlan_enabled(br))
pvid = priv->bridge_pvid[port];
else
pvid = priv->tag_8021q_pvid[port];
@@ -2004,7 +2005,7 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
*/
if (i == port)
continue;
- if (dsa_to_port(ds, i)->bridge_dev != br)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
continue;
sja1105_port_allow_traffic(l2_fwd, i, port, member);
sja1105_port_allow_traffic(l2_fwd, port, i, member);
@@ -2587,8 +2588,9 @@ static int sja1105_prechangeupper(struct dsa_switch *ds, int port,
if (netif_is_bridge_master(upper)) {
list_for_each_entry(dp, &dst->ports, list) {
- if (dp->bridge_dev && dp->bridge_dev != upper &&
- br_vlan_enabled(dp->bridge_dev)) {
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
+
+ if (br && br != upper && br_vlan_enabled(br)) {
NL_SET_ERR_MSG_MOD(extack,
"Only one VLAN-aware bridge is supported");
return -EBUSY;
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 910fcb3b252b..7c2b6c32242d 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -513,14 +513,14 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
cpu_mask |= BIT(i);
- if (dsa_to_port(ds, i)->bridge_dev == bridge)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) == bridge)
continue;
mask |= BIT(i);
}
for (i = 0; i < ds->num_ports; i++) {
- if (dsa_to_port(ds, i)->bridge_dev != bridge)
+ if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
continue;
/* 1 = Disable forwarding to the port */
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 00fbd87ae4ff..18bce0383267 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -599,6 +599,27 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
return dp->slave;
}
+static inline struct net_device *
+dsa_port_bridge_dev_get(const struct dsa_port *dp)
+{
+ return dp->bridge_dev;
+}
+
+static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp)
+{
+ return dp->bridge_num;
+}
+
+static inline bool dsa_port_bridge_same(const struct dsa_port *a,
+ const struct dsa_port *b)
+{
+ struct net_device *br_a = dsa_port_bridge_dev_get(a);
+ struct net_device *br_b = dsa_port_bridge_dev_get(b);
+
+ /* Standalone ports are not in the same bridge with one another */
+ return (!br_a || !br_b) ? false : (br_a == br_b);
+}
+
typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
bool is_static, void *data);
struct dsa_switch_ops {
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 70c4a5b36a8b..46a0adfb03cd 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -278,7 +278,7 @@ static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
/* DSA ports connected to a bridge, and event was emitted
* for the bridge.
*/
- return dp->bridge_dev == bridge_dev;
+ return dsa_port_bridge_dev_get(dp) == bridge_dev;
}
/* Returns true if any port of this tree offloads the given net_device */
@@ -345,7 +345,7 @@ dsa_slave_to_master(const struct net_device *dev)
static inline struct sk_buff *dsa_untag_bridge_pvid(struct sk_buff *skb)
{
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct net_device *br = dp->bridge_dev;
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct net_device *dev = skb->dev;
struct net_device *upper_dev;
u16 vid, pvid, proto;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 199a56faf460..f6ea41cbcdd5 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -221,7 +221,7 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
struct netlink_ext_ack *extack)
{
struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
- struct net_device *br = dp->bridge_dev;
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
int err;
err = dsa_port_inherit_brport_flags(dp, extack);
@@ -372,7 +372,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
goto out_rollback;
tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
- dp->bridge_num);
+ dsa_port_bridge_num_get(dp));
err = switchdev_bridge_port_offload(brport_dev, dev, dp,
&dsa_slave_switchdev_notifier,
@@ -415,13 +415,13 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br)
void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
{
+ unsigned int bridge_num = dsa_port_bridge_num_get(dp);
struct dsa_notifier_bridge_info info = {
.tree_index = dp->ds->dst->index,
.sw_index = dp->ds->index,
.port = dp->index,
.br = br,
};
- int bridge_num = dp->bridge_num;
int err;
/* Here the port is already unbridged. Reflect the current configuration
@@ -507,12 +507,15 @@ int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag,
void dsa_port_pre_lag_leave(struct dsa_port *dp, struct net_device *lag)
{
- if (dp->bridge_dev)
- dsa_port_pre_bridge_leave(dp, dp->bridge_dev);
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
+
+ if (br)
+ dsa_port_pre_bridge_leave(dp, br);
}
void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag)
{
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct dsa_notifier_lag_info info = {
.sw_index = dp->ds->index,
.port = dp->index,
@@ -526,8 +529,8 @@ void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag)
/* Port might have been part of a LAG that in turn was
* attached to a bridge.
*/
- if (dp->bridge_dev)
- dsa_port_bridge_leave(dp, dp->bridge_dev);
+ if (br)
+ dsa_port_bridge_leave(dp, br);
dp->lag_tx_enabled = false;
dp->lag_dev = NULL;
@@ -556,8 +559,8 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
* as long as we have 8021q uppers.
*/
if (vlan_filtering && dsa_port_is_user(dp)) {
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct net_device *upper_dev, *slave = dp->slave;
- struct net_device *br = dp->bridge_dev;
struct list_head *iter;
netdev_for_each_upper_dev_rcu(slave, upper_dev, iter) {
@@ -591,17 +594,15 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
* different setting than what is being requested.
*/
dsa_switch_for_each_port(other_dp, ds) {
- struct net_device *other_bridge;
+ struct net_device *other_br = dsa_port_bridge_dev_get(other_dp);
- other_bridge = other_dp->bridge_dev;
- if (!other_bridge)
- continue;
/* If it's the same bridge, it also has same
* vlan_filtering setting => no need to check
*/
- if (other_bridge == dp->bridge_dev)
+ if (!other_br || other_br == dsa_port_bridge_dev_get(dp))
continue;
- if (br_vlan_enabled(other_bridge) != vlan_filtering) {
+
+ if (br_vlan_enabled(other_br) != vlan_filtering) {
NL_SET_ERR_MSG_MOD(extack,
"VLAN filtering is a global setting");
return false;
@@ -685,13 +686,13 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
*/
bool dsa_port_skip_vlan_configuration(struct dsa_port *dp)
{
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct dsa_switch *ds = dp->ds;
- if (!dp->bridge_dev)
+ if (!br)
return false;
- return (!ds->configure_vlan_while_not_filtering &&
- !br_vlan_enabled(dp->bridge_dev));
+ return !ds->configure_vlan_while_not_filtering && !br_vlan_enabled(br);
}
int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 33b54eadc641..283029ba70b3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -363,7 +363,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
/* Deny adding a bridge VLAN when there is already an 802.1Q upper with
* the same VID.
*/
- if (br_vlan_enabled(dp->bridge_dev)) {
+ if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
rcu_read_lock();
err = dsa_slave_vlan_check_for_8021q_uppers(dev, &vlan);
rcu_read_unlock();
@@ -1580,7 +1580,8 @@ static void dsa_bridge_mtu_normalization(struct dsa_port *dp)
if (other_dp->type != DSA_PORT_TYPE_USER)
continue;
- if (other_dp->bridge_dev != dp->bridge_dev)
+ if (dsa_port_bridge_dev_get(other_dp) !=
+ dsa_port_bridge_dev_get(dp))
continue;
if (!other_dp->ds->mtu_enforcement_ingress)
@@ -2220,7 +2221,7 @@ dsa_prevent_bridging_8021q_upper(struct net_device *dev,
struct netdev_notifier_changeupper_info *info)
{
struct netlink_ext_ack *ext_ack;
- struct net_device *slave;
+ struct net_device *slave, *br;
struct dsa_port *dp;
ext_ack = netdev_notifier_info_to_extack(&info->info);
@@ -2233,11 +2234,12 @@ dsa_prevent_bridging_8021q_upper(struct net_device *dev,
return NOTIFY_DONE;
dp = dsa_slave_to_port(slave);
- if (!dp->bridge_dev)
+ br = dsa_port_bridge_dev_get(dp);
+ if (!br)
return NOTIFY_DONE;
/* Deny enslaving a VLAN device into a VLAN-aware bridge */
- if (br_vlan_enabled(dp->bridge_dev) &&
+ if (br_vlan_enabled(br) &&
netif_is_bridge_master(info->upper_dev) && info->linking) {
NL_SET_ERR_MSG_MOD(ext_ack,
"Cannot enslave VLAN device into VLAN aware bridge");
@@ -2252,7 +2254,7 @@ dsa_slave_check_8021q_upper(struct net_device *dev,
struct netdev_notifier_changeupper_info *info)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
- struct net_device *br = dp->bridge_dev;
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct bridge_vlan_info br_info;
struct netlink_ext_ack *extack;
int err = NOTIFY_DONE;
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index bb155a16d454..7993192fe769 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -151,11 +151,9 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
*/
if (change_vlan_filtering && ds->vlan_filtering_is_global) {
dsa_switch_for_each_port(dp, ds) {
- struct net_device *bridge_dev;
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
- bridge_dev = dp->bridge_dev;
-
- if (bridge_dev && br_vlan_enabled(bridge_dev)) {
+ if (br && br_vlan_enabled(br)) {
change_vlan_filtering = false;
break;
}
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index df59f16436a5..e9d5e566973c 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -337,7 +337,7 @@ dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
return false;
if (dsa_port_is_user(dp))
- return dp->bridge_dev == info->br;
+ return dsa_port_bridge_dev_get(dp) == info->br;
return false;
}
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index a7d70ae7cc97..8abf39dcac64 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -132,6 +132,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
u8 *dsa_header;
if (skb->offload_fwd_mark) {
+ unsigned int bridge_num = dsa_port_bridge_num_get(dp);
struct dsa_switch_tree *dst = dp->ds->dst;
cmd = DSA_CMD_FORWARD;
@@ -140,7 +141,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
* packets on behalf of a virtual switch device with an index
* past the physical switches.
*/
- tag_dev = dst->last_switch + dp->bridge_num;
+ tag_dev = dst->last_switch + bridge_num;
tag_port = 0;
} else {
cmd = DSA_CMD_FROM_CPU;
@@ -165,7 +166,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
dsa_header[2] &= ~0x10;
}
} else {
- struct net_device *br = dp->bridge_dev;
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
u16 vid;
vid = br ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index de1c849a0a70..4ba460c5a880 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -12,7 +12,7 @@
static void ocelot_xmit_get_vlan_info(struct sk_buff *skb, struct dsa_port *dp,
u64 *vlan_tci, u64 *tag_type)
{
- struct net_device *br = READ_ONCE(dp->bridge_dev);
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
struct vlan_ethhdr *hdr;
u16 proto, tci;
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 262c8833a910..6c293c2a3008 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -159,14 +159,16 @@ static u16 sja1105_xmit_tpid(struct dsa_port *dp)
* need to find it.
*/
dsa_switch_for_each_port(other_dp, ds) {
- if (!other_dp->bridge_dev)
+ struct net_device *br = dsa_port_bridge_dev_get(other_dp);
+
+ if (!br)
continue;
/* Error is returned only if CONFIG_BRIDGE_VLAN_FILTERING,
* which seems pointless to handle, as our port cannot become
* VLAN-aware in that case.
*/
- br_vlan_get_proto(other_dp->bridge_dev, &proto);
+ br_vlan_get_proto(br, &proto);
return proto;
}
@@ -180,7 +182,8 @@ static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb,
struct net_device *netdev)
{
struct dsa_port *dp = dsa_slave_to_port(netdev);
- struct net_device *br = dp->bridge_dev;
+ unsigned int bridge_num = dsa_port_bridge_num_get(dp);
+ struct net_device *br = dsa_port_bridge_dev_get(dp);
u16 tx_vid;
/* If the port is under a VLAN-aware bridge, just slide the
@@ -196,7 +199,7 @@ static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb,
* TX VLAN that targets the bridge's entire broadcast domain,
* instead of just the specific port.
*/
- tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(dp->bridge_num);
+ tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
return dsa_8021q_xmit(skb, netdev, sja1105_xmit_tpid(dp), tx_vid);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 3/7] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers
2021-12-04 20:11 ` [PATCH v2 net-next 3/7] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
@ 2021-12-04 23:27 ` Vladimir Oltean
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 23:27 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
On Sat, Dec 04, 2021 at 10:11:42PM +0200, Vladimir Oltean wrote:
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index a818df35b239..c583ece83b24 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1647,8 +1645,10 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
> static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
> u16 vid)
> {
> + struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> struct mv88e6xxx_chip *chip = ds->priv;
> struct mv88e6xxx_vtu_entry vlan;
> + struct net_device *other_br;
> int i, err;
>
> /* DSA and CPU ports have to be members of multiple vlans */
> @@ -1662,27 +1662,30 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
> if (!vlan.valid)
> return 0;
>
> - for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
> - if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
> + list_for_each_entry(other_dp, &ds->dst->ports, list) {
> + if (other_dp->ds != ds)
> continue;
>
> - if (!dsa_to_port(ds, i)->slave)
> + if (dsa_port_is_dsa(other_dp) || dsa_port_is_cpu(other_dp))
> + continue;
> +
> + if (!other_dp->slave)
> continue;
>
> if (vlan.member[i] ==
Some badly rebased changes from some other patches snuck in and made a
mess out of this. I removed the port iteration using "int i" but did not
remove that variable, and it is also still used.. uninitialized. Will
split the mv88e6xxx conversion to iterate using dp to a separate patch.
> MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
> continue;
>
> - if (dsa_to_port(ds, i)->bridge_dev ==
> - dsa_to_port(ds, port)->bridge_dev)
> + other_br = dsa_port_bridge_dev_get(other_dp);
> +
> + if (dsa_port_bridge_same(dp, other_dp))
> break; /* same bridge, check next VLAN */
>
> - if (!dsa_to_port(ds, i)->bridge_dev)
> + if (!other_br)
> continue;
>
> dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n",
> - port, vlan.vid, i,
> - netdev_name(dsa_to_port(ds, i)->bridge_dev));
> + port, vlan.vid, i, netdev_name(other_br));
> return -EOPNOTSUPP;
> }
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 4/7] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
` (2 preceding siblings ...)
2021-12-04 20:11 ` [PATCH v2 net-next 3/7] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 5/7] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
Currently the majority of dsa_port_bridge_dev_get() calls in drivers is
just to check whether a port is under the bridge device provided as
argument by the DSA API.
We'd like to change that DSA API so that a more complex structure is
provided as argument. To keep things more generic, and considering that
the new complex structure will be provided by value and not by
reference, direct comparisons between dp->bridge and the provided bridge
will be broken. The generic way to do the checking would simply be to
do something like dsa_port_offloads_bridge(dp, &bridge).
But there's a problem, we already have a function named that way, which
actually takes a bridge_dev net_device as argument. Rename it so that we
can use dsa_port_offloads_bridge for something else.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v1->v2: none
net/dsa/dsa_priv.h | 12 +++++++-----
net/dsa/slave.c | 18 +++++++++---------
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 46a0adfb03cd..33fef1be62a3 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -272,8 +272,9 @@ static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
return dsa_port_to_bridge_port(dp) == dev;
}
-static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
- const struct net_device *bridge_dev)
+static inline bool
+dsa_port_offloads_bridge_dev(struct dsa_port *dp,
+ const struct net_device *bridge_dev)
{
/* DSA ports connected to a bridge, and event was emitted
* for the bridge.
@@ -295,13 +296,14 @@ static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
}
/* Returns true if any port of this tree offloads the given bridge */
-static inline bool dsa_tree_offloads_bridge(struct dsa_switch_tree *dst,
- const struct net_device *bridge_dev)
+static inline bool
+dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
+ const struct net_device *bridge_dev)
{
struct dsa_port *dp;
list_for_each_entry(dp, &dst->ports, list)
- if (dsa_port_offloads_bridge(dp, bridge_dev))
+ if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
return true;
return false;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 283029ba70b3..7c7aea19db08 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -289,14 +289,14 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
ret = dsa_port_set_state(dp, attr->u.stp_state, true);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
- if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
return -EOPNOTSUPP;
ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
extack);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
- if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
return -EOPNOTSUPP;
ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
@@ -409,7 +409,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_HOST_MDB:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
@@ -421,13 +421,13 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
err = dsa_slave_vlan_add(dev, obj, extack);
break;
case SWITCHDEV_OBJ_ID_MRP:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_mrp_add(dp, SWITCHDEV_OBJ_MRP(obj));
break;
case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_mrp_add_ring_role(dp,
@@ -483,7 +483,7 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_HOST_MDB:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
@@ -495,13 +495,13 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
err = dsa_slave_vlan_del(dev, obj);
break;
case SWITCHDEV_OBJ_ID_MRP:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
break;
case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
- if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+ if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
return -EOPNOTSUPP;
err = dsa_port_mrp_del_ring_role(dp,
@@ -2451,7 +2451,7 @@ static bool dsa_foreign_dev_check(const struct net_device *dev,
struct dsa_switch_tree *dst = dp->ds->dst;
if (netif_is_bridge_master(foreign_dev))
- return !dsa_tree_offloads_bridge(dst, foreign_dev);
+ return !dsa_tree_offloads_bridge_dev(dst, foreign_dev);
if (netif_is_bridge_port(foreign_dev))
return !dsa_tree_offloads_bridge_port(dst, foreign_dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 net-next 5/7] net: dsa: keep the bridge_dev and bridge_num as part of the same structure
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
` (3 preceding siblings ...)
2021-12-04 20:11 ` [PATCH v2 net-next 4/7] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-04 23:18 ` Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 6/7] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join Vladimir Oltean
2021-12-04 20:11 ` [PATCH v2 net-next 7/7] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
6 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
The main desire behind this is to provide coherent bridge information to
the fast path without locking.
For example, right now we set dp->bridge_dev and dp->bridge_num from
separate code paths, it is theoretically possible for a packet
transmission to read these two port properties consecutively and find a
bridge number which does not correspond with the bridge device.
Another desire is to start passing more complex bridge information to
dsa_switch_ops functions. For example, with FDB isolation, it is
expected that drivers will need to be passed the bridge which requested
an FDB/MDB entry to be offloaded, and along with that bridge_dev, the
associated bridge_num should be passed too, in case the driver might
want to implement an isolation scheme based on that number.
We already pass the {bridge_dev, bridge_num} pair to the TX forwarding
offload switch API, however we'd like to remove that and squash it into
the basic bridge join/leave API. So that means we need to pass this
pair to the bridge join/leave API.
During dsa_port_bridge_leave, first we unset dp->bridge_dev, then we
call the driver's .port_bridge_leave with what used to be our
dp->bridge_dev, but provided as an argument.
When bridge_dev and bridge_num get folded into a single structure, we
need to preserve this behavior in dsa_port_bridge_leave: we need a copy
of what used to be in dp->bridge.
Switch drivers check bridge membership by comparing dp->bridge_dev with
the provided bridge_dev, but now, if we provide the struct dsa_bridge as
a pointer, they cannot keep comparing dp->bridge to the provided
pointer, since this only points to an on-stack copy. To make this
obvious and prevent driver writers from forgetting and doing stupid
things, in this new API, the struct dsa_bridge is provided as a full
structure (not very large, contains an int and a pointer) instead of a
pointer. An explicit comparison function needs to be used to determine
bridge membership: dsa_port_offloads_bridge().
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v1->v2: don't call dsa_bridge_num_find() from dsa_bridge_num_put(), it
isn't needed, as pointed out by Alvin. Also add a comment to
clarify why.
drivers/net/dsa/b53/b53_common.c | 8 +--
drivers/net/dsa/b53/b53_priv.h | 4 +-
drivers/net/dsa/dsa_loop.c | 8 +--
drivers/net/dsa/hirschmann/hellcreek.c | 4 +-
drivers/net/dsa/lan9303-core.c | 4 +-
drivers/net/dsa/lantiq_gswip.c | 14 +++--
drivers/net/dsa/microchip/ksz_common.c | 4 +-
drivers/net/dsa/microchip/ksz_common.h | 4 +-
drivers/net/dsa/mt7530.c | 8 +--
drivers/net/dsa/mv88e6xxx/chip.c | 26 ++++-----
drivers/net/dsa/ocelot/felix.c | 8 +--
drivers/net/dsa/qca8k.c | 12 ++--
drivers/net/dsa/rtl8366rb.c | 8 +--
drivers/net/dsa/sja1105/sja1105_main.c | 12 ++--
drivers/net/dsa/xrs700x/xrs700x.c | 10 ++--
include/linux/dsa/8021q.h | 7 +--
include/net/dsa.h | 77 +++++++++++++++++++++-----
net/dsa/dsa2.c | 43 +++++++++-----
net/dsa/dsa_priv.h | 53 ++----------------
net/dsa/port.c | 69 ++++++++++++-----------
net/dsa/slave.c | 2 +-
net/dsa/switch.c | 13 +++--
net/dsa/tag_8021q.c | 12 ++--
23 files changed, 219 insertions(+), 191 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index d5e78f51f42d..4e41b1a63108 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1860,7 +1860,7 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
}
EXPORT_SYMBOL(b53_mdb_del);
-int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
{
struct b53_device *dev = ds->priv;
s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
@@ -1887,7 +1887,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
b53_for_each_port(dev, i) {
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* Add this local port to the remote port VLAN control
@@ -1911,7 +1911,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
}
EXPORT_SYMBOL(b53_br_join);
-void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
+void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
{
struct b53_device *dev = ds->priv;
struct b53_vlan *vl = &dev->vlans[0];
@@ -1923,7 +1923,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
b53_for_each_port(dev, i) {
/* Don't touch the remaining ports */
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), ®);
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 579da74ada64..ee17f8b516ca 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -324,8 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
int b53_get_sset_count(struct dsa_switch *ds, int port, int sset);
void b53_get_ethtool_phy_stats(struct dsa_switch *ds, int port, uint64_t *data);
-int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
-void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
+void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
void b53_br_fast_age(struct dsa_switch *ds, int port);
int b53_br_flags_pre(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index e638e3eea911..70db3a9aa355 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -167,19 +167,19 @@ static int dsa_loop_phy_write(struct dsa_switch *ds, int port,
}
static int dsa_loop_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
- __func__, port, bridge->name);
+ __func__, port, bridge.dev->name);
return 0;
}
static void dsa_loop_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
- __func__, port, bridge->name);
+ __func__, port, bridge.dev->name);
}
static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 86839b43011b..c8dc83c69147 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -674,7 +674,7 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
}
static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct hellcreek *hellcreek = ds->priv;
@@ -691,7 +691,7 @@ static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
}
static void hellcreek_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct hellcreek *hellcreek = ds->priv;
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 1c2bdcde6979..29d909484275 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1103,7 +1103,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
}
static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct lan9303 *chip = ds->priv;
@@ -1117,7 +1117,7 @@ static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
}
static void lan9303_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct lan9303 *chip = ds->priv;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 6317d0ae42d0..1f59fefc29c1 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1146,16 +1146,17 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
}
static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
+ struct net_device *br = bridge.dev;
struct gswip_priv *priv = ds->priv;
int err;
/* When the bridge uses VLAN filtering we have to configure VLAN
* specific bridges. No bridge is configured here.
*/
- if (!br_vlan_enabled(bridge)) {
- err = gswip_vlan_add_unaware(priv, bridge, port);
+ if (!br_vlan_enabled(br)) {
+ err = gswip_vlan_add_unaware(priv, br, port);
if (err)
return err;
priv->port_vlan_filter &= ~BIT(port);
@@ -1166,8 +1167,9 @@ static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
}
static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
+ struct net_device *br = bridge.dev;
struct gswip_priv *priv = ds->priv;
gswip_add_single_port_br(priv, port, true);
@@ -1175,8 +1177,8 @@ static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
/* When the bridge uses VLAN filtering we have to configure VLAN
* specific bridges. No bridge is configured here.
*/
- if (!br_vlan_enabled(bridge))
- gswip_vlan_remove(priv, bridge, port, 0, true, false);
+ if (!br_vlan_enabled(br))
+ gswip_vlan_remove(priv, br, port, 0, true, false);
}
static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index cebcb73cda76..40d6e3f4deb5 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -192,7 +192,7 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
int ksz_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
/* port_stp_state_set() will be called after to put the port in
* appropriate state so there is no need to do anything.
@@ -203,7 +203,7 @@ int ksz_port_bridge_join(struct dsa_switch *ds, int port,
EXPORT_SYMBOL_GPL(ksz_port_bridge_join);
void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
/* port_stp_state_set() will be called after to put the port in
* forwarding state so there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 54b456bc8972..88e5a5d56219 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -155,9 +155,9 @@ void ksz_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
int ksz_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br);
+ struct dsa_bridge bridge);
void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br);
+ struct dsa_bridge bridge);
void ksz_port_fast_age(struct dsa_switch *ds, int port);
int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
void *data);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 79bde263b06f..85cc5aca7f96 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1186,7 +1186,7 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
static int
mt7530_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
struct mt7530_priv *priv = ds->priv;
u32 port_bitmap = BIT(MT7530_CPU_PORT);
@@ -1202,7 +1202,7 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
* and not being setup until the port becomes enabled.
*/
if (dsa_port_is_user(other_dp) && i != port) {
- if (dsa_port_bridge_dev_get(other_dp) != bridge)
+ if (!dsa_port_offloads_bridge(other_dp, &bridge))
continue;
if (priv->ports[i].enable)
mt7530_set(priv, MT7530_PCR_P(i),
@@ -1301,7 +1301,7 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
static void
mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
struct mt7530_priv *priv = ds->priv;
int i;
@@ -1316,7 +1316,7 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
* is kept and not being setup until the port becomes enabled.
*/
if (dsa_port_is_user(other_dp) && i != port) {
- if (dsa_port_bridge_dev_get(other_dp) != bridge)
+ if (!dsa_port_offloads_bridge(other_dp, &bridge))
continue;
if (priv->ports[i].enable)
mt7530_clear(priv, MT7530_PCR_P(i),
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c583ece83b24..b06ac29a1f7b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2420,7 +2420,7 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
}
static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct dsa_switch *ds = chip->ds;
struct dsa_switch_tree *dst = ds->dst;
@@ -2428,7 +2428,7 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
int err;
list_for_each_entry(dp, &dst->ports, list) {
- if (dsa_port_bridge_dev_get(dp) == br) {
+ if (dsa_port_offloads_bridge(dp, &bridge)) {
if (dp->ds == ds) {
/* This is a local bridge group member,
* remap its Port VLAN Map.
@@ -2452,14 +2452,14 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
}
static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct mv88e6xxx_chip *chip = ds->priv;
int err;
mv88e6xxx_reg_lock(chip);
- err = mv88e6xxx_bridge_map(chip, br);
+ err = mv88e6xxx_bridge_map(chip, bridge);
if (err)
goto unlock;
@@ -2474,14 +2474,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
}
static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct mv88e6xxx_chip *chip = ds->priv;
int err;
mv88e6xxx_reg_lock(chip);
- if (mv88e6xxx_bridge_map(chip, br) ||
+ if (mv88e6xxx_bridge_map(chip, bridge) ||
mv88e6xxx_port_vlan_map(chip, port))
dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
@@ -2496,7 +2496,7 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
static int mv88e6xxx_crosschip_bridge_join(struct dsa_switch *ds,
int tree_index, int sw_index,
- int port, struct net_device *br)
+ int port, struct dsa_bridge bridge)
{
struct mv88e6xxx_chip *chip = ds->priv;
int err;
@@ -2513,7 +2513,7 @@ static int mv88e6xxx_crosschip_bridge_join(struct dsa_switch *ds,
static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
int tree_index, int sw_index,
- int port, struct net_device *br)
+ int port, struct dsa_bridge bridge)
{
struct mv88e6xxx_chip *chip = ds->priv;
@@ -2545,19 +2545,17 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
}
static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
- return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
+ return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
}
static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
int err;
- err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
+ err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
if (err) {
dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
ERR_PTR(err));
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 0e102caddb73..e563bafca74f 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -706,21 +706,21 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
}
static int felix_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct ocelot *ocelot = ds->priv;
- ocelot_port_bridge_join(ocelot, port, br);
+ ocelot_port_bridge_join(ocelot, port, bridge.dev);
return 0;
}
static void felix_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
struct ocelot *ocelot = ds->priv;
- ocelot_port_bridge_leave(ocelot, port, br);
+ ocelot_port_bridge_leave(ocelot, port, bridge.dev);
}
static int felix_lag_join(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 7053a3510d71..dc983f79f0d6 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1810,8 +1810,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
QCA8K_PORT_LOOKUP_STATE_MASK, stp_state);
}
-static int
-qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
+static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
+ struct dsa_bridge bridge)
{
struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
int port_mask, cpu_port;
@@ -1823,7 +1823,7 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
for (i = 0; i < QCA8K_NUM_PORTS; i++) {
if (dsa_is_cpu_port(ds, i))
continue;
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* Add this port to the portvlan mask of the other ports
* in the bridge
@@ -1844,8 +1844,8 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
return ret;
}
-static void
-qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
+static void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
+ struct dsa_bridge bridge)
{
struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
int cpu_port, i;
@@ -1855,7 +1855,7 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
for (i = 0; i < QCA8K_NUM_PORTS; i++) {
if (dsa_is_cpu_port(ds, i))
continue;
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* Remove this port to the portvlan mask of the other ports
* in the bridge
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index b6f277a04989..fac2333a3f5e 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1186,7 +1186,7 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
static int
rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
struct realtek_smi *smi = ds->priv;
unsigned int port_bitmap = 0;
@@ -1198,7 +1198,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
if (i == port)
continue;
/* Not on this bridge */
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* Join this port to each other port on the bridge */
ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
@@ -1218,7 +1218,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
static void
rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
struct realtek_smi *smi = ds->priv;
unsigned int port_bitmap = 0;
@@ -1230,7 +1230,7 @@ rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
if (i == port)
continue;
/* Not on this bridge */
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* Remove this port from any other port on the bridge */
ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 5e03eda4c16f..24584fe2e760 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1980,7 +1980,7 @@ static int sja1105_manage_flood_domains(struct sja1105_private *priv)
}
static int sja1105_bridge_member(struct dsa_switch *ds, int port,
- struct net_device *br, bool member)
+ struct dsa_bridge bridge, bool member)
{
struct sja1105_l2_forwarding_entry *l2_fwd;
struct sja1105_private *priv = ds->priv;
@@ -2005,7 +2005,7 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
*/
if (i == port)
continue;
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
sja1105_port_allow_traffic(l2_fwd, i, port, member);
sja1105_port_allow_traffic(l2_fwd, port, i, member);
@@ -2074,15 +2074,15 @@ static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
}
static int sja1105_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
- return sja1105_bridge_member(ds, port, br, true);
+ return sja1105_bridge_member(ds, port, bridge, true);
}
static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *br)
+ struct dsa_bridge bridge)
{
- sja1105_bridge_member(ds, port, br, false);
+ sja1105_bridge_member(ds, port, bridge, false);
}
#define BYTES_PER_KBIT (1000LL / 8)
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 7c2b6c32242d..ebb55dfd9c4e 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -501,7 +501,7 @@ static void xrs700x_mac_link_up(struct dsa_switch *ds, int port,
}
static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
- struct net_device *bridge, bool join)
+ struct dsa_bridge bridge, bool join)
{
unsigned int i, cpu_mask = 0, mask = 0;
struct xrs700x *priv = ds->priv;
@@ -513,14 +513,14 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
cpu_mask |= BIT(i);
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) == bridge)
+ if (dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
mask |= BIT(i);
}
for (i = 0; i < ds->num_ports; i++) {
- if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+ if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
continue;
/* 1 = Disable forwarding to the port */
@@ -540,13 +540,13 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
}
static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
return xrs700x_bridge_common(ds, port, bridge, true);
}
static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
- struct net_device *bridge)
+ struct dsa_bridge bridge)
{
xrs700x_bridge_common(ds, port, bridge, false);
}
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 0af4371fbebb..939a1beaddf7 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -7,6 +7,7 @@
#include <linux/refcount.h>
#include <linux/types.h>
+#include <net/dsa.h>
struct dsa_switch;
struct dsa_port;
@@ -37,12 +38,10 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num);
+ struct dsa_bridge bridge);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num);
+ struct dsa_bridge bridge);
u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 18bce0383267..b9789c0cd5e3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -219,6 +219,11 @@ struct dsa_mall_tc_entry {
};
};
+struct dsa_bridge {
+ struct net_device *dev;
+ unsigned int num;
+ refcount_t refcount;
+};
struct dsa_port {
/* A CPU port is physically connected to a master device.
@@ -256,8 +261,7 @@ struct dsa_port {
/* Managed by DSA on user ports and by drivers on CPU and DSA ports */
bool learning;
u8 stp_state;
- struct net_device *bridge_dev;
- unsigned int bridge_num;
+ struct dsa_bridge *bridge;
struct devlink_port devlink_port;
bool devlink_port_setup;
struct phylink *pl;
@@ -588,7 +592,7 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
static inline
struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
{
- if (!dp->bridge_dev)
+ if (!dp->bridge)
return NULL;
if (dp->lag_dev)
@@ -602,12 +606,12 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
static inline struct net_device *
dsa_port_bridge_dev_get(const struct dsa_port *dp)
{
- return dp->bridge_dev;
+ return dp->bridge ? dp->bridge->dev : NULL;
}
static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp)
{
- return dp->bridge_num;
+ return dp->bridge ? dp->bridge->num : 0;
}
static inline bool dsa_port_bridge_same(const struct dsa_port *a,
@@ -620,6 +624,55 @@ static inline bool dsa_port_bridge_same(const struct dsa_port *a,
return (!br_a || !br_b) ? false : (br_a == br_b);
}
+static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
+ const struct net_device *dev)
+{
+ return dsa_port_to_bridge_port(dp) == dev;
+}
+
+static inline bool
+dsa_port_offloads_bridge_dev(struct dsa_port *dp,
+ const struct net_device *bridge_dev)
+{
+ /* DSA ports connected to a bridge, and event was emitted
+ * for the bridge.
+ */
+ return dsa_port_bridge_dev_get(dp) == bridge_dev;
+}
+
+static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
+ const struct dsa_bridge *bridge)
+{
+ return dsa_port_bridge_dev_get(dp) == bridge->dev;
+}
+
+/* Returns true if any port of this tree offloads the given net_device */
+static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
+ const struct net_device *dev)
+{
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_offloads_bridge_port(dp, dev))
+ return true;
+
+ return false;
+}
+
+/* Returns true if any port of this tree offloads the given bridge */
+static inline bool
+dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
+ const struct net_device *bridge_dev)
+{
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
+ return true;
+
+ return false;
+}
+
typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
bool is_static, void *data);
struct dsa_switch_ops {
@@ -769,17 +822,15 @@ struct dsa_switch_ops {
*/
int (*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs);
int (*port_bridge_join)(struct dsa_switch *ds, int port,
- struct net_device *bridge);
+ struct dsa_bridge bridge);
void (*port_bridge_leave)(struct dsa_switch *ds, int port,
- struct net_device *bridge);
+ struct dsa_bridge bridge);
/* Called right after .port_bridge_join() */
int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
- struct net_device *bridge,
- unsigned int bridge_num);
+ struct dsa_bridge bridge);
/* Called right before .port_bridge_leave() */
void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
- struct net_device *bridge,
- unsigned int bridge_num);
+ struct dsa_bridge bridge);
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
void (*port_fast_age)(struct dsa_switch *ds, int port);
@@ -851,10 +902,10 @@ struct dsa_switch_ops {
*/
int (*crosschip_bridge_join)(struct dsa_switch *ds, int tree_index,
int sw_index, int port,
- struct net_device *br);
+ struct dsa_bridge bridge);
void (*crosschip_bridge_leave)(struct dsa_switch *ds, int tree_index,
int sw_index, int port,
- struct net_device *br);
+ struct dsa_bridge bridge);
int (*crosschip_lag_change)(struct dsa_switch *ds, int sw_index,
int port);
int (*crosschip_lag_join)(struct dsa_switch *ds, int sw_index,
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4901cdc264ee..8814fa0e44c8 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -129,20 +129,29 @@ void dsa_lag_unmap(struct dsa_switch_tree *dst, struct net_device *lag)
}
}
+struct dsa_bridge *dsa_tree_bridge_find(struct dsa_switch_tree *dst,
+ const struct net_device *br)
+{
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_bridge_dev_get(dp) == br)
+ return dp->bridge;
+
+ return NULL;
+}
+
static int dsa_bridge_num_find(const struct net_device *bridge_dev)
{
struct dsa_switch_tree *dst;
- struct dsa_port *dp;
- /* When preparing the offload for a port, it will have a valid
- * dp->bridge_dev pointer but a not yet valid dp->bridge_num.
- * However there might be other ports having the same dp->bridge_dev
- * and a valid dp->bridge_num, so just ignore this port.
- */
- list_for_each_entry(dst, &dsa_tree_list, list)
- list_for_each_entry(dp, &dst->ports, list)
- if (dp->bridge_dev == bridge_dev && dp->bridge_num)
- return dp->bridge_num;
+ list_for_each_entry(dst, &dsa_tree_list, list) {
+ struct dsa_bridge *bridge;
+
+ bridge = dsa_tree_bridge_find(dst, bridge_dev);
+ if (bridge)
+ return bridge->num;
+ }
return 0;
}
@@ -151,6 +160,12 @@ unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
{
unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
+ /* Switches without FDB isolation support don't get unique
+ * bridge numbering
+ */
+ if (!max)
+ return 0;
+
if (!bridge_num) {
/* First port that requests FDB isolation or TX forwarding
* offload for this bridge
@@ -170,11 +185,11 @@ unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
void dsa_bridge_num_put(const struct net_device *bridge_dev,
unsigned int bridge_num)
{
- /* Check if the bridge is still in use, otherwise it is time
- * to clean it up so we can reuse this bridge_num later.
+ /* Since we refcount bridges, we know that when we call this function
+ * it is no longer in use, so we can just go ahead and remove it from
+ * the bit mask.
*/
- if (!dsa_bridge_num_find(bridge_dev))
- clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
+ clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
}
struct dsa_switch *dsa_switch_find(int tree_index, int sw_index)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 33fef1be62a3..da6ff99ba5ed 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -52,7 +52,7 @@ struct dsa_notifier_ageing_time_info {
/* DSA_NOTIFIER_BRIDGE_* */
struct dsa_notifier_bridge_info {
- struct net_device *br;
+ struct dsa_bridge bridge;
int tree_index;
int sw_index;
int port;
@@ -266,49 +266,6 @@ void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
-static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
- const struct net_device *dev)
-{
- return dsa_port_to_bridge_port(dp) == dev;
-}
-
-static inline bool
-dsa_port_offloads_bridge_dev(struct dsa_port *dp,
- const struct net_device *bridge_dev)
-{
- /* DSA ports connected to a bridge, and event was emitted
- * for the bridge.
- */
- return dsa_port_bridge_dev_get(dp) == bridge_dev;
-}
-
-/* Returns true if any port of this tree offloads the given net_device */
-static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
- const struct net_device *dev)
-{
- struct dsa_port *dp;
-
- list_for_each_entry(dp, &dst->ports, list)
- if (dsa_port_offloads_bridge_port(dp, dev))
- return true;
-
- return false;
-}
-
-/* Returns true if any port of this tree offloads the given bridge */
-static inline bool
-dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
- const struct net_device *bridge_dev)
-{
- struct dsa_port *dp;
-
- list_for_each_entry(dp, &dst->ports, list)
- if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
- return true;
-
- return false;
-}
-
/* slave.c */
extern const struct dsa_device_ops notag_netdev_ops;
extern struct notifier_block dsa_slave_switchdev_notifier;
@@ -417,7 +374,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
if (dp->type != DSA_PORT_TYPE_USER)
continue;
- if (!dp->bridge_dev)
+ if (!dp->bridge)
continue;
if (dp->stp_state != BR_STATE_LEARNING &&
@@ -446,7 +403,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
/* If the ingress port offloads the bridge, we mark the frame as autonomously
* forwarded by hardware, so the software bridge doesn't forward in twice, back
* to us, because we already did. However, if we're in fallback mode and we do
- * software bridging, we are not offloading it, therefore the dp->bridge_dev
+ * software bridging, we are not offloading it, therefore the dp->bridge
* pointer is not populated, and flooding needs to be done by software (we are
* effectively operating in standalone ports mode).
*/
@@ -454,7 +411,7 @@ static inline void dsa_default_offload_fwd_mark(struct sk_buff *skb)
{
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- skb->offload_fwd_mark = !!(dp->bridge_dev);
+ skb->offload_fwd_mark = !!(dp->bridge);
}
/* Helper for removing DSA header tags from packets in the RX path.
@@ -551,6 +508,8 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
void dsa_bridge_num_put(const struct net_device *bridge_dev,
unsigned int bridge_num);
+struct dsa_bridge *dsa_tree_bridge_find(struct dsa_switch_tree *dst,
+ const struct net_device *br);
/* tag_8021q.c */
int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index f6ea41cbcdd5..b6c8a1a9ec18 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -130,7 +130,7 @@ int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
return err;
}
- if (!dp->bridge_dev)
+ if (!dp->bridge)
dsa_port_set_state_now(dp, BR_STATE_FORWARDING, false);
if (dp->pl)
@@ -158,7 +158,7 @@ void dsa_port_disable_rt(struct dsa_port *dp)
if (dp->pl)
phylink_stop(dp->pl);
- if (!dp->bridge_dev)
+ if (!dp->bridge)
dsa_port_set_state_now(dp, BR_STATE_DISABLED, false);
if (ds->ops->port_disable)
@@ -271,36 +271,32 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
}
static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
- struct net_device *bridge_dev,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
struct dsa_switch *ds = dp->ds;
/* No bridge TX forwarding offload => do nothing */
- if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
+ if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
return;
/* Notify the chips only once the offload has been deactivated, so
* that they can update their configuration accordingly.
*/
- ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge_dev,
- bridge_num);
+ ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
}
static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
- struct net_device *bridge_dev,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
struct dsa_switch *ds = dp->ds;
int err;
/* FDB isolation is required for TX forwarding offload */
- if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
+ if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
return false;
/* Notify the driver */
- err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
- bridge_num);
+ err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
return err ? false : true;
}
@@ -310,21 +306,31 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
struct netlink_ext_ack *extack)
{
struct dsa_switch *ds = dp->ds;
- unsigned int bridge_num;
+ struct dsa_bridge *bridge;
- dp->bridge_dev = br;
-
- if (!ds->max_num_bridges)
+ bridge = dsa_tree_bridge_find(ds->dst, br);
+ if (bridge) {
+ refcount_inc(&bridge->refcount);
+ dp->bridge = bridge;
return 0;
+ }
+
+ bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
+ if (!bridge)
+ return -ENOMEM;
+
+ refcount_set(&bridge->refcount, 1);
+
+ bridge->dev = br;
- bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
- if (!bridge_num) {
+ bridge->num = dsa_bridge_num_get(br, ds->max_num_bridges);
+ if (ds->max_num_bridges && !bridge->num) {
NL_SET_ERR_MSG_MOD(extack,
"Range of offloadable bridges exceeded");
return -EOPNOTSUPP;
}
- dp->bridge_num = bridge_num;
+ dp->bridge = bridge;
return 0;
}
@@ -332,16 +338,17 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
static void dsa_port_bridge_destroy(struct dsa_port *dp,
const struct net_device *br)
{
- struct dsa_switch *ds = dp->ds;
+ struct dsa_bridge *bridge = dp->bridge;
+
+ dp->bridge = NULL;
- dp->bridge_dev = NULL;
+ if (!refcount_dec_and_test(&bridge->refcount))
+ return;
- if (ds->max_num_bridges) {
- int bridge_num = dp->bridge_num;
+ if (bridge->num)
+ dsa_bridge_num_put(br, bridge->num);
- dp->bridge_num = 0;
- dsa_bridge_num_put(br, bridge_num);
- }
+ kfree(bridge);
}
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
@@ -351,7 +358,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
.tree_index = dp->ds->dst->index,
.sw_index = dp->ds->index,
.port = dp->index,
- .br = br,
};
struct net_device *dev = dp->slave;
struct net_device *brport_dev;
@@ -367,12 +373,12 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
brport_dev = dsa_port_to_bridge_port(dp);
+ info.bridge = *dp->bridge;
err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info);
if (err)
goto out_rollback;
- tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
- dsa_port_bridge_num_get(dp));
+ tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
err = switchdev_bridge_port_offload(brport_dev, dev, dp,
&dsa_slave_switchdev_notifier,
@@ -415,12 +421,11 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br)
void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
{
- unsigned int bridge_num = dsa_port_bridge_num_get(dp);
struct dsa_notifier_bridge_info info = {
.tree_index = dp->ds->dst->index,
.sw_index = dp->ds->index,
.port = dp->index,
- .br = br,
+ .bridge = *dp->bridge,
};
int err;
@@ -429,7 +434,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
*/
dsa_port_bridge_destroy(dp, br);
- dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
+ dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
if (err)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7c7aea19db08..2b153b366118 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1564,7 +1564,7 @@ static void dsa_bridge_mtu_normalization(struct dsa_port *dp)
if (!dp->ds->mtu_enforcement_ingress)
return;
- if (!dp->bridge_dev)
+ if (!dp->bridge)
return;
INIT_LIST_HEAD(&hw_port_list);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 7993192fe769..cd0630dd5417 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -95,7 +95,7 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
if (!ds->ops->port_bridge_join)
return -EOPNOTSUPP;
- err = ds->ops->port_bridge_join(ds, info->port, info->br);
+ err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
if (err)
return err;
}
@@ -104,7 +104,7 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
ds->ops->crosschip_bridge_join) {
err = ds->ops->crosschip_bridge_join(ds, info->tree_index,
info->sw_index,
- info->port, info->br);
+ info->port, info->bridge);
if (err)
return err;
}
@@ -124,19 +124,20 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
if (dst->index == info->tree_index && ds->index == info->sw_index &&
ds->ops->port_bridge_leave)
- ds->ops->port_bridge_leave(ds, info->port, info->br);
+ ds->ops->port_bridge_leave(ds, info->port, info->bridge);
if ((dst->index != info->tree_index || ds->index != info->sw_index) &&
ds->ops->crosschip_bridge_leave)
ds->ops->crosschip_bridge_leave(ds, info->tree_index,
info->sw_index, info->port,
- info->br);
+ info->bridge);
- if (ds->needs_standalone_vlan_filtering && !br_vlan_enabled(info->br)) {
+ if (ds->needs_standalone_vlan_filtering &&
+ !br_vlan_enabled(info->bridge.dev)) {
change_vlan_filtering = true;
vlan_filtering = true;
} else if (!ds->needs_standalone_vlan_filtering &&
- br_vlan_enabled(info->br)) {
+ br_vlan_enabled(info->bridge.dev)) {
change_vlan_filtering = true;
vlan_filtering = false;
}
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index e9d5e566973c..27712a81c967 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -337,7 +337,7 @@ dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
return false;
if (dsa_port_is_user(dp))
- return dsa_port_bridge_dev_get(dp) == info->br;
+ return dsa_port_offloads_bridge(dp, &info->bridge);
return false;
}
@@ -410,10 +410,9 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
}
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
- u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+ u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid,
true);
@@ -421,10 +420,9 @@ int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
- struct net_device *br,
- unsigned int bridge_num)
+ struct dsa_bridge bridge)
{
- u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+ u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid, true);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 5/7] net: dsa: keep the bridge_dev and bridge_num as part of the same structure
2021-12-04 20:11 ` [PATCH v2 net-next 5/7] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
@ 2021-12-04 23:18 ` Vladimir Oltean
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
On Sat, Dec 04, 2021 at 10:11:44PM +0200, Vladimir Oltean wrote:
> @@ -310,21 +306,31 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
> struct netlink_ext_ack *extack)
> {
> struct dsa_switch *ds = dp->ds;
> - unsigned int bridge_num;
> + struct dsa_bridge *bridge;
>
> - dp->bridge_dev = br;
> -
> - if (!ds->max_num_bridges)
> + bridge = dsa_tree_bridge_find(ds->dst, br);
> + if (bridge) {
> + refcount_inc(&bridge->refcount);
> + dp->bridge = bridge;
> return 0;
> + }
> +
> + bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
> + if (!bridge)
> + return -ENOMEM;
> +
> + refcount_set(&bridge->refcount, 1);
> +
> + bridge->dev = br;
>
> - bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
> - if (!bridge_num) {
> + bridge->num = dsa_bridge_num_get(br, ds->max_num_bridges);
> + if (ds->max_num_bridges && !bridge->num) {
> NL_SET_ERR_MSG_MOD(extack,
> "Range of offloadable bridges exceeded");
There should be a kfree(bridge) here before returning -EOPNOTSUPP.
I'll wait for further feedback before reposting.
> return -EOPNOTSUPP;
> }
>
> - dp->bridge_num = bridge_num;
> + dp->bridge = bridge;
>
> return 0;
> }
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 6/7] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
` (4 preceding siblings ...)
2021-12-04 20:11 ` [PATCH v2 net-next 5/7] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-06 11:01 ` Alvin Šipraga
2021-12-04 20:11 ` [PATCH v2 net-next 7/7] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
6 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
This is a preparation patch for the removal of the DSA switch methods
->port_bridge_tx_fwd_offload() and ->port_bridge_tx_fwd_unoffload().
The plan is for the switch to report whether it offloads TX forwarding
directly as a response to the ->port_bridge_join() method.
This change deals with the noisy portion of converting all existing
function prototypes to take this new boolean pointer argument.
The bool is placed in the cross-chip notifier structure for bridge join,
and a reference to it is provided to drivers. In the next change, DSA
will then actually look at this value instead of calling
->port_bridge_tx_fwd_offload().
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is kind of new, the noisy conversion from v1's patch 6/6
has been split into a separate change. Had to drop Alvin's
Reviewed-by tag because he technically did not review the change
in this form.
drivers/net/dsa/b53/b53_common.c | 3 ++-
drivers/net/dsa/b53/b53_priv.h | 3 ++-
drivers/net/dsa/dsa_loop.c | 3 ++-
drivers/net/dsa/hirschmann/hellcreek.c | 3 ++-
drivers/net/dsa/lan9303-core.c | 3 ++-
drivers/net/dsa/lantiq_gswip.c | 3 ++-
drivers/net/dsa/microchip/ksz_common.c | 3 ++-
drivers/net/dsa/microchip/ksz_common.h | 2 +-
drivers/net/dsa/mt7530.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 3 ++-
drivers/net/dsa/ocelot/felix.c | 2 +-
drivers/net/dsa/qca8k.c | 3 ++-
drivers/net/dsa/rtl8366rb.c | 3 ++-
drivers/net/dsa/sja1105/sja1105_main.c | 3 ++-
drivers/net/dsa/xrs700x/xrs700x.c | 2 +-
include/net/dsa.h | 3 ++-
net/dsa/dsa_priv.h | 1 +
net/dsa/switch.c | 3 ++-
18 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 4e41b1a63108..3867f3d4545f 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1860,7 +1860,8 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
}
EXPORT_SYMBOL(b53_mdb_del);
-int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct b53_device *dev = ds->priv;
s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index ee17f8b516ca..b41dc8ac2ca8 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -324,7 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
int b53_get_sset_count(struct dsa_switch *ds, int port, int sset);
void b53_get_ethtool_phy_stats(struct dsa_switch *ds, int port, uint64_t *data);
-int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
+ bool *tx_fwd_offload);
void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
void b53_br_fast_age(struct dsa_switch *ds, int port);
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 70db3a9aa355..33daaf10c488 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -167,7 +167,8 @@ static int dsa_loop_phy_write(struct dsa_switch *ds, int port,
}
static int dsa_loop_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
__func__, port, bridge.dev->name);
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index c8dc83c69147..9eecb7529573 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -674,7 +674,8 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
}
static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct hellcreek *hellcreek = ds->priv;
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 29d909484275..d55784d19fa4 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1103,7 +1103,8 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
}
static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct lan9303 *chip = ds->priv;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 1f59fefc29c1..46ed953e787e 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1146,7 +1146,8 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
}
static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct net_device *br = bridge.dev;
struct gswip_priv *priv = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 40d6e3f4deb5..47a856533cff 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -192,7 +192,8 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
int ksz_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
/* port_stp_state_set() will be called after to put the port in
* appropriate state so there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 88e5a5d56219..df8ae59c8525 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -155,7 +155,7 @@ void ksz_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
int ksz_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge);
+ struct dsa_bridge bridge, bool *tx_fwd_offload);
void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
struct dsa_bridge bridge);
void ksz_port_fast_age(struct dsa_switch *ds, int port);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 85cc5aca7f96..dcd1a3932cfb 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1186,7 +1186,7 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
static int
mt7530_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge, bool *tx_fwd_offload)
{
struct mt7530_priv *priv = ds->priv;
u32 port_bitmap = BIT(MT7530_CPU_PORT);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b06ac29a1f7b..c30d1f825776 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2452,7 +2452,8 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
}
static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct mv88e6xxx_chip *chip = ds->priv;
int err;
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index e563bafca74f..f76dcf0d369f 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -706,7 +706,7 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
}
static int felix_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge, bool *tx_fwd_offload)
{
struct ocelot *ocelot = ds->priv;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index dc983f79f0d6..039694518788 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1811,7 +1811,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
}
static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
int port_mask, cpu_port;
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index fac2333a3f5e..ecc19bd5115f 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1186,7 +1186,8 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
static int
rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
struct realtek_smi *smi = ds->priv;
unsigned int port_bitmap = 0;
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 24584fe2e760..21622c60faab 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2074,7 +2074,8 @@ static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
}
static int sja1105_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload)
{
return sja1105_bridge_member(ds, port, bridge, true);
}
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index ebb55dfd9c4e..35fa19ddaf19 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -540,7 +540,7 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
}
static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
+ struct dsa_bridge bridge, bool *tx_fwd_offload)
{
return xrs700x_bridge_common(ds, port, bridge, true);
}
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b9789c0cd5e3..584b3f9462a0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -822,7 +822,8 @@ struct dsa_switch_ops {
*/
int (*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs);
int (*port_bridge_join)(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge);
+ struct dsa_bridge bridge,
+ bool *tx_fwd_offload);
void (*port_bridge_leave)(struct dsa_switch *ds, int port,
struct dsa_bridge bridge);
/* Called right after .port_bridge_join() */
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index da6ff99ba5ed..38ce5129a33d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -56,6 +56,7 @@ struct dsa_notifier_bridge_info {
int tree_index;
int sw_index;
int port;
+ bool tx_fwd_offload;
};
/* DSA_NOTIFIER_FDB_* */
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index cd0630dd5417..9c92edd96961 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -95,7 +95,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
if (!ds->ops->port_bridge_join)
return -EOPNOTSUPP;
- err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
+ err = ds->ops->port_bridge_join(ds, info->port, info->bridge,
+ &info->tx_fwd_offload);
if (err)
return err;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 6/7] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join
2021-12-04 20:11 ` [PATCH v2 net-next 6/7] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join Vladimir Oltean
@ 2021-12-06 11:01 ` Alvin Šipraga
0 siblings, 0 replies; 12+ messages in thread
From: Alvin Šipraga @ 2021-12-06 11:01 UTC (permalink / raw)
To: Vladimir Oltean, netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
UNGLinuxDriver, Sean Wang, Landen Chao, Matthias Brugger,
Claudiu Manoil, Alexandre Belloni, Linus Walleij,
George McCollister
On 12/4/21 21:11, Vladimir Oltean wrote:
> This is a preparation patch for the removal of the DSA switch methods
> ->port_bridge_tx_fwd_offload() and ->port_bridge_tx_fwd_unoffload().
> The plan is for the switch to report whether it offloads TX forwarding
> directly as a response to the ->port_bridge_join() method.
>
> This change deals with the noisy portion of converting all existing
> function prototypes to take this new boolean pointer argument.
> The bool is placed in the cross-chip notifier structure for bridge join,
> and a reference to it is provided to drivers. In the next change, DSA
> will then actually look at this value instead of calling
> ->port_bridge_tx_fwd_offload().
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> v1->v2: patch is kind of new, the noisy conversion from v1's patch 6/6
> has been split into a separate change. Had to drop Alvin's
> Reviewed-by tag because he technically did not review the change
> in this form.
>
> drivers/net/dsa/b53/b53_common.c | 3 ++-
> drivers/net/dsa/b53/b53_priv.h | 3 ++-
> drivers/net/dsa/dsa_loop.c | 3 ++-
> drivers/net/dsa/hirschmann/hellcreek.c | 3 ++-
> drivers/net/dsa/lan9303-core.c | 3 ++-
> drivers/net/dsa/lantiq_gswip.c | 3 ++-
> drivers/net/dsa/microchip/ksz_common.c | 3 ++-
> drivers/net/dsa/microchip/ksz_common.h | 2 +-
> drivers/net/dsa/mt7530.c | 2 +-
> drivers/net/dsa/mv88e6xxx/chip.c | 3 ++-
> drivers/net/dsa/ocelot/felix.c | 2 +-
> drivers/net/dsa/qca8k.c | 3 ++-
> drivers/net/dsa/rtl8366rb.c | 3 ++-
> drivers/net/dsa/sja1105/sja1105_main.c | 3 ++-
> drivers/net/dsa/xrs700x/xrs700x.c | 2 +-
> include/net/dsa.h | 3 ++-
> net/dsa/dsa_priv.h | 1 +
> net/dsa/switch.c | 3 ++-
> 18 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 4e41b1a63108..3867f3d4545f 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1860,7 +1860,8 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
> }
> EXPORT_SYMBOL(b53_mdb_del);
>
> -int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct b53_device *dev = ds->priv;
> s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> index ee17f8b516ca..b41dc8ac2ca8 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -324,7 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
> void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
> int b53_get_sset_count(struct dsa_switch *ds, int port, int sset);
> void b53_get_ethtool_phy_stats(struct dsa_switch *ds, int port, uint64_t *data);
> -int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> + bool *tx_fwd_offload);
> void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
> void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
> void b53_br_fast_age(struct dsa_switch *ds, int port);
> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
> index 70db3a9aa355..33daaf10c488 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -167,7 +167,8 @@ static int dsa_loop_phy_write(struct dsa_switch *ds, int port,
> }
>
> static int dsa_loop_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
> __func__, port, bridge.dev->name);
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index c8dc83c69147..9eecb7529573 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -674,7 +674,8 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
> }
>
> static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct hellcreek *hellcreek = ds->priv;
>
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 29d909484275..d55784d19fa4 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1103,7 +1103,8 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
> }
>
> static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct lan9303 *chip = ds->priv;
>
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 1f59fefc29c1..46ed953e787e 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1146,7 +1146,8 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
> }
>
> static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct net_device *br = bridge.dev;
> struct gswip_priv *priv = ds->priv;
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 40d6e3f4deb5..47a856533cff 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -192,7 +192,8 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
> EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
>
> int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> /* port_stp_state_set() will be called after to put the port in
> * appropriate state so there is no need to do anything.
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 88e5a5d56219..df8ae59c8525 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -155,7 +155,7 @@ void ksz_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
> int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
> void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
> int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge);
> + struct dsa_bridge bridge, bool *tx_fwd_offload);
> void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge);
> void ksz_port_fast_age(struct dsa_switch *ds, int port);
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 85cc5aca7f96..dcd1a3932cfb 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1186,7 +1186,7 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
>
> static int
> mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge, bool *tx_fwd_offload)
> {
> struct mt7530_priv *priv = ds->priv;
> u32 port_bitmap = BIT(MT7530_CPU_PORT);
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index b06ac29a1f7b..c30d1f825776 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2452,7 +2452,8 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
> }
>
> static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct mv88e6xxx_chip *chip = ds->priv;
> int err;
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index e563bafca74f..f76dcf0d369f 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -706,7 +706,7 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
> }
>
> static int felix_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge, bool *tx_fwd_offload)
> {
> struct ocelot *ocelot = ds->priv;
>
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index dc983f79f0d6..039694518788 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -1811,7 +1811,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
> }
>
> static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> int port_mask, cpu_port;
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index fac2333a3f5e..ecc19bd5115f 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -1186,7 +1186,8 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
>
> static int
> rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> struct realtek_smi *smi = ds->priv;
> unsigned int port_bitmap = 0;
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index 24584fe2e760..21622c60faab 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -2074,7 +2074,8 @@ static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
> }
>
> static int sja1105_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload)
> {
> return sja1105_bridge_member(ds, port, bridge, true);
> }
> diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
> index ebb55dfd9c4e..35fa19ddaf19 100644
> --- a/drivers/net/dsa/xrs700x/xrs700x.c
> +++ b/drivers/net/dsa/xrs700x/xrs700x.c
> @@ -540,7 +540,7 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
> }
>
> static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> + struct dsa_bridge bridge, bool *tx_fwd_offload)
> {
> return xrs700x_bridge_common(ds, port, bridge, true);
> }
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index b9789c0cd5e3..584b3f9462a0 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -822,7 +822,8 @@ struct dsa_switch_ops {
> */
> int (*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs);
> int (*port_bridge_join)(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge);
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload);
> void (*port_bridge_leave)(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge);
> /* Called right after .port_bridge_join() */
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index da6ff99ba5ed..38ce5129a33d 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -56,6 +56,7 @@ struct dsa_notifier_bridge_info {
> int tree_index;
> int sw_index;
> int port;
> + bool tx_fwd_offload;
> };
>
> /* DSA_NOTIFIER_FDB_* */
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index cd0630dd5417..9c92edd96961 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -95,7 +95,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
> if (!ds->ops->port_bridge_join)
> return -EOPNOTSUPP;
>
> - err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
> + err = ds->ops->port_bridge_join(ds, info->port, info->bridge,
> + &info->tx_fwd_offload);
> if (err)
> return err;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 7/7] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
2021-12-04 20:11 [PATCH v2 net-next 0/7] Rework DSA bridge TX forwarding offload API Vladimir Oltean
` (5 preceding siblings ...)
2021-12-04 20:11 ` [PATCH v2 net-next 6/7] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join Vladimir Oltean
@ 2021-12-04 20:11 ` Vladimir Oltean
2021-12-06 11:01 ` Alvin Šipraga
6 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-12-04 20:11 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Alvin Šipraga, Kurt Kanzenbach,
Hauke Mehrtens, Woojung Huh, UNGLinuxDriver, Sean Wang,
Landen Chao, Matthias Brugger, Claudiu Manoil, Alexandre Belloni,
Linus Walleij, George McCollister
We don't really need new switch API for these, and with new switches
which intend to add support for this feature, it will become cumbersome
to maintain.
The change consists in restructuring the two drivers that implement this
offload (sja1105 and mv88e6xxx) such that the offload is enabled and
disabled from the ->port_bridge_{join,leave} methods instead of the old
->port_bridge_tx_fwd_{,un}offload.
The only non-trivial change is that mv88e6xxx_map_virtual_bridge_to_pvt()
has been moved to avoid a forward declaration, and the
mv88e6xxx_reg_lock() calls from inside it have been removed, since
locking is now done from mv88e6xxx_port_bridge_{join,leave}.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: avoid a deadlock in mv88e6xxx pointed out by Tobias due to the
register lock being already held in the port_bridge_join/leave
methods. Again had to drop Alvin's Reviewed-by tag.
drivers/net/dsa/mv88e6xxx/chip.c | 63 ++++++++++----------------
drivers/net/dsa/sja1105/sja1105_main.c | 19 ++++++--
include/net/dsa.h | 7 +--
net/dsa/port.c | 39 ++--------------
4 files changed, 45 insertions(+), 83 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c30d1f825776..ec515045c16e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2451,6 +2451,19 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
return 0;
}
+/* Treat the software bridge as a virtual single-port switch behind the
+ * CPU and map in the PVT. First dst->last_switch elements are taken by
+ * physical switches, so start from beyond that range.
+ */
+static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
+ unsigned int bridge_num)
+{
+ u8 dev = bridge_num + ds->dst->last_switch;
+ struct mv88e6xxx_chip *chip = ds->priv;
+
+ return mv88e6xxx_pvt_map(chip, dev, 0);
+}
+
static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
struct dsa_bridge bridge,
bool *tx_fwd_offload)
@@ -2468,6 +2481,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
if (err)
goto unlock;
+ if (mv88e6xxx_has_pvt(chip)) {
+ err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
+ if (err)
+ goto unlock;
+
+ *tx_fwd_offload = true;
+ }
+
unlock:
mv88e6xxx_reg_unlock(chip);
@@ -2482,6 +2503,10 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
mv88e6xxx_reg_lock(chip);
+ if (bridge.tx_fwd_offload &&
+ mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num))
+ dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
+
if (mv88e6xxx_bridge_map(chip, bridge) ||
mv88e6xxx_port_vlan_map(chip, port))
dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
@@ -2527,42 +2552,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
mv88e6xxx_reg_unlock(chip);
}
-/* Treat the software bridge as a virtual single-port switch behind the
- * CPU and map in the PVT. First dst->last_switch elements are taken by
- * physical switches, so start from beyond that range.
- */
-static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
- unsigned int bridge_num)
-{
- u8 dev = bridge_num + ds->dst->last_switch;
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- mv88e6xxx_reg_lock(chip);
- err = mv88e6xxx_pvt_map(chip, dev, 0);
- mv88e6xxx_reg_unlock(chip);
-
- return err;
-}
-
-static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
-{
- return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
-}
-
-static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge)
-{
- int err;
-
- err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
- if (err) {
- dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
- ERR_PTR(err));
- }
-}
-
static int mv88e6xxx_software_reset(struct mv88e6xxx_chip *chip)
{
if (chip->info->ops->reset)
@@ -6282,8 +6271,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.crosschip_lag_change = mv88e6xxx_crosschip_lag_change,
.crosschip_lag_join = mv88e6xxx_crosschip_lag_join,
.crosschip_lag_leave = mv88e6xxx_crosschip_lag_leave,
- .port_bridge_tx_fwd_offload = mv88e6xxx_bridge_tx_fwd_offload,
- .port_bridge_tx_fwd_unoffload = mv88e6xxx_bridge_tx_fwd_unoffload,
};
static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 21622c60faab..cefde41ce8d6 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2077,12 +2077,27 @@ static int sja1105_bridge_join(struct dsa_switch *ds, int port,
struct dsa_bridge bridge,
bool *tx_fwd_offload)
{
- return sja1105_bridge_member(ds, port, bridge, true);
+ int rc;
+
+ rc = sja1105_bridge_member(ds, port, bridge, true);
+ if (rc)
+ return rc;
+
+ rc = dsa_tag_8021q_bridge_tx_fwd_offload(ds, port, bridge);
+ if (rc) {
+ sja1105_bridge_member(ds, port, bridge, false);
+ return rc;
+ }
+
+ *tx_fwd_offload = true;
+
+ return 0;
}
static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
struct dsa_bridge bridge)
{
+ dsa_tag_8021q_bridge_tx_fwd_unoffload(ds, port, bridge);
sja1105_bridge_member(ds, port, bridge, false);
}
@@ -3231,8 +3246,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.tag_8021q_vlan_add = sja1105_dsa_8021q_vlan_add,
.tag_8021q_vlan_del = sja1105_dsa_8021q_vlan_del,
.port_prechangeupper = sja1105_prechangeupper,
- .port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
- .port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
};
static const struct of_device_id sja1105_dt_ids[];
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 584b3f9462a0..bdf308a5c55e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -222,6 +222,7 @@ struct dsa_mall_tc_entry {
struct dsa_bridge {
struct net_device *dev;
unsigned int num;
+ bool tx_fwd_offload;
refcount_t refcount;
};
@@ -826,12 +827,6 @@ struct dsa_switch_ops {
bool *tx_fwd_offload);
void (*port_bridge_leave)(struct dsa_switch *ds, int port,
struct dsa_bridge bridge);
- /* Called right after .port_bridge_join() */
- int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge);
- /* Called right before .port_bridge_leave() */
- void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
- struct dsa_bridge bridge);
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
void (*port_fast_age)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b6c8a1a9ec18..fbe8c054d2b8 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -270,37 +270,6 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
*/
}
-static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
- struct dsa_bridge bridge)
-{
- struct dsa_switch *ds = dp->ds;
-
- /* No bridge TX forwarding offload => do nothing */
- if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
- return;
-
- /* Notify the chips only once the offload has been deactivated, so
- * that they can update their configuration accordingly.
- */
- ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
-}
-
-static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
- struct dsa_bridge bridge)
-{
- struct dsa_switch *ds = dp->ds;
- int err;
-
- /* FDB isolation is required for TX forwarding offload */
- if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
- return false;
-
- /* Notify the driver */
- err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
-
- return err ? false : true;
-}
-
static int dsa_port_bridge_create(struct dsa_port *dp,
struct net_device *br,
struct netlink_ext_ack *extack)
@@ -361,7 +330,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
};
struct net_device *dev = dp->slave;
struct net_device *brport_dev;
- bool tx_fwd_offload;
int err;
/* Here the interface is already bridged. Reflect the current
@@ -378,12 +346,13 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
if (err)
goto out_rollback;
- tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
+ /* Drivers which support bridge TX forwarding should set this */
+ dp->bridge->tx_fwd_offload = info.tx_fwd_offload;
err = switchdev_bridge_port_offload(brport_dev, dev, dp,
&dsa_slave_switchdev_notifier,
&dsa_slave_switchdev_blocking_notifier,
- tx_fwd_offload, extack);
+ dp->bridge->tx_fwd_offload, extack);
if (err)
goto out_rollback_unbridge;
@@ -434,8 +403,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
*/
dsa_port_bridge_destroy(dp, br);
- dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
-
err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
if (err)
dev_err(dp->ds->dev,
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 7/7] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
2021-12-04 20:11 ` [PATCH v2 net-next 7/7] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
@ 2021-12-06 11:01 ` Alvin Šipraga
0 siblings, 0 replies; 12+ messages in thread
From: Alvin Šipraga @ 2021-12-06 11:01 UTC (permalink / raw)
To: Vladimir Oltean, netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
DENG Qingfang, Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
UNGLinuxDriver, Sean Wang, Landen Chao, Matthias Brugger,
Claudiu Manoil, Alexandre Belloni, Linus Walleij,
George McCollister
On 12/4/21 21:11, Vladimir Oltean wrote:
> We don't really need new switch API for these, and with new switches
> which intend to add support for this feature, it will become cumbersome
> to maintain.
>
> The change consists in restructuring the two drivers that implement this
> offload (sja1105 and mv88e6xxx) such that the offload is enabled and
> disabled from the ->port_bridge_{join,leave} methods instead of the old
> ->port_bridge_tx_fwd_{,un}offload.
>
> The only non-trivial change is that mv88e6xxx_map_virtual_bridge_to_pvt()
> has been moved to avoid a forward declaration, and the
> mv88e6xxx_reg_lock() calls from inside it have been removed, since
> locking is now done from mv88e6xxx_port_bridge_{join,leave}.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> v1->v2: avoid a deadlock in mv88e6xxx pointed out by Tobias due to the
> register lock being already held in the port_bridge_join/leave
> methods. Again had to drop Alvin's Reviewed-by tag.
>
> drivers/net/dsa/mv88e6xxx/chip.c | 63 ++++++++++----------------
> drivers/net/dsa/sja1105/sja1105_main.c | 19 ++++++--
> include/net/dsa.h | 7 +--
> net/dsa/port.c | 39 ++--------------
> 4 files changed, 45 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index c30d1f825776..ec515045c16e 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2451,6 +2451,19 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
> return 0;
> }
>
> +/* Treat the software bridge as a virtual single-port switch behind the
> + * CPU and map in the PVT. First dst->last_switch elements are taken by
> + * physical switches, so start from beyond that range.
> + */
> +static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> + unsigned int bridge_num)
> +{
> + u8 dev = bridge_num + ds->dst->last_switch;
> + struct mv88e6xxx_chip *chip = ds->priv;
> +
> + return mv88e6xxx_pvt_map(chip, dev, 0);
> +}
> +
> static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge,
> bool *tx_fwd_offload)
> @@ -2468,6 +2481,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
> if (err)
> goto unlock;
>
> + if (mv88e6xxx_has_pvt(chip)) {
> + err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> + if (err)
> + goto unlock;
> +
> + *tx_fwd_offload = true;
> + }
> +
> unlock:
> mv88e6xxx_reg_unlock(chip);
>
> @@ -2482,6 +2503,10 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
>
> mv88e6xxx_reg_lock(chip);
>
> + if (bridge.tx_fwd_offload &&
> + mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num))
> + dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
I must admit I don't see how this would ever fail in practice. Moreover,
isn't this the exact same call made in port_bridge_join, hence a noop?
Anyway, that seems to have been the case before as well, so it is not
really a problem for your patch series.
> +
> if (mv88e6xxx_bridge_map(chip, bridge) ||
> mv88e6xxx_port_vlan_map(chip, port))
> dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
> @@ -2527,42 +2552,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
> mv88e6xxx_reg_unlock(chip);
> }
>
> -/* Treat the software bridge as a virtual single-port switch behind the
> - * CPU and map in the PVT. First dst->last_switch elements are taken by
> - * physical switches, so start from beyond that range.
> - */
> -static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> - unsigned int bridge_num)
> -{
> - u8 dev = bridge_num + ds->dst->last_switch;
> - struct mv88e6xxx_chip *chip = ds->priv;
> - int err;
> -
> - mv88e6xxx_reg_lock(chip);
> - err = mv88e6xxx_pvt_map(chip, dev, 0);
> - mv88e6xxx_reg_unlock(chip);
> -
> - return err;
> -}
> -
> -static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> -{
> - return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> -}
> -
> -static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge)
> -{
> - int err;
> -
> - err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> - if (err) {
> - dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
> - ERR_PTR(err));
> - }
> -}
> -
> static int mv88e6xxx_software_reset(struct mv88e6xxx_chip *chip)
> {
> if (chip->info->ops->reset)
> @@ -6282,8 +6271,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
> .crosschip_lag_change = mv88e6xxx_crosschip_lag_change,
> .crosschip_lag_join = mv88e6xxx_crosschip_lag_join,
> .crosschip_lag_leave = mv88e6xxx_crosschip_lag_leave,
> - .port_bridge_tx_fwd_offload = mv88e6xxx_bridge_tx_fwd_offload,
> - .port_bridge_tx_fwd_unoffload = mv88e6xxx_bridge_tx_fwd_unoffload,
> };
>
> static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index 21622c60faab..cefde41ce8d6 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -2077,12 +2077,27 @@ static int sja1105_bridge_join(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge,
> bool *tx_fwd_offload)
> {
> - return sja1105_bridge_member(ds, port, bridge, true);
> + int rc;
> +
> + rc = sja1105_bridge_member(ds, port, bridge, true);
> + if (rc)
> + return rc;
> +
> + rc = dsa_tag_8021q_bridge_tx_fwd_offload(ds, port, bridge);
> + if (rc) {
> + sja1105_bridge_member(ds, port, bridge, false);
> + return rc;
> + }
> +
> + *tx_fwd_offload = true;
> +
> + return 0;
> }
>
> static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge)
> {
> + dsa_tag_8021q_bridge_tx_fwd_unoffload(ds, port, bridge);
> sja1105_bridge_member(ds, port, bridge, false);
> }
>
> @@ -3231,8 +3246,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
> .tag_8021q_vlan_add = sja1105_dsa_8021q_vlan_add,
> .tag_8021q_vlan_del = sja1105_dsa_8021q_vlan_del,
> .port_prechangeupper = sja1105_prechangeupper,
> - .port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
> - .port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
> };
>
> static const struct of_device_id sja1105_dt_ids[];
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 584b3f9462a0..bdf308a5c55e 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -222,6 +222,7 @@ struct dsa_mall_tc_entry {
> struct dsa_bridge {
> struct net_device *dev;
> unsigned int num;
> + bool tx_fwd_offload;
> refcount_t refcount;
> };
>
> @@ -826,12 +827,6 @@ struct dsa_switch_ops {
> bool *tx_fwd_offload);
> void (*port_bridge_leave)(struct dsa_switch *ds, int port,
> struct dsa_bridge bridge);
> - /* Called right after .port_bridge_join() */
> - int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge);
> - /* Called right before .port_bridge_leave() */
> - void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
> - struct dsa_bridge bridge);
> void (*port_stp_state_set)(struct dsa_switch *ds, int port,
> u8 state);
> void (*port_fast_age)(struct dsa_switch *ds, int port);
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index b6c8a1a9ec18..fbe8c054d2b8 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -270,37 +270,6 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
> */
> }
>
> -static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> - struct dsa_bridge bridge)
> -{
> - struct dsa_switch *ds = dp->ds;
> -
> - /* No bridge TX forwarding offload => do nothing */
> - if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
> - return;
> -
> - /* Notify the chips only once the offload has been deactivated, so
> - * that they can update their configuration accordingly.
> - */
> - ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
> -}
> -
> -static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
> - struct dsa_bridge bridge)
> -{
> - struct dsa_switch *ds = dp->ds;
> - int err;
> -
> - /* FDB isolation is required for TX forwarding offload */
> - if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
> - return false;
> -
> - /* Notify the driver */
> - err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
> -
> - return err ? false : true;
> -}
> -
> static int dsa_port_bridge_create(struct dsa_port *dp,
> struct net_device *br,
> struct netlink_ext_ack *extack)
> @@ -361,7 +330,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> };
> struct net_device *dev = dp->slave;
> struct net_device *brport_dev;
> - bool tx_fwd_offload;
> int err;
>
> /* Here the interface is already bridged. Reflect the current
> @@ -378,12 +346,13 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> if (err)
> goto out_rollback;
>
> - tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
> + /* Drivers which support bridge TX forwarding should set this */
> + dp->bridge->tx_fwd_offload = info.tx_fwd_offload;
>
> err = switchdev_bridge_port_offload(brport_dev, dev, dp,
> &dsa_slave_switchdev_notifier,
> &dsa_slave_switchdev_blocking_notifier,
> - tx_fwd_offload, extack);
> + dp->bridge->tx_fwd_offload, extack);
> if (err)
> goto out_rollback_unbridge;
>
> @@ -434,8 +403,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
> */
> dsa_port_bridge_destroy(dp, br);
>
> - dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
> -
> err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
> if (err)
> dev_err(dp->ds->dev,
>
^ permalink raw reply [flat|nested] 12+ messages in thread