All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup
@ 2022-01-05 13:18 Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 1/3] net: dsa: fix incorrect function pointer check for MRP ring roles Vladimir Oltean
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Vladimir Oltean @ 2022-01-05 13:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Horatiu Vultur, George McCollister

This series deletes the no-op cross-chip notifier support for MRP and
HSR, features which were introduced relatively recently and did not get
full review at the time. The new code is functionally equivalent, but
simpler.

Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: George McCollister <george.mccollister@gmail.com>

Vladimir Oltean (3):
  net: dsa: fix incorrect function pointer check for MRP ring roles
  net: dsa: remove cross-chip support for MRP
  net: dsa: remove cross-chip support for HSR

 net/dsa/dsa_priv.h | 27 --------------
 net/dsa/port.c     | 73 +++++++++++++++++---------------------
 net/dsa/switch.c   | 88 ----------------------------------------------
 3 files changed, 33 insertions(+), 155 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2 net-next 1/3] net: dsa: fix incorrect function pointer check for MRP ring roles
  2022-01-05 13:18 [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup Vladimir Oltean
@ 2022-01-05 13:18 ` Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 2/3] net: dsa: remove cross-chip support for MRP Vladimir Oltean
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2022-01-05 13:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Horatiu Vultur

The cross-chip notifier boilerplate code meant to check the presence of
ds->ops->port_mrp_add_ring_role before calling it, but checked
ds->ops->port_mrp_add instead, before calling
ds->ops->port_mrp_add_ring_role.

Therefore, a driver which implements one operation but not the other
would trigger a NULL pointer dereference.

There isn't any such driver in DSA yet, so there is no reason to
backport the change. Issue found through code inspection.

Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Fixes: c595c4330da0 ("net: dsa: add MRP support")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new

 net/dsa/switch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 393f2d8a860a..260d8e7d6e5a 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -729,7 +729,7 @@ static int
 dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
 			     struct dsa_notifier_mrp_ring_role_info *info)
 {
-	if (!ds->ops->port_mrp_add)
+	if (!ds->ops->port_mrp_add_ring_role)
 		return -EOPNOTSUPP;
 
 	if (ds->index == info->sw_index)
@@ -743,7 +743,7 @@ static int
 dsa_switch_mrp_del_ring_role(struct dsa_switch *ds,
 			     struct dsa_notifier_mrp_ring_role_info *info)
 {
-	if (!ds->ops->port_mrp_del)
+	if (!ds->ops->port_mrp_del_ring_role)
 		return -EOPNOTSUPP;
 
 	if (ds->index == info->sw_index)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v2 net-next 2/3] net: dsa: remove cross-chip support for MRP
  2022-01-05 13:18 [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 1/3] net: dsa: fix incorrect function pointer check for MRP ring roles Vladimir Oltean
@ 2022-01-05 13:18 ` Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 3/3] net: dsa: remove cross-chip support for HSR Vladimir Oltean
  2022-01-05 15:20 ` [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2022-01-05 13:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Horatiu Vultur

The cross-chip notifiers for MRP are bypass operations, meaning that
even though all switches in a tree are notified, only the switch
specified in the info structure is targeted.

We can eliminate the unnecessary complexity by deleting the cross-chip
notifier logic and calling the ds->ops straight from port.c.

Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: delete leftover definitions of struct dsa_notifier_mrp_info and
        struct dsa_notifier_mrp_ring_role_info.

 net/dsa/dsa_priv.h | 18 -------------
 net/dsa/port.c     | 44 +++++++++++++++----------------
 net/dsa/switch.c   | 64 ----------------------------------------------
 3 files changed, 20 insertions(+), 106 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b5ae21f172a8..c593d56c94b3 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -40,10 +40,6 @@ enum {
 	DSA_NOTIFIER_TAG_PROTO,
 	DSA_NOTIFIER_TAG_PROTO_CONNECT,
 	DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
-	DSA_NOTIFIER_MRP_ADD,
-	DSA_NOTIFIER_MRP_DEL,
-	DSA_NOTIFIER_MRP_ADD_RING_ROLE,
-	DSA_NOTIFIER_MRP_DEL_RING_ROLE,
 	DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
 	DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
 };
@@ -107,20 +103,6 @@ struct dsa_notifier_tag_proto_info {
 	const struct dsa_device_ops *tag_ops;
 };
 
-/* DSA_NOTIFIER_MRP_* */
-struct dsa_notifier_mrp_info {
-	const struct switchdev_obj_mrp *mrp;
-	int sw_index;
-	int port;
-};
-
-/* DSA_NOTIFIER_MRP_* */
-struct dsa_notifier_mrp_ring_role_info {
-	const struct switchdev_obj_ring_role_mrp *mrp;
-	int sw_index;
-	int port;
-};
-
 /* DSA_NOTIFIER_TAG_8021Q_VLAN_* */
 struct dsa_notifier_tag_8021q_vlan_info {
 	int tree_index;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 05677e016982..05be4577b044 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -907,49 +907,45 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 int dsa_port_mrp_add(const struct dsa_port *dp,
 		     const struct switchdev_obj_mrp *mrp)
 {
-	struct dsa_notifier_mrp_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.mrp = mrp,
-	};
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->port_mrp_add)
+		return -EOPNOTSUPP;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD, &info);
+	return ds->ops->port_mrp_add(ds, dp->index, mrp);
 }
 
 int dsa_port_mrp_del(const struct dsa_port *dp,
 		     const struct switchdev_obj_mrp *mrp)
 {
-	struct dsa_notifier_mrp_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.mrp = mrp,
-	};
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->port_mrp_del)
+		return -EOPNOTSUPP;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL, &info);
+	return ds->ops->port_mrp_del(ds, dp->index, mrp);
 }
 
 int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
 			       const struct switchdev_obj_ring_role_mrp *mrp)
 {
-	struct dsa_notifier_mrp_ring_role_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.mrp = mrp,
-	};
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->port_mrp_add_ring_role)
+		return -EOPNOTSUPP;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD_RING_ROLE, &info);
+	return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp);
 }
 
 int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
 			       const struct switchdev_obj_ring_role_mrp *mrp)
 {
-	struct dsa_notifier_mrp_ring_role_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.mrp = mrp,
-	};
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->port_mrp_del_ring_role)
+		return -EOPNOTSUPP;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL_RING_ROLE, &info);
+	return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
 }
 
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 260d8e7d6e5a..a164ec02b4e9 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -701,58 +701,6 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
 	return 0;
 }
 
-static int dsa_switch_mrp_add(struct dsa_switch *ds,
-			      struct dsa_notifier_mrp_info *info)
-{
-	if (!ds->ops->port_mrp_add)
-		return -EOPNOTSUPP;
-
-	if (ds->index == info->sw_index)
-		return ds->ops->port_mrp_add(ds, info->port, info->mrp);
-
-	return 0;
-}
-
-static int dsa_switch_mrp_del(struct dsa_switch *ds,
-			      struct dsa_notifier_mrp_info *info)
-{
-	if (!ds->ops->port_mrp_del)
-		return -EOPNOTSUPP;
-
-	if (ds->index == info->sw_index)
-		return ds->ops->port_mrp_del(ds, info->port, info->mrp);
-
-	return 0;
-}
-
-static int
-dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
-			     struct dsa_notifier_mrp_ring_role_info *info)
-{
-	if (!ds->ops->port_mrp_add_ring_role)
-		return -EOPNOTSUPP;
-
-	if (ds->index == info->sw_index)
-		return ds->ops->port_mrp_add_ring_role(ds, info->port,
-						       info->mrp);
-
-	return 0;
-}
-
-static int
-dsa_switch_mrp_del_ring_role(struct dsa_switch *ds,
-			     struct dsa_notifier_mrp_ring_role_info *info)
-{
-	if (!ds->ops->port_mrp_del_ring_role)
-		return -EOPNOTSUPP;
-
-	if (ds->index == info->sw_index)
-		return ds->ops->port_mrp_del_ring_role(ds, info->port,
-						       info->mrp);
-
-	return 0;
-}
-
 static int dsa_switch_event(struct notifier_block *nb,
 			    unsigned long event, void *info)
 {
@@ -826,18 +774,6 @@ static int dsa_switch_event(struct notifier_block *nb,
 	case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
 		err = dsa_switch_disconnect_tag_proto(ds, info);
 		break;
-	case DSA_NOTIFIER_MRP_ADD:
-		err = dsa_switch_mrp_add(ds, info);
-		break;
-	case DSA_NOTIFIER_MRP_DEL:
-		err = dsa_switch_mrp_del(ds, info);
-		break;
-	case DSA_NOTIFIER_MRP_ADD_RING_ROLE:
-		err = dsa_switch_mrp_add_ring_role(ds, info);
-		break;
-	case DSA_NOTIFIER_MRP_DEL_RING_ROLE:
-		err = dsa_switch_mrp_del_ring_role(ds, info);
-		break;
 	case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
 		err = dsa_switch_tag_8021q_vlan_add(ds, info);
 		break;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v2 net-next 3/3] net: dsa: remove cross-chip support for HSR
  2022-01-05 13:18 [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 1/3] net: dsa: fix incorrect function pointer check for MRP ring roles Vladimir Oltean
  2022-01-05 13:18 ` [PATCH v2 net-next 2/3] net: dsa: remove cross-chip support for MRP Vladimir Oltean
@ 2022-01-05 13:18 ` Vladimir Oltean
  2022-01-05 15:20 ` [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2022-01-05 13:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, George McCollister

The cross-chip notifiers for HSR are bypass operations, meaning that
even though all switches in a tree are notified, only the switch
specified in the info structure is targeted.

We can eliminate the unnecessary complexity by deleting the cross-chip
notifier logic and calling the ds->ops straight from port.c.

Cc: George McCollister <george.mccollister@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: George McCollister <george.mccollister@gmail.com>
---
v1->v2:
- delete leftover definition of struct dsa_notifier_hsr_info
- guard against absence of ds->ops->port_hsr_join and
  ds->ops->port_hsr_leave

 net/dsa/dsa_priv.h |  9 ---------
 net/dsa/port.c     | 29 +++++++++++++----------------
 net/dsa/switch.c   | 24 ------------------------
 3 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index c593d56c94b3..760306f0012f 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -25,8 +25,6 @@ enum {
 	DSA_NOTIFIER_FDB_DEL,
 	DSA_NOTIFIER_HOST_FDB_ADD,
 	DSA_NOTIFIER_HOST_FDB_DEL,
-	DSA_NOTIFIER_HSR_JOIN,
-	DSA_NOTIFIER_HSR_LEAVE,
 	DSA_NOTIFIER_LAG_CHANGE,
 	DSA_NOTIFIER_LAG_JOIN,
 	DSA_NOTIFIER_LAG_LEAVE,
@@ -125,13 +123,6 @@ struct dsa_switchdev_event_work {
 	bool host_addr;
 };
 
-/* DSA_NOTIFIER_HSR_* */
-struct dsa_notifier_hsr_info {
-	struct net_device *hsr;
-	int sw_index;
-	int port;
-};
-
 struct dsa_slave_priv {
 	/* Copy of CPU port xmit for faster access in slave transmit hot path */
 	struct sk_buff *	(*xmit)(struct sk_buff *skb,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 05be4577b044..bd78192e0e47 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1317,16 +1317,15 @@ EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count);
 
 int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
 {
-	struct dsa_notifier_hsr_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.hsr = hsr,
-	};
+	struct dsa_switch *ds = dp->ds;
 	int err;
 
+	if (!ds->ops->port_hsr_join)
+		return -EOPNOTSUPP;
+
 	dp->hsr_dev = hsr;
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info);
+	err = ds->ops->port_hsr_join(ds, dp->index, hsr);
 	if (err)
 		dp->hsr_dev = NULL;
 
@@ -1335,20 +1334,18 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
 
 void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
 {
-	struct dsa_notifier_hsr_info info = {
-		.sw_index = dp->ds->index,
-		.port = dp->index,
-		.hsr = hsr,
-	};
+	struct dsa_switch *ds = dp->ds;
 	int err;
 
 	dp->hsr_dev = NULL;
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info);
-	if (err)
-		dev_err(dp->ds->dev,
-			"port %d failed to notify DSA_NOTIFIER_HSR_LEAVE: %pe\n",
-			dp->index, ERR_PTR(err));
+	if (ds->ops->port_hsr_leave) {
+		err = ds->ops->port_hsr_leave(ds, dp->index, hsr);
+		if (err)
+			dev_err(dp->ds->dev,
+				"port %d failed to leave HSR %s: %pe\n",
+				dp->index, hsr->name, ERR_PTR(err));
+	}
 }
 
 int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index a164ec02b4e9..e3c7d2627a61 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -437,24 +437,6 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
 	return dsa_port_do_fdb_del(dp, info->addr, info->vid);
 }
 
-static int dsa_switch_hsr_join(struct dsa_switch *ds,
-			       struct dsa_notifier_hsr_info *info)
-{
-	if (ds->index == info->sw_index && ds->ops->port_hsr_join)
-		return ds->ops->port_hsr_join(ds, info->port, info->hsr);
-
-	return -EOPNOTSUPP;
-}
-
-static int dsa_switch_hsr_leave(struct dsa_switch *ds,
-				struct dsa_notifier_hsr_info *info)
-{
-	if (ds->index == info->sw_index && ds->ops->port_hsr_leave)
-		return ds->ops->port_hsr_leave(ds, info->port, info->hsr);
-
-	return -EOPNOTSUPP;
-}
-
 static int dsa_switch_lag_change(struct dsa_switch *ds,
 				 struct dsa_notifier_lag_info *info)
 {
@@ -729,12 +711,6 @@ static int dsa_switch_event(struct notifier_block *nb,
 	case DSA_NOTIFIER_HOST_FDB_DEL:
 		err = dsa_switch_host_fdb_del(ds, info);
 		break;
-	case DSA_NOTIFIER_HSR_JOIN:
-		err = dsa_switch_hsr_join(ds, info);
-		break;
-	case DSA_NOTIFIER_HSR_LEAVE:
-		err = dsa_switch_hsr_leave(ds, info);
-		break;
 	case DSA_NOTIFIER_LAG_CHANGE:
 		err = dsa_switch_lag_change(ds, info);
 		break;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup
  2022-01-05 13:18 [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup Vladimir Oltean
                   ` (2 preceding siblings ...)
  2022-01-05 13:18 ` [PATCH v2 net-next 3/3] net: dsa: remove cross-chip support for HSR Vladimir Oltean
@ 2022-01-05 15:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-01-05 15:20 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, davem, kuba, andrew, vivien.didelot, f.fainelli,
	horatiu.vultur, george.mccollister

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed,  5 Jan 2022 15:18:10 +0200 you wrote:
> This series deletes the no-op cross-chip notifier support for MRP and
> HSR, features which were introduced relatively recently and did not get
> full review at the time. The new code is functionally equivalent, but
> simpler.
> 
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Cc: George McCollister <george.mccollister@gmail.com>
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/3] net: dsa: fix incorrect function pointer check for MRP ring roles
    https://git.kernel.org/netdev/net-next/c/ff91e1b68490
  - [v2,net-next,2/3] net: dsa: remove cross-chip support for MRP
    https://git.kernel.org/netdev/net-next/c/cad69019f2f8
  - [v2,net-next,3/3] net: dsa: remove cross-chip support for HSR
    https://git.kernel.org/netdev/net-next/c/a68dc7b938fb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-05 15:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-05 13:18 [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup Vladimir Oltean
2022-01-05 13:18 ` [PATCH v2 net-next 1/3] net: dsa: fix incorrect function pointer check for MRP ring roles Vladimir Oltean
2022-01-05 13:18 ` [PATCH v2 net-next 2/3] net: dsa: remove cross-chip support for MRP Vladimir Oltean
2022-01-05 13:18 ` [PATCH v2 net-next 3/3] net: dsa: remove cross-chip support for HSR Vladimir Oltean
2022-01-05 15:20 ` [PATCH v2 net-next 0/3] DSA cross-chip notifier cleanup patchwork-bot+netdevbpf

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.