From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org
Cc: "Florian Fainelli" <f.fainelli@gmail.com>,
"Andrew Lunn" <andrew@lunn.ch>,
"Vivien Didelot" <vivien.didelot@gmail.com>,
"Tobias Waldekranz" <tobias@waldekranz.com>,
"DENG Qingfang" <dqfext@gmail.com>,
"Alvin Šipraga" <alsi@bang-olufsen.dk>,
"Kurt Kanzenbach" <kurt@linutronix.de>,
"Hauke Mehrtens" <hauke@hauke-m.de>,
"Woojung Huh" <woojung.huh@microchip.com>,
UNGLinuxDriver@microchip.com,
"Sean Wang" <sean.wang@mediatek.com>,
"Landen Chao" <Landen.Chao@mediatek.com>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Claudiu Manoil" <claudiu.manoil@nxp.com>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
"Linus Walleij" <linus.walleij@linaro.org>,
"George McCollister" <george.mccollister@gmail.com>
Subject: [PATCH v3 net-next 01/12] net: dsa: make dp->bridge_num one-based
Date: Mon, 6 Dec 2021 18:57:47 +0200 [thread overview]
Message-ID: <20211206165758.1553882-2-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20211206165758.1553882-1-vladimir.oltean@nxp.com>
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->v3: 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
next prev parent reply other threads:[~2021-12-06 17:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-06 16:57 [PATCH v3 net-next 00/12] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-12-06 16:57 ` Vladimir Oltean [this message]
2021-12-06 16:57 ` [PATCH v3 net-next 02/12] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 03/12] net: dsa: mt7530: iterate using dsa_switch_for_each_user_port in bridging ops Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 04/12] net: dsa: mv88e6xxx: iterate using dsa_switch_for_each_user_port in mv88e6xxx_port_check_hw_vlan Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 05/12] net: dsa: mv88e6xxx: compute port vlan membership based on dp->bridge_dev comparison Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 06/12] net: dsa: hide dp->bridge_dev and dp->bridge_num in the core behind helpers Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 07/12] net: dsa: hide dp->bridge_dev and dp->bridge_num in drivers " Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 08/12] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 09/12] net: dsa: export bridging offload helpers to drivers Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 10/12] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 11/12] net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join Vladimir Oltean
2021-12-06 16:57 ` [PATCH v3 net-next 12/12] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
2021-12-08 23:00 ` [PATCH v3 net-next 00/12] Rework DSA bridge TX forwarding offload API patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211206165758.1553882-2-vladimir.oltean@nxp.com \
--to=vladimir.oltean@nxp.com \
--cc=Landen.Chao@mediatek.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alsi@bang-olufsen.dk \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=dqfext@gmail.com \
--cc=f.fainelli@gmail.com \
--cc=george.mccollister@gmail.com \
--cc=hauke@hauke-m.de \
--cc=kurt@linutronix.de \
--cc=linus.walleij@linaro.org \
--cc=matthias.bgg@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=sean.wang@mediatek.com \
--cc=tobias@waldekranz.com \
--cc=vivien.didelot@gmail.com \
--cc=woojung.huh@microchip.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.