Skip to contentSkip to conversation

mlxsw: Cleanups in router code

PATCH Applied Petr Machata 3 participants 18 messages Jun 9, 2023 – Jun 12, 2023

Subsystems (3)
Networking Network drivers Mellanox Ethernet switch drivers
lore
Share & tools
lore

Petr Machata

author maintainer
#0 lore raw

cover mlxsw: Cleanups in router code

This patchset moves some router-related code from spectrum.c to spectrum_router.c where it should be. It also simplifies handlers of netevent notifications.

- Patch #1 caches router pointer in a dedicated variable. This obviates the need to access the same as mlxsw_sp->router, making lines shorter, and permitting a future patch to add code that fits within 80 character limit.

- Patch #2 moves IP / IPv6 validation notifier blocks from spectrum.c to spectrum_router, where the handlers are anyway.

- In patch #3, pass router pointer to scheduler of deferred work directly, instead of having it deduce it on its own.

- This makes the router pointer available in the handler function mlxsw_sp_router_netevent_event(), so in patch #4, use it directly, instead of finding it through mlxsw_sp_port.

- In patch #5, extend mlxsw_sp_router_schedule_work() so that the NETEVENT_NEIGH_UPDATE handler can use it directly instead of inlining equivalent code.

- In patches #6 and #7, add helpers for two common operations involving a backing netdev of a RIF. This makes it unnecessary for the function mlxsw_sp_rif_dev() to be visible outside of the router module, so in patch #8, hide it.

Petr Machata (8):
mlxsw: spectrum_router: mlxsw_sp_router_fini(): Extract a helper
variable
mlxsw: spectrum_router: Move here inetaddr validator notifiers
mlxsw: spectrum_router: Pass router to mlxsw_sp_router_schedule_work()
directly
mlxsw: spectrum_router: Use the available router pointer for netevent
handling
mlxsw: spectrum_router: Reuse work neighbor initialization in work
scheduler
mlxsw: Convert RIF-has-netdevice queries to a dedicated helper
mlxsw: Convert does-RIF-have-this-netdev queries to a dedicated helper
mlxsw: spectrum_router: Privatize mlxsw_sp_rif_dev()

 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  18 +--
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |   4 -
 .../ethernet/mellanox/mlxsw/spectrum_dpipe.c  |   2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_mr.c |  19 ++-
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 123 +++++++++++-------
 .../ethernet/mellanox/mlxsw/spectrum_router.h |   6 +-
 6 files changed, 90 insertions(+), 82 deletions(-)
Signature · 1 line
2.40.1
author maintainer
#1 lore raw

PATCH 1/8 mlxsw: spectrum_router: mlxsw_sp_router_fini(): Extract a helper variable

Make mlxsw_sp_router_fini() more similar to the _init() function (and more concise) by extracting the router' handle to a named variable and using that throughout. The availability of a dedicated router' variable will come in handy in following patches.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 25 ++++++++++---------
 1 file changed, 13 insertions(+), 12 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +13 -12
index 7304e8a29cf9..583d0b717e25 100644
@@ -10664,15 +10664,16 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
 
 void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 {
+	struct mlxsw_sp_router *router = mlxsw_sp->router;
+
 	unregister_netdevice_notifier_net(mlxsw_sp_net(mlxsw_sp),
-					  &mlxsw_sp->router->netdevice_nb);
-	unregister_fib_notifier(mlxsw_sp_net(mlxsw_sp),
-				&mlxsw_sp->router->fib_nb);
+					  &router->netdevice_nb);
+	unregister_fib_notifier(mlxsw_sp_net(mlxsw_sp), &router->fib_nb);
 	unregister_nexthop_notifier(mlxsw_sp_net(mlxsw_sp),
-				    &mlxsw_sp->router->nexthop_nb);
-	unregister_netevent_notifier(&mlxsw_sp->router->netevent_nb);
-	unregister_inet6addr_notifier(&mlxsw_sp->router->inet6addr_nb);
-	unregister_inetaddr_notifier(&mlxsw_sp->router->inetaddr_nb);
+				    &router->nexthop_nb);
+	unregister_netevent_notifier(&router->netevent_nb);
+	unregister_inet6addr_notifier(&router->inet6addr_nb);
+	unregister_inetaddr_notifier(&router->inetaddr_nb);
 	mlxsw_core_flush_owq();
 	mlxsw_sp_mp_hash_fini(mlxsw_sp);
 	mlxsw_sp_neigh_fini(mlxsw_sp);

@@ -10680,12 +10681,12 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 	mlxsw_sp_vrs_fini(mlxsw_sp);
 	mlxsw_sp_mr_fini(mlxsw_sp);
 	mlxsw_sp_lpm_fini(mlxsw_sp);
-	rhashtable_destroy(&mlxsw_sp->router->nexthop_group_ht);
-	rhashtable_destroy(&mlxsw_sp->router->nexthop_ht);
+	rhashtable_destroy(&router->nexthop_group_ht);
+	rhashtable_destroy(&router->nexthop_ht);
 	mlxsw_sp_ipips_fini(mlxsw_sp);
 	mlxsw_sp_rifs_fini(mlxsw_sp);
 	__mlxsw_sp_router_fini(mlxsw_sp);
-	cancel_delayed_work_sync(&mlxsw_sp->router->nh_grp_activity_dw);
-	mutex_destroy(&mlxsw_sp->router->lock);
-	kfree(mlxsw_sp->router);
+	cancel_delayed_work_sync(&router->nh_grp_activity_dw);
+	mutex_destroy(&router->lock);
+	kfree(router);
 }
Signature · 1 line
2.40.1
Reviewed-by on v1 1/8
#2 lore raw

Re: [PATCH net-next 1/8] mlxsw: spectrum_router: mlxsw_sp_router_fini(): Extract a helper variable

Quoted 7 lines · Petr Machata

Make mlxsw_sp_router_fini() more similar to the _init() function (and more concise) by extracting the router' handle to a named variable and using that throughout. The availability of a dedicated router' variable will come in handy in following patches.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#3 lore raw

PATCH 2/8 mlxsw: spectrum_router: Move here inetaddr validator notifiers

In reply to Petr Machata

The validation logic is already in the router code. Move there the notifier blocks themselves as well.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 18 +------------
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  4 ---
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 26 ++++++++++++++++---
 .../ethernet/mellanox/mlxsw/spectrum_router.h |  2 ++
 4 files changed, 25 insertions(+), 25 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum.c +1 -17
index 02a327744a61..4609b13bda02 100644
@@ -5139,14 +5139,6 @@ static int mlxsw_sp_netdevice_event(struct notifier_block *nb,
 	return notifier_from_errno(err);
 }
 
-static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
-	.notifier_call = mlxsw_sp_inetaddr_valid_event,
-};
-
-static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
-	.notifier_call = mlxsw_sp_inet6addr_valid_event,
-};
-
 static const struct pci_device_id mlxsw_sp1_pci_id_table[] = {
 	{PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_SPECTRUM), 0},
 	{0, },

@@ -5191,12 +5183,9 @@ static int __init mlxsw_sp_module_init(void)
 {
 	int err;
 
-	register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
-	register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
-
 	err = mlxsw_core_driver_register(&mlxsw_sp1_driver);
 	if (err)
-		goto err_sp1_core_driver_register;
+		return err;
 
 	err = mlxsw_core_driver_register(&mlxsw_sp2_driver);
 	if (err)

@@ -5242,9 +5231,6 @@ static int __init mlxsw_sp_module_init(void)
 	mlxsw_core_driver_unregister(&mlxsw_sp2_driver);
 err_sp2_core_driver_register:
 	mlxsw_core_driver_unregister(&mlxsw_sp1_driver);
-err_sp1_core_driver_register:
-	unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
-	unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
 	return err;
 }
 

@@ -5258,8 +5244,6 @@ static void __exit mlxsw_sp_module_exit(void)
 	mlxsw_core_driver_unregister(&mlxsw_sp3_driver);
 	mlxsw_core_driver_unregister(&mlxsw_sp2_driver);
 	mlxsw_core_driver_unregister(&mlxsw_sp1_driver);
-	unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
-	unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
 }
 
 module_init(mlxsw_sp_module_init);
drivers/net/ethernet/mellanox/mlxsw/spectrum.h +0 -4
index 4c22f8004514..0b57c8d0cce0 100644
@@ -755,10 +755,6 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
 void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_rif_macvlan_del(struct mlxsw_sp *mlxsw_sp,
 			      const struct net_device *macvlan_dev);
-int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
-				  unsigned long event, void *ptr);
-int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
-				   unsigned long event, void *ptr);
 int
 mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
 			       struct net_device *l3_dev,
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +22 -4
index 583d0b717e25..edfc42230285 100644
@@ -8879,8 +8879,8 @@ static int mlxsw_sp_inetaddr_event(struct notifier_block *nb,
 	return notifier_from_errno(err);
 }
 
-int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
-				  unsigned long event, void *ptr)
+static int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+					 unsigned long event, void *ptr)
 {
 	struct in_validator_info *ivi = (struct in_validator_info *) ptr;
 	struct net_device *dev = ivi->ivi_dev->dev;

@@ -8962,8 +8962,8 @@ static int mlxsw_sp_inet6addr_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
-				   unsigned long event, void *ptr)
+static int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+					  unsigned long event, void *ptr)
 {
 	struct in6_validator_info *i6vi = (struct in6_validator_info *) ptr;
 	struct net_device *dev = i6vi->i6vi_dev->dev;

@@ -10510,6 +10510,7 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
 			 struct netlink_ext_ack *extack)
 {
 	struct mlxsw_sp_router *router;
+	struct notifier_block *nb;
 	int err;
 
 	router = kzalloc(sizeof(*mlxsw_sp->router), GFP_KERNEL);

@@ -10588,6 +10589,17 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		goto err_register_inet6addr_notifier;
 
+	router->inetaddr_valid_nb.notifier_call = mlxsw_sp_inetaddr_valid_event;
+	err = register_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
+	if (err)
+		goto err_register_inetaddr_valid_notifier;
+
+	nb = &router->inet6addr_valid_nb;
+	nb->notifier_call = mlxsw_sp_inet6addr_valid_event;
+	err = register_inet6addr_validator_notifier(nb);
+	if (err)
+		goto err_register_inet6addr_valid_notifier;
+
 	mlxsw_sp->router->netevent_nb.notifier_call =
 		mlxsw_sp_router_netevent_event;
 	err = register_netevent_notifier(&mlxsw_sp->router->netevent_nb);

@@ -10627,6 +10639,10 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
 err_register_nexthop_notifier:
 	unregister_netevent_notifier(&mlxsw_sp->router->netevent_nb);
 err_register_netevent_notifier:
+	unregister_inet6addr_validator_notifier(&router->inet6addr_valid_nb);
+err_register_inet6addr_valid_notifier:
+	unregister_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
+err_register_inetaddr_valid_notifier:
 	unregister_inet6addr_notifier(&router->inet6addr_nb);
 err_register_inet6addr_notifier:
 	unregister_inetaddr_notifier(&router->inetaddr_nb);

@@ -10672,6 +10688,8 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 	unregister_nexthop_notifier(mlxsw_sp_net(mlxsw_sp),
 				    &router->nexthop_nb);
 	unregister_netevent_notifier(&router->netevent_nb);
+	unregister_inet6addr_validator_notifier(&router->inet6addr_valid_nb);
+	unregister_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
 	unregister_inet6addr_notifier(&router->inet6addr_nb);
 	unregister_inetaddr_notifier(&router->inetaddr_nb);
 	mlxsw_core_flush_owq();
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h +2 -0
index 37d6e4c80e6a..229d38c514b9 100644
@@ -52,6 +52,8 @@ struct mlxsw_sp_router {
 	struct notifier_block inetaddr_nb;
 	struct notifier_block inet6addr_nb;
 	struct notifier_block netdevice_nb;
+	struct notifier_block inetaddr_valid_nb;
+	struct notifier_block inet6addr_valid_nb;
 	const struct mlxsw_sp_rif_ops **rif_ops_arr;
 	const struct mlxsw_sp_ipip_ops **ipip_ops_arr;
 	struct mlxsw_sp_router_nve_decap nve_decap_config;
Signature · 1 line
2.40.1
author maintainer
#5 lore raw

PATCH 3/8 mlxsw: spectrum_router: Pass router to mlxsw_sp_router_schedule_work() directly

In reply to Petr Machata

Instead of passing a notifier block and deducing the router pointer from that in the helper, do that in the caller, and pass the result. In the following patches, the pointer will also be made useful in the caller.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +6 -5
index edfc42230285..7b1877c116ed 100644
@@ -2748,13 +2748,11 @@ static void mlxsw_sp_router_update_priority_work(struct work_struct *work)
 }
 
 static int mlxsw_sp_router_schedule_work(struct net *net,
-					 struct notifier_block *nb,
+					 struct mlxsw_sp_router *router,
 					 void (*cb)(struct work_struct *))
 {
 	struct mlxsw_sp_netevent_work *net_work;
-	struct mlxsw_sp_router *router;
 
-	router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
 	if (!net_eq(net, mlxsw_sp_net(router->mlxsw_sp)))
 		return NOTIFY_DONE;
 

@@ -2773,11 +2771,14 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 {
 	struct mlxsw_sp_netevent_work *net_work;
 	struct mlxsw_sp_port *mlxsw_sp_port;
+	struct mlxsw_sp_router *router;
 	struct mlxsw_sp *mlxsw_sp;
 	unsigned long interval;
 	struct neigh_parms *p;
 	struct neighbour *n;
 
+	router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
+
 	switch (event) {
 	case NETEVENT_DELAY_PROBE_TIME_UPDATE:
 		p = ptr;

@@ -2830,11 +2831,11 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		break;
 	case NETEVENT_IPV4_MPATH_HASH_UPDATE:
 	case NETEVENT_IPV6_MPATH_HASH_UPDATE:
-		return mlxsw_sp_router_schedule_work(ptr, nb,
+		return mlxsw_sp_router_schedule_work(ptr, router,
 				mlxsw_sp_router_mp_hash_event_work);
 
 	case NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE:
-		return mlxsw_sp_router_schedule_work(ptr, nb,
+		return mlxsw_sp_router_schedule_work(ptr, router,
 				mlxsw_sp_router_update_priority_work);
 	}
Signature · 1 line
2.40.1
Reviewed-by on v1 3/8
#6 lore raw

Re: [PATCH net-next 3/8] mlxsw: spectrum_router: Pass router to mlxsw_sp_router_schedule_work() directly

Quoted 6 lines · Petr Machata

Instead of passing a notifier block and deducing the router pointer from that in the helper, do that in the caller, and pass the result. In the following patches, the pointer will also be made useful in the caller.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#7 lore raw

PATCH 4/8 mlxsw: spectrum_router: Use the available router pointer for netevent handling

In reply to Petr Machata

This code handles NETEVENT_DELAY_PROBE_TIME_UPDATE, which is invoked every time the delay_probe_time changes. mlxsw router currently only maintains one timer, so the last delay_probe_time set wins.

Currently, mlxsw uses mlxsw_sp_port_lower_dev_hold() to find a reference to the router. This is no longer necessary. But as a side effect, this makes sure that only updates to "interesting netdevices" (ones that have a physical netdevice lower) are projected.

Retain that side effect by calling mlxsw_sp_port_dev_lower_find_rcu() and punting if there is none. Then just proceed using the router pointer that's already at hand in the helper.

Note that previously, the code took and put a reference of the netdevice. Because the mlxsw_sp pointer is now obtained from the notifier block, the port pointer (non-) NULL-ness is all that's relevant, and the reference does not need to be taken anymore.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +12 -7
index 7b1877c116ed..9d34fc846b93 100644
@@ -2766,13 +2766,22 @@ static int mlxsw_sp_router_schedule_work(struct net *net,
 	return NOTIFY_DONE;
 }
 
+static bool mlxsw_sp_dev_lower_is_port(struct net_device *dev)
+{
+	struct mlxsw_sp_port *mlxsw_sp_port;
+
+	rcu_read_lock();
+	mlxsw_sp_port = mlxsw_sp_port_dev_lower_find_rcu(dev);
+	rcu_read_unlock();
+	return !!mlxsw_sp_port;
+}
+
 static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 					  unsigned long event, void *ptr)
 {
 	struct mlxsw_sp_netevent_work *net_work;
 	struct mlxsw_sp_port *mlxsw_sp_port;
 	struct mlxsw_sp_router *router;
-	struct mlxsw_sp *mlxsw_sp;
 	unsigned long interval;
 	struct neigh_parms *p;
 	struct neighbour *n;

@@ -2791,15 +2800,11 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		/* We are in atomic context and can't take RTNL mutex,
 		 * so use RCU variant to walk the device chain.
 		 */
-		mlxsw_sp_port = mlxsw_sp_port_lower_dev_hold(p->dev);
-		if (!mlxsw_sp_port)
+		if (!mlxsw_sp_dev_lower_is_port(p->dev))
 			return NOTIFY_DONE;
 
-		mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 		interval = jiffies_to_msecs(NEIGH_VAR(p, DELAY_PROBE_TIME));
-		mlxsw_sp->router->neighs_update.interval = interval;
-
-		mlxsw_sp_port_dev_put(mlxsw_sp_port);
+		router->neighs_update.interval = interval;
 		break;
 	case NETEVENT_NEIGH_UPDATE:
 		n = ptr;
Signature · 1 line
2.40.1
Reviewed-by on v1 4/8
#8 lore raw

Re: [PATCH net-next 4/8] mlxsw: spectrum_router: Use the available router pointer for netevent handling

Quoted 13 lines · Petr Machata

This code handles NETEVENT_DELAY_PROBE_TIME_UPDATE, which is invoked every time the delay_probe_time changes. mlxsw router currently only maintains one timer, so the last delay_probe_time set wins.

Currently, mlxsw uses mlxsw_sp_port_lower_dev_hold() to find a reference to the router. This is no longer necessary. But as a side effect, this makes sure that only updates to "interesting netdevices" (ones that have a physical netdevice lower) are projected.

Retain that side effect by calling mlxsw_sp_port_dev_lower_find_rcu() and punting if there is none. Then just proceed using the router pointer that's already at hand in the helper.

Quoted 7 lines

Note that previously, the code took and put a reference of the netdevice. Because the mlxsw_sp pointer is now obtained from the notifier block, the port pointer (non-) NULL-ness is all that's relevant, and the reference does not need to be taken anymore.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#9 lore raw

PATCH 5/8 mlxsw: spectrum_router: Reuse work neighbor initialization in work scheduler

In reply to Petr Machata

After the struct mlxsw_sp_netevent_work.n field initialization is moved here, the body of code that handles NETEVENT_NEIGH_UPDATE is almost identical to the one in the helper function. Therefore defer to the helper instead of inlining the equivalent.

Note that previously, the code took and put a reference of the netdevice. The new code defers to mlxsw_sp_dev_lower_is_port() to obviate the need for taking the reference.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 29 +++++++------------
 1 file changed, 10 insertions(+), 19 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +10 -19
index 9d34fc846b93..a0598aa4cb5d 100644
@@ -2749,6 +2749,7 @@ static void mlxsw_sp_router_update_priority_work(struct work_struct *work)
 
 static int mlxsw_sp_router_schedule_work(struct net *net,
 					 struct mlxsw_sp_router *router,
+					 struct neighbour *n,
 					 void (*cb)(struct work_struct *))
 {
 	struct mlxsw_sp_netevent_work *net_work;

@@ -2762,6 +2763,7 @@ static int mlxsw_sp_router_schedule_work(struct net *net,
 
 	INIT_WORK(&net_work->work, cb);
 	net_work->mlxsw_sp = router->mlxsw_sp;
+	net_work->n = n;
 	mlxsw_core_schedule_work(&net_work->work);
 	return NOTIFY_DONE;
 }

@@ -2779,12 +2781,11 @@ static bool mlxsw_sp_dev_lower_is_port(struct net_device *dev)
 static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 					  unsigned long event, void *ptr)
 {
-	struct mlxsw_sp_netevent_work *net_work;
-	struct mlxsw_sp_port *mlxsw_sp_port;
 	struct mlxsw_sp_router *router;
 	unsigned long interval;
 	struct neigh_parms *p;
 	struct neighbour *n;
+	struct net *net;
 
 	router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
 

@@ -2808,39 +2809,29 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		break;
 	case NETEVENT_NEIGH_UPDATE:
 		n = ptr;
+		net = neigh_parms_net(n->parms);
 
 		if (n->tbl->family != AF_INET && n->tbl->family != AF_INET6)
 			return NOTIFY_DONE;
 
-		mlxsw_sp_port = mlxsw_sp_port_lower_dev_hold(n->dev);
-		if (!mlxsw_sp_port)
+		if (!mlxsw_sp_dev_lower_is_port(n->dev))
 			return NOTIFY_DONE;
 
-		net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
-		if (!net_work) {
-			mlxsw_sp_port_dev_put(mlxsw_sp_port);
-			return NOTIFY_BAD;
-		}
-
-		INIT_WORK(&net_work->work, mlxsw_sp_router_neigh_event_work);
-		net_work->mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
-		net_work->n = n;
-
 		/* Take a reference to ensure the neighbour won't be
 		 * destructed until we drop the reference in delayed
 		 * work.
 		 */
 		neigh_clone(n);
-		mlxsw_core_schedule_work(&net_work->work);
-		mlxsw_sp_port_dev_put(mlxsw_sp_port);
-		break;
+		return mlxsw_sp_router_schedule_work(net, router, n,
+				mlxsw_sp_router_neigh_event_work);
+
 	case NETEVENT_IPV4_MPATH_HASH_UPDATE:
 	case NETEVENT_IPV6_MPATH_HASH_UPDATE:
-		return mlxsw_sp_router_schedule_work(ptr, router,
+		return mlxsw_sp_router_schedule_work(ptr, router, NULL,
 				mlxsw_sp_router_mp_hash_event_work);
 
 	case NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE:
-		return mlxsw_sp_router_schedule_work(ptr, router,
+		return mlxsw_sp_router_schedule_work(ptr, router, NULL,
 				mlxsw_sp_router_update_priority_work);
 	}
Signature · 1 line
2.40.1
Reviewed-by on v1 5/8
#10 lore raw

Re: [PATCH net-next 5/8] mlxsw: spectrum_router: Reuse work neighbor initialization in work scheduler

Quoted 11 lines · Petr Machata

After the struct mlxsw_sp_netevent_work.n field initialization is moved here, the body of code that handles NETEVENT_NEIGH_UPDATE is almost identical to the one in the helper function. Therefore defer to the helper instead of inlining the equivalent.

Note that previously, the code took and put a reference of the netdevice. The new code defers to mlxsw_sp_dev_lower_is_port() to obviate the need for taking the reference.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#11 lore raw

PATCH 6/8 mlxsw: Convert RIF-has-netdevice queries to a dedicated helper

In reply to Petr Machata

In a number of places, a netdevice underlying a RIF is obtained only to check if it a NULL pointer. In order to clean up the interface between the router and the other modules, add a new helper to specifically answer this question, and convert the relevant uses to this new interface.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c  | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c     | 6 +++---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 5 +++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h | 1 +
 4 files changed, 10 insertions(+), 4 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c +1 -1
index 5416093c0e35..c8a356accdf8 100644
@@ -221,7 +221,7 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
 	for (; i < rif_count; i++) {
 		struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
 
-		if (!rif || !mlxsw_sp_rif_dev(rif))
+		if (!rif || !mlxsw_sp_rif_has_dev(rif))
 			continue;
 		err = mlxsw_sp_erif_entry_get(mlxsw_sp, &entry, rif,
 					      counters_enabled);
drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c +3 -3
index 1f6bc0c7e91d..b0f03009c130 100644
@@ -720,7 +720,7 @@ int mlxsw_sp_mr_rif_add(struct mlxsw_sp_mr_table *mr_table,
 	const struct net_device *rif_dev = mlxsw_sp_rif_dev(rif);
 	struct mlxsw_sp_mr_vif *mr_vif;
 
-	if (!rif_dev)
+	if (!mlxsw_sp_rif_has_dev(rif))
 		return 0;
 
 	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif_dev);

@@ -736,7 +736,7 @@ void mlxsw_sp_mr_rif_del(struct mlxsw_sp_mr_table *mr_table,
 	const struct net_device *rif_dev = mlxsw_sp_rif_dev(rif);
 	struct mlxsw_sp_mr_vif *mr_vif;
 
-	if (!rif_dev)
+	if (!mlxsw_sp_rif_has_dev(rif))
 		return;
 
 	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif_dev);

@@ -754,7 +754,7 @@ void mlxsw_sp_mr_rif_mtu_update(struct mlxsw_sp_mr_table *mr_table,
 	struct mlxsw_sp_mr *mr = mlxsw_sp->mr;
 	struct mlxsw_sp_mr_vif *mr_vif;
 
-	if (!rif_dev)
+	if (!mlxsw_sp_rif_has_dev(rif))
 		return;
 
 	/* Search for a VIF that use that RIF */
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +5 -0
index a0598aa4cb5d..3259aede09ec 100644
@@ -8080,6 +8080,11 @@ const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
 	return rif->dev;
 }
 
+bool mlxsw_sp_rif_has_dev(const struct mlxsw_sp_rif *rif)
+{
+	return !!mlxsw_sp_rif_dev(rif);
+}
+
 static void mlxsw_sp_rif_push_l3_stats(struct mlxsw_sp_rif *rif)
 {
 	struct rtnl_hw_stats64 stats = {};
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h +1 -0
index 229d38c514b9..a6a8cf0b4500 100644
@@ -94,6 +94,7 @@ u16 mlxsw_sp_ipip_lb_ul_rif_id(const struct mlxsw_sp_rif_ipip_lb *lb_rif);
 u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev);
 int mlxsw_sp_rif_dev_ifindex(const struct mlxsw_sp_rif *rif);
 const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif);
+bool mlxsw_sp_rif_has_dev(const struct mlxsw_sp_rif *rif);
 int mlxsw_sp_rif_counter_value_get(struct mlxsw_sp *mlxsw_sp,
 				   struct mlxsw_sp_rif *rif,
 				   enum mlxsw_sp_rif_counter_dir dir,
Signature · 1 line
2.40.1
Reviewed-by on v1 6/8
#12 lore raw

Re: [PATCH net-next 6/8] mlxsw: Convert RIF-has-netdevice queries to a dedicated helper

Quoted 7 lines · Petr Machata

In a number of places, a netdevice underlying a RIF is obtained only to check if it a NULL pointer. In order to clean up the interface between the router and the other modules, add a new helper to specifically answer this question, and convert the relevant uses to this new interface.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#13 lore raw

PATCH 7/8 mlxsw: Convert does-RIF-have-this-netdev queries to a dedicated helper

In reply to Petr Machata

In a number of places, a netdevice underlying a RIF is obtained only to compare it to another pointer. In order to clean up the interface between the router and the other modules, add a new helper to specifically answer this question, and convert the relevant uses to this new interface.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c   | 13 +++++--------
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c   |  8 +++++++-
 .../net/ethernet/mellanox/mlxsw/spectrum_router.h   |  2 ++
 3 files changed, 14 insertions(+), 9 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c +5 -8
index b0f03009c130..69cd689dbc83 100644
@@ -704,12 +704,12 @@ void mlxsw_sp_mr_vif_del(struct mlxsw_sp_mr_table *mr_table, vifi_t vif_index)
 
 static struct mlxsw_sp_mr_vif *
 mlxsw_sp_mr_dev_vif_lookup(struct mlxsw_sp_mr_table *mr_table,
-			   const struct net_device *dev)
+			   const struct mlxsw_sp_rif *rif)
 {
 	vifi_t vif_index;
 
 	for (vif_index = 0; vif_index < MAXVIFS; vif_index++)
-		if (mr_table->vifs[vif_index].dev == dev)
+		if (mlxsw_sp_rif_dev_is(rif, mr_table->vifs[vif_index].dev))
 			return &mr_table->vifs[vif_index];
 	return NULL;
 }

@@ -717,13 +717,12 @@ mlxsw_sp_mr_dev_vif_lookup(struct mlxsw_sp_mr_table *mr_table,
 int mlxsw_sp_mr_rif_add(struct mlxsw_sp_mr_table *mr_table,
 			const struct mlxsw_sp_rif *rif)
 {
-	const struct net_device *rif_dev = mlxsw_sp_rif_dev(rif);
 	struct mlxsw_sp_mr_vif *mr_vif;
 
 	if (!mlxsw_sp_rif_has_dev(rif))
 		return 0;
 
-	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif_dev);
+	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif);
 	if (!mr_vif)
 		return 0;
 	return mlxsw_sp_mr_vif_resolve(mr_table, mr_vif->dev, mr_vif,

@@ -733,13 +732,12 @@ int mlxsw_sp_mr_rif_add(struct mlxsw_sp_mr_table *mr_table,
 void mlxsw_sp_mr_rif_del(struct mlxsw_sp_mr_table *mr_table,
 			 const struct mlxsw_sp_rif *rif)
 {
-	const struct net_device *rif_dev = mlxsw_sp_rif_dev(rif);
 	struct mlxsw_sp_mr_vif *mr_vif;
 
 	if (!mlxsw_sp_rif_has_dev(rif))
 		return;
 
-	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif_dev);
+	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif);
 	if (!mr_vif)
 		return;
 	mlxsw_sp_mr_vif_unresolve(mr_table, mr_vif->dev, mr_vif);

@@ -748,7 +746,6 @@ void mlxsw_sp_mr_rif_del(struct mlxsw_sp_mr_table *mr_table,
 void mlxsw_sp_mr_rif_mtu_update(struct mlxsw_sp_mr_table *mr_table,
 				const struct mlxsw_sp_rif *rif, int mtu)
 {
-	const struct net_device *rif_dev = mlxsw_sp_rif_dev(rif);
 	struct mlxsw_sp *mlxsw_sp = mr_table->mlxsw_sp;
 	struct mlxsw_sp_mr_route_vif_entry *rve;
 	struct mlxsw_sp_mr *mr = mlxsw_sp->mr;

@@ -758,7 +755,7 @@ void mlxsw_sp_mr_rif_mtu_update(struct mlxsw_sp_mr_table *mr_table,
 		return;
 
 	/* Search for a VIF that use that RIF */
-	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif_dev);
+	mr_vif = mlxsw_sp_mr_dev_vif_lookup(mr_table, rif);
 	if (!mr_vif)
 		return;
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +7 -1
index 3259aede09ec..537730b22c7a 100644
@@ -7705,7 +7705,7 @@ mlxsw_sp_rif_find_by_dev(const struct mlxsw_sp *mlxsw_sp,
 
 	for (i = 0; i < max_rifs; i++)
 		if (mlxsw_sp->router->rifs[i] &&
-		    mlxsw_sp->router->rifs[i]->dev == dev)
+		    mlxsw_sp_rif_dev_is(mlxsw_sp->router->rifs[i], dev))
 			return mlxsw_sp->router->rifs[i];
 
 	return NULL;

@@ -8085,6 +8085,12 @@ bool mlxsw_sp_rif_has_dev(const struct mlxsw_sp_rif *rif)
 	return !!mlxsw_sp_rif_dev(rif);
 }
 
+bool mlxsw_sp_rif_dev_is(const struct mlxsw_sp_rif *rif,
+			 const struct net_device *dev)
+{
+	return mlxsw_sp_rif_dev(rif) == dev;
+}
+
 static void mlxsw_sp_rif_push_l3_stats(struct mlxsw_sp_rif *rif)
 {
 	struct rtnl_hw_stats64 stats = {};
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h +2 -0
index a6a8cf0b4500..b941e781e476 100644
@@ -95,6 +95,8 @@ u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev);
 int mlxsw_sp_rif_dev_ifindex(const struct mlxsw_sp_rif *rif);
 const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif);
 bool mlxsw_sp_rif_has_dev(const struct mlxsw_sp_rif *rif);
+bool mlxsw_sp_rif_dev_is(const struct mlxsw_sp_rif *rif,
+			 const struct net_device *dev);
 int mlxsw_sp_rif_counter_value_get(struct mlxsw_sp *mlxsw_sp,
 				   struct mlxsw_sp_rif *rif,
 				   enum mlxsw_sp_rif_counter_dir dir,
Signature · 1 line
2.40.1
Reviewed-by on v1 7/8
#14 lore raw

Re: [PATCH net-next 7/8] mlxsw: Convert does-RIF-have-this-netdev queries to a dedicated helper

Quoted 7 lines · Petr Machata

In a number of places, a netdevice underlying a RIF is obtained only to compare it to another pointer. In order to clean up the interface between the router and the other modules, add a new helper to specifically answer this question, and convert the relevant uses to this new interface.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#15 lore raw

PATCH 8/8 mlxsw: spectrum_router: Privatize mlxsw_sp_rif_dev()

In reply to Petr Machata

Now that the external users of mlxsw_sp_rif_dev() have been converted in the preceding patches, make the function static.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Amit Cohen <[email protected]>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +1 -1
index 537730b22c7a..f9328e8410f5 100644
@@ -8075,7 +8075,7 @@ int mlxsw_sp_rif_dev_ifindex(const struct mlxsw_sp_rif *rif)
 	return rif->dev->ifindex;
 }
 
-const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
+static const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
 {
 	return rif->dev;
 }
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h +0 -1
index b941e781e476..5ff443f27136 100644
@@ -93,7 +93,6 @@ u16 mlxsw_sp_ipip_lb_ul_vr_id(const struct mlxsw_sp_rif_ipip_lb *rif);
 u16 mlxsw_sp_ipip_lb_ul_rif_id(const struct mlxsw_sp_rif_ipip_lb *lb_rif);
 u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev);
 int mlxsw_sp_rif_dev_ifindex(const struct mlxsw_sp_rif *rif);
-const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif);
 bool mlxsw_sp_rif_has_dev(const struct mlxsw_sp_rif *rif);
 bool mlxsw_sp_rif_dev_is(const struct mlxsw_sp_rif *rif,
 			 const struct net_device *dev);
Signature · 1 line
2.40.1
CI Applied
#17 lore raw
In reply to Petr Machata
This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Quoted 10 lines · On Fri, 9 Jun 2023 19:32:05 +0200 you wrote:

This patchset moves some router-related code from spectrum.c to spectrum_router.c where it should be. It also simplifies handlers of netevent notifications.

- Patch #1 caches router pointer in a dedicated variable. This obviates the need to access the same as mlxsw_sp->router, making lines shorter, and permitting a future patch to add code that fits within 80 character limit.

[...]

Here is the summary with links:
- [net-next,1/8] mlxsw: spectrum_router: mlxsw_sp_router_fini(): Extract a helper variable

    https://git.kernel.org/netdev/net-next/c/50f6c3d57e9a
  - [net-next,2/8] mlxsw: spectrum_router: Move here inetaddr validator notifiers
    https://git.kernel.org/netdev/net-next/c/41b2bd208e8a
  - [net-next,3/8] mlxsw: spectrum_router: Pass router to mlxsw_sp_router_schedule_work() directly
    https://git.kernel.org/netdev/net-next/c/48dde35ea157
  - [net-next,4/8] mlxsw: spectrum_router: Use the available router pointer for netevent handling
    https://git.kernel.org/netdev/net-next/c/14304e70634c
  - [net-next,5/8] mlxsw: spectrum_router: Reuse work neighbor initialization in work scheduler
    https://git.kernel.org/netdev/net-next/c/151b89f6025a
  - [net-next,6/8] mlxsw: Convert RIF-has-netdevice queries to a dedicated helper
    https://git.kernel.org/netdev/net-next/c/0255f74845c0
  - [net-next,7/8] mlxsw: Convert does-RIF-have-this-netdev queries to a dedicated helper
    https://git.kernel.org/netdev/net-next/c/5374a50f2eb6
  - [net-next,8/8] mlxsw: spectrum_router: Privatize mlxsw_sp_rif_dev()
    https://git.kernel.org/netdev/net-next/c/df95ae66cc0a

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html