* [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path
@ 2020-02-21 17:54 Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device Ido Schimmel
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
This patch set removes RTNL from the route insertion path in mlxsw in
order to reduce the control plane latency: the time it takes to push
routes from user space to the kernel and mlxsw.
Up until now mlxsw did not have a lock to protect its shared router data
structures and instead relied on RTNL. While this was simple and worked,
it resulted in large control plane latencies as RTNL was heavily
contended - by both the task receiving the netlink messages from user
space and the mlxsw workqueue that programs the routes to the device.
By removing RTNL and introducing a new router mutex, this patch set
reduces the control plane latency by ~80%. A single mutex is added as
inside mlxsw there is not a lot of concurrency. In addition, a more
fine-grained locking scheme is much more error-prone.
Patches #1-#6 are preparations. They add needed locking in NVE and
multicast routing code instead of relying on RTNL
Patch #7 introduces the new mutex
Patches #8-#12 gradually take the lock in various entry points into the
routing code
Patch #13 removes RTNL in places where it is no longer required
Ido Schimmel (12):
mlxsw: spectrum_mr: Publish multicast route after writing it to the
device
mlxsw: spectrum_mr: Protect multicast table list with a lock
mlxsw: spectrum_mr: Protect multicast route list with a lock
mlxsw: spectrum_router: Expose router struct to internal users
mlxsw: spectrum_router: Store NVE decapsulation configuration in
router
mlxsw: spectrum_router: Introduce router lock
mlxsw: spectrum_router: Take router lock from inside routing code
mlxsw: spectrum_dpipe: Take router lock from dpipe code
mlxsw: spectrum_router: Take router lock from netdev listener
mlxsw: spectrum_router: Take router lock from inetaddr listeners
mlxsw: spectrum_router: Take router lock from exported helpers
mlxsw: spectrum: Remove RTNL where possible
.../net/ethernet/mellanox/mlxsw/spectrum.h | 5 +-
.../ethernet/mellanox/mlxsw/spectrum_dpipe.c | 35 +--
.../net/ethernet/mellanox/mlxsw/spectrum_mr.c | 52 +++-
.../ethernet/mellanox/mlxsw/spectrum_nve.c | 21 --
.../ethernet/mellanox/mlxsw/spectrum_router.c | 267 +++++++++++-------
.../ethernet/mellanox/mlxsw/spectrum_router.h | 43 +++
6 files changed, 267 insertions(+), 156 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 02/12] mlxsw: spectrum_mr: Protect multicast table list with a lock Ido Schimmel
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The driver periodically traverses the linked list of multicast routes
and updates the kernel about packets and bytes statistics from each
multicast route. These statistics are read from a counter associated
with the route when it is written to the device.
Currently, multicast routes are published via this linked list before
they are associated with a counter. Despite that, it is not possible for
the driver to access an invalid counter because the delayed work that
reads the statistics and multicast route addition / deletion are
mutually exclusive using RTNL.
In order to be able to remove RTNL, the list needs to be protected by a
dedicated lock, but any route published via the list must have an
associated counter, otherwise the driver will access an invalid counter.
Solve this by re-ordering the operations during multicast route addition
so that the route is only added to the linked list after it was written
to the device. Similarly, during deletion the route is first removed
from the linked list before its deletion from the device.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_mr.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
index 423eedebcd22..e40437d5aa76 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
@@ -371,10 +371,10 @@ static void __mlxsw_sp_mr_route_del(struct mlxsw_sp_mr_table *mr_table,
struct mlxsw_sp_mr_route *mr_route)
{
mlxsw_sp_mr_mfc_offload_set(mr_route, false);
- mlxsw_sp_mr_route_erase(mr_table, mr_route);
rhashtable_remove_fast(&mr_table->route_ht, &mr_route->ht_node,
mlxsw_sp_mr_route_ht_params);
list_del(&mr_route->node);
+ mlxsw_sp_mr_route_erase(mr_table, mr_route);
mlxsw_sp_mr_route_destroy(mr_table, mr_route);
}
@@ -415,6 +415,11 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
goto err_duplicate_route;
}
+ /* Write the route to the hardware */
+ err = mlxsw_sp_mr_route_write(mr_table, mr_route, replace);
+ if (err)
+ goto err_mr_route_write;
+
/* Put it in the table data-structures */
list_add_tail(&mr_route->node, &mr_table->route_list);
err = rhashtable_insert_fast(&mr_table->route_ht,
@@ -423,11 +428,6 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
if (err)
goto err_rhashtable_insert;
- /* Write the route to the hardware */
- err = mlxsw_sp_mr_route_write(mr_table, mr_route, replace);
- if (err)
- goto err_mr_route_write;
-
/* Destroy the original route */
if (replace) {
rhashtable_remove_fast(&mr_table->route_ht,
@@ -440,11 +440,10 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
mlxsw_sp_mr_mfc_offload_update(mr_route);
return 0;
-err_mr_route_write:
- rhashtable_remove_fast(&mr_table->route_ht, &mr_route->ht_node,
- mlxsw_sp_mr_route_ht_params);
err_rhashtable_insert:
list_del(&mr_route->node);
+ mlxsw_sp_mr_route_erase(mr_table, mr_route);
+err_mr_route_write:
err_no_orig_route:
err_duplicate_route:
mlxsw_sp_mr_route_destroy(mr_table, mr_route);
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 02/12] mlxsw: spectrum_mr: Protect multicast table list with a lock
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 03/12] mlxsw: spectrum_mr: Protect multicast route " Ido Schimmel
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The multicast table list is traversed from a delayed work that
periodically updates the kernel about packets and bytes statistics from
each multicast route.
The list is currently protected by RTNL, but subsequent patches will
remove the driver's dependence on this contended lock.
In order to be able to remove dependence on RTNL in the next patch,
guard this list with a dedicated mutex.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
index e40437d5aa76..0d64d8c4038b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
/* Copyright (c) 2017-2018 Mellanox Technologies. All rights reserved */
+#include <linux/mutex.h>
#include <linux/rhashtable.h>
#include <net/ipv6.h>
@@ -12,6 +13,7 @@ struct mlxsw_sp_mr {
void *catchall_route_priv;
struct delayed_work stats_update_dw;
struct list_head table_list;
+ struct mutex table_list_lock; /* Protects table_list */
#define MLXSW_SP_MR_ROUTES_COUNTER_UPDATE_INTERVAL 5000 /* ms */
unsigned long priv[0];
/* priv has to be always the last item */
@@ -926,7 +928,9 @@ struct mlxsw_sp_mr_table *mlxsw_sp_mr_table_create(struct mlxsw_sp *mlxsw_sp,
&catchall_route_params);
if (err)
goto err_ops_route_create;
+ mutex_lock(&mr->table_list_lock);
list_add_tail(&mr_table->node, &mr->table_list);
+ mutex_unlock(&mr->table_list_lock);
return mr_table;
err_ops_route_create:
@@ -942,7 +946,9 @@ void mlxsw_sp_mr_table_destroy(struct mlxsw_sp_mr_table *mr_table)
struct mlxsw_sp_mr *mr = mlxsw_sp->mr;
WARN_ON(!mlxsw_sp_mr_table_empty(mr_table));
+ mutex_lock(&mr->table_list_lock);
list_del(&mr_table->node);
+ mutex_unlock(&mr->table_list_lock);
mr->mr_ops->route_destroy(mlxsw_sp, mr->priv,
&mr_table->catchall_route_priv);
rhashtable_destroy(&mr_table->route_ht);
@@ -1000,10 +1006,12 @@ static void mlxsw_sp_mr_stats_update(struct work_struct *work)
unsigned long interval;
rtnl_lock();
+ mutex_lock(&mr->table_list_lock);
list_for_each_entry(mr_table, &mr->table_list, node)
list_for_each_entry(mr_route, &mr_table->route_list, node)
mlxsw_sp_mr_route_stats_update(mr_table->mlxsw_sp,
mr_route);
+ mutex_unlock(&mr->table_list_lock);
rtnl_unlock();
interval = msecs_to_jiffies(MLXSW_SP_MR_ROUTES_COUNTER_UPDATE_INTERVAL);
@@ -1023,6 +1031,7 @@ int mlxsw_sp_mr_init(struct mlxsw_sp *mlxsw_sp,
mr->mr_ops = mr_ops;
mlxsw_sp->mr = mr;
INIT_LIST_HEAD(&mr->table_list);
+ mutex_init(&mr->table_list_lock);
err = mr_ops->init(mlxsw_sp, mr->priv);
if (err)
@@ -1034,6 +1043,7 @@ int mlxsw_sp_mr_init(struct mlxsw_sp *mlxsw_sp,
mlxsw_core_schedule_dw(&mr->stats_update_dw, interval);
return 0;
err:
+ mutex_destroy(&mr->table_list_lock);
kfree(mr);
return err;
}
@@ -1044,5 +1054,6 @@ void mlxsw_sp_mr_fini(struct mlxsw_sp *mlxsw_sp)
cancel_delayed_work_sync(&mr->stats_update_dw);
mr->mr_ops->fini(mlxsw_sp, mr->priv);
+ mutex_destroy(&mr->table_list_lock);
kfree(mr);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 03/12] mlxsw: spectrum_mr: Protect multicast route list with a lock
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 02/12] mlxsw: spectrum_mr: Protect multicast table list with a lock Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 04/12] mlxsw: spectrum_router: Expose router struct to internal users Ido Schimmel
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Protect the per-table multicast route list with a lock and remove RTNL
from the delayed work that periodically updates the kernel about packets
and bytes statistics from each multicast route.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_mr.c | 24 +++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
index 0d64d8c4038b..085d9676e34b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
@@ -68,6 +68,7 @@ struct mlxsw_sp_mr_table {
u32 vr_id;
struct mlxsw_sp_mr_vif vifs[MAXVIFS];
struct list_head route_list;
+ struct mutex route_list_lock; /* Protects route_list */
struct rhashtable route_ht;
const struct mlxsw_sp_mr_table_ops *ops;
char catchall_route_priv[];
@@ -372,6 +373,8 @@ static void mlxsw_sp_mr_mfc_offload_update(struct mlxsw_sp_mr_route *mr_route)
static void __mlxsw_sp_mr_route_del(struct mlxsw_sp_mr_table *mr_table,
struct mlxsw_sp_mr_route *mr_route)
{
+ WARN_ON_ONCE(!mutex_is_locked(&mr_table->route_list_lock));
+
mlxsw_sp_mr_mfc_offload_set(mr_route, false);
rhashtable_remove_fast(&mr_table->route_ht, &mr_route->ht_node,
mlxsw_sp_mr_route_ht_params);
@@ -423,7 +426,9 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
goto err_mr_route_write;
/* Put it in the table data-structures */
+ mutex_lock(&mr_table->route_list_lock);
list_add_tail(&mr_route->node, &mr_table->route_list);
+ mutex_unlock(&mr_table->route_list_lock);
err = rhashtable_insert_fast(&mr_table->route_ht,
&mr_route->ht_node,
mlxsw_sp_mr_route_ht_params);
@@ -443,7 +448,9 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
return 0;
err_rhashtable_insert:
+ mutex_lock(&mr_table->route_list_lock);
list_del(&mr_route->node);
+ mutex_unlock(&mr_table->route_list_lock);
mlxsw_sp_mr_route_erase(mr_table, mr_route);
err_mr_route_write:
err_no_orig_route:
@@ -461,8 +468,11 @@ void mlxsw_sp_mr_route_del(struct mlxsw_sp_mr_table *mr_table,
mr_table->ops->key_create(mr_table, &key, mfc);
mr_route = rhashtable_lookup_fast(&mr_table->route_ht, &key,
mlxsw_sp_mr_route_ht_params);
- if (mr_route)
+ if (mr_route) {
+ mutex_lock(&mr_table->route_list_lock);
__mlxsw_sp_mr_route_del(mr_table, mr_route);
+ mutex_unlock(&mr_table->route_list_lock);
+ }
}
/* Should be called after the VIF struct is updated */
@@ -911,6 +921,7 @@ struct mlxsw_sp_mr_table *mlxsw_sp_mr_table_create(struct mlxsw_sp *mlxsw_sp,
mr_table->proto = proto;
mr_table->ops = &mlxsw_sp_mr_table_ops_arr[proto];
INIT_LIST_HEAD(&mr_table->route_list);
+ mutex_init(&mr_table->route_list_lock);
err = rhashtable_init(&mr_table->route_ht,
&mlxsw_sp_mr_route_ht_params);
@@ -936,6 +947,7 @@ struct mlxsw_sp_mr_table *mlxsw_sp_mr_table_create(struct mlxsw_sp *mlxsw_sp,
err_ops_route_create:
rhashtable_destroy(&mr_table->route_ht);
err_route_rhashtable_init:
+ mutex_destroy(&mr_table->route_list_lock);
kfree(mr_table);
return ERR_PTR(err);
}
@@ -952,6 +964,7 @@ void mlxsw_sp_mr_table_destroy(struct mlxsw_sp_mr_table *mr_table)
mr->mr_ops->route_destroy(mlxsw_sp, mr->priv,
&mr_table->catchall_route_priv);
rhashtable_destroy(&mr_table->route_ht);
+ mutex_destroy(&mr_table->route_list_lock);
kfree(mr_table);
}
@@ -960,8 +973,10 @@ void mlxsw_sp_mr_table_flush(struct mlxsw_sp_mr_table *mr_table)
struct mlxsw_sp_mr_route *mr_route, *tmp;
int i;
+ mutex_lock(&mr_table->route_list_lock);
list_for_each_entry_safe(mr_route, tmp, &mr_table->route_list, node)
__mlxsw_sp_mr_route_del(mr_table, mr_route);
+ mutex_unlock(&mr_table->route_list_lock);
for (i = 0; i < MAXVIFS; i++) {
mr_table->vifs[i].dev = NULL;
@@ -1005,14 +1020,15 @@ static void mlxsw_sp_mr_stats_update(struct work_struct *work)
struct mlxsw_sp_mr_route *mr_route;
unsigned long interval;
- rtnl_lock();
mutex_lock(&mr->table_list_lock);
- list_for_each_entry(mr_table, &mr->table_list, node)
+ list_for_each_entry(mr_table, &mr->table_list, node) {
+ mutex_lock(&mr_table->route_list_lock);
list_for_each_entry(mr_route, &mr_table->route_list, node)
mlxsw_sp_mr_route_stats_update(mr_table->mlxsw_sp,
mr_route);
+ mutex_unlock(&mr_table->route_list_lock);
+ }
mutex_unlock(&mr->table_list_lock);
- rtnl_unlock();
interval = msecs_to_jiffies(MLXSW_SP_MR_ROUTES_COUNTER_UPDATE_INTERVAL);
mlxsw_core_schedule_dw(&mr->stats_update_dw, interval);
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 04/12] mlxsw: spectrum_router: Expose router struct to internal users
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (2 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 03/12] mlxsw: spectrum_mr: Protect multicast route " Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 05/12] mlxsw: spectrum_router: Store NVE decapsulation configuration in router Ido Schimmel
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The dpipe code accesses internal router data structures and acquires
RTNL to protect against their changes. Subsequent patches will remove
reliance on RTNL and introduce a dedicated lock to protect router data
structures.
Publish the router struct to internal users such as the dpipe, so that
they could acquire it instead of RTNL.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_router.c | 33 -------------------
.../ethernet/mellanox/mlxsw/spectrum_router.h | 33 +++++++++++++++++++
2 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 634a9a949777..991095f66fc2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -48,39 +48,6 @@ struct mlxsw_sp_vr;
struct mlxsw_sp_lpm_tree;
struct mlxsw_sp_rif_ops;
-struct mlxsw_sp_router {
- struct mlxsw_sp *mlxsw_sp;
- struct mlxsw_sp_rif **rifs;
- struct mlxsw_sp_vr *vrs;
- struct rhashtable neigh_ht;
- struct rhashtable nexthop_group_ht;
- struct rhashtable nexthop_ht;
- struct list_head nexthop_list;
- struct {
- /* One tree for each protocol: IPv4 and IPv6 */
- struct mlxsw_sp_lpm_tree *proto_trees[2];
- struct mlxsw_sp_lpm_tree *trees;
- unsigned int tree_count;
- } lpm;
- struct {
- struct delayed_work dw;
- unsigned long interval; /* ms */
- } neighs_update;
- struct delayed_work nexthop_probe_dw;
-#define MLXSW_SP_UNRESOLVED_NH_PROBE_INTERVAL 5000 /* ms */
- struct list_head nexthop_neighs_list;
- struct list_head ipip_list;
- bool aborted;
- struct notifier_block fib_nb;
- struct notifier_block netevent_nb;
- struct notifier_block inetaddr_nb;
- struct notifier_block inet6addr_nb;
- const struct mlxsw_sp_rif_ops **rif_ops_arr;
- const struct mlxsw_sp_ipip_ops **ipip_ops_arr;
- u32 adj_discard_index;
- bool adj_discard_index_valid;
-};
-
struct mlxsw_sp_rif {
struct list_head nexthop_list;
struct list_head neigh_list;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index c9b94f435cdd..b2554727d8ee 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -7,6 +7,39 @@
#include "spectrum.h"
#include "reg.h"
+struct mlxsw_sp_router {
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif **rifs;
+ struct mlxsw_sp_vr *vrs;
+ struct rhashtable neigh_ht;
+ struct rhashtable nexthop_group_ht;
+ struct rhashtable nexthop_ht;
+ struct list_head nexthop_list;
+ struct {
+ /* One tree for each protocol: IPv4 and IPv6 */
+ struct mlxsw_sp_lpm_tree *proto_trees[2];
+ struct mlxsw_sp_lpm_tree *trees;
+ unsigned int tree_count;
+ } lpm;
+ struct {
+ struct delayed_work dw;
+ unsigned long interval; /* ms */
+ } neighs_update;
+ struct delayed_work nexthop_probe_dw;
+#define MLXSW_SP_UNRESOLVED_NH_PROBE_INTERVAL 5000 /* ms */
+ struct list_head nexthop_neighs_list;
+ struct list_head ipip_list;
+ bool aborted;
+ struct notifier_block fib_nb;
+ struct notifier_block netevent_nb;
+ struct notifier_block inetaddr_nb;
+ struct notifier_block inet6addr_nb;
+ const struct mlxsw_sp_rif_ops **rif_ops_arr;
+ const struct mlxsw_sp_ipip_ops **ipip_ops_arr;
+ u32 adj_discard_index;
+ bool adj_discard_index_valid;
+};
+
struct mlxsw_sp_rif_ipip_lb;
struct mlxsw_sp_rif_ipip_lb_config {
enum mlxsw_reg_ritr_loopback_ipip_type lb_ipipt;
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 05/12] mlxsw: spectrum_router: Store NVE decapsulation configuration in router
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (3 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 04/12] mlxsw: spectrum_router: Expose router struct to internal users Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 06/12] mlxsw: spectrum_router: Introduce router lock Ido Schimmel
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
When a host route is added, the driver checks if the route needs to be
promoted to perform NVE decapsulation based on the current NVE
configuration. If so, the index of the decapsulation entry is retrieved
and associated with the route.
Currently, this information is stored in the NVE module which the router
module consults. Since the information is protected under RTNL and since
route insertion happens with RTNL held, there is no problem to retrieve
the information from the NVE module.
However, this is going to change and route insertion will no longer
happen under RTNL. Instead, a dedicated lock will be introduced for the
router module.
Therefore, store this information in the router module and change the
router module to consult this copy.
The validity of the information is set / cleared whenever an NVE tunnel
is initialized / de-initialized. When this happens the NVE module calls
into the router module to promote / demote the relevant host route.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.h | 3 --
.../ethernet/mellanox/mlxsw/spectrum_nve.c | 21 ----------
.../ethernet/mellanox/mlxsw/spectrum_router.c | 42 ++++++++++++++++---
.../ethernet/mellanox/mlxsw/spectrum_router.h | 9 ++++
4 files changed, 46 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index bed86f4825a8..66cfe27e1f9b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -966,9 +966,6 @@ void mlxsw_sp_nve_flood_ip_del(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_fid *fid,
enum mlxsw_sp_l3proto proto,
union mlxsw_sp_l3addr *addr);
-u32 mlxsw_sp_nve_decap_tunnel_index_get(const struct mlxsw_sp *mlxsw_sp);
-bool mlxsw_sp_nve_ipv4_route_is_decap(const struct mlxsw_sp *mlxsw_sp,
- u32 tb_id, __be32 addr);
int mlxsw_sp_nve_fid_enable(struct mlxsw_sp *mlxsw_sp, struct mlxsw_sp_fid *fid,
struct mlxsw_sp_nve_params *params,
struct netlink_ext_ack *extack);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
index eced553fd4ef..54d3e7dcd303 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
@@ -713,27 +713,6 @@ static void mlxsw_sp_nve_flood_ip_flush(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_nve_mc_list_put(mlxsw_sp, mc_list);
}
-u32 mlxsw_sp_nve_decap_tunnel_index_get(const struct mlxsw_sp *mlxsw_sp)
-{
- WARN_ON(mlxsw_sp->nve->num_nve_tunnels == 0);
-
- return mlxsw_sp->nve->tunnel_index;
-}
-
-bool mlxsw_sp_nve_ipv4_route_is_decap(const struct mlxsw_sp *mlxsw_sp,
- u32 tb_id, __be32 addr)
-{
- struct mlxsw_sp_nve *nve = mlxsw_sp->nve;
- struct mlxsw_sp_nve_config *config = &nve->config;
-
- if (nve->num_nve_tunnels &&
- config->ul_proto == MLXSW_SP_L3_PROTO_IPV4 &&
- config->ul_sip.addr4 == addr && config->ul_tb_id == tb_id)
- return true;
-
- return false;
-}
-
static int mlxsw_sp_nve_tunnel_init(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_nve_config *config)
{
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 991095f66fc2..fe8c1f386651 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1833,9 +1833,19 @@ int mlxsw_sp_router_nve_promote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
u32 tunnel_index)
{
enum mlxsw_sp_fib_entry_type type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
+ struct mlxsw_sp_router *router = mlxsw_sp->router;
struct mlxsw_sp_fib_entry *fib_entry;
int err;
+ if (WARN_ON_ONCE(router->nve_decap_config.valid))
+ return -EINVAL;
+
+ router->nve_decap_config.ul_tb_id = ul_tb_id;
+ router->nve_decap_config.tunnel_index = tunnel_index;
+ router->nve_decap_config.ul_proto = ul_proto;
+ router->nve_decap_config.ul_sip = *ul_sip;
+ router->nve_decap_config.valid = true;
+
/* It is valid to create a tunnel with a local IP and only later
* assign this IP address to a local interface
*/
@@ -1865,8 +1875,14 @@ void mlxsw_sp_router_nve_demote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
const union mlxsw_sp_l3addr *ul_sip)
{
enum mlxsw_sp_fib_entry_type type = MLXSW_SP_FIB_ENTRY_TYPE_NVE_DECAP;
+ struct mlxsw_sp_router *router = mlxsw_sp->router;
struct mlxsw_sp_fib_entry *fib_entry;
+ if (WARN_ON_ONCE(!router->nve_decap_config.valid))
+ return;
+
+ router->nve_decap_config.valid = false;
+
fib_entry = mlxsw_sp_router_ip2me_fib_entry_find(mlxsw_sp, ul_tb_id,
ul_proto, ul_sip,
type);
@@ -1877,6 +1893,20 @@ void mlxsw_sp_router_nve_demote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
mlxsw_sp_fib_entry_update(mlxsw_sp, fib_entry);
}
+static bool mlxsw_sp_router_nve_is_decap(struct mlxsw_sp *mlxsw_sp,
+ u32 ul_tb_id,
+ enum mlxsw_sp_l3proto ul_proto,
+ const union mlxsw_sp_l3addr *ul_sip)
+{
+ struct mlxsw_sp_router *router = mlxsw_sp->router;
+
+ return router->nve_decap_config.valid &&
+ router->nve_decap_config.ul_tb_id == ul_tb_id &&
+ router->nve_decap_config.ul_proto == ul_proto &&
+ !memcmp(&router->nve_decap_config.ul_sip, ul_sip,
+ sizeof(*ul_sip));
+}
+
struct mlxsw_sp_neigh_key {
struct neighbour *n;
};
@@ -4466,6 +4496,7 @@ mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
{
struct net_device *dev = fib_info_nh(fen_info->fi, 0)->fib_nh_dev;
union mlxsw_sp_l3addr dip = { .addr4 = htonl(fen_info->dst) };
+ struct mlxsw_sp_router *router = mlxsw_sp->router;
u32 tb_id = mlxsw_sp_fix_tb_id(fen_info->tb_id);
struct mlxsw_sp_ipip_entry *ipip_entry;
struct fib_info *fi = fen_info->fi;
@@ -4480,12 +4511,13 @@ mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
fib_entry,
ipip_entry);
}
- if (mlxsw_sp_nve_ipv4_route_is_decap(mlxsw_sp, tb_id,
- dip.addr4)) {
- u32 t_index;
+ if (mlxsw_sp_router_nve_is_decap(mlxsw_sp, tb_id,
+ MLXSW_SP_L3_PROTO_IPV4,
+ &dip)) {
+ u32 tunnel_index;
- t_index = mlxsw_sp_nve_decap_tunnel_index_get(mlxsw_sp);
- fib_entry->decap.tunnel_index = t_index;
+ tunnel_index = router->nve_decap_config.tunnel_index;
+ fib_entry->decap.tunnel_index = tunnel_index;
fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_NVE_DECAP;
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index b2554727d8ee..3c99db1f434b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -7,6 +7,14 @@
#include "spectrum.h"
#include "reg.h"
+struct mlxsw_sp_router_nve_decap {
+ u32 ul_tb_id;
+ u32 tunnel_index;
+ enum mlxsw_sp_l3proto ul_proto;
+ union mlxsw_sp_l3addr ul_sip;
+ u8 valid:1;
+};
+
struct mlxsw_sp_router {
struct mlxsw_sp *mlxsw_sp;
struct mlxsw_sp_rif **rifs;
@@ -38,6 +46,7 @@ struct mlxsw_sp_router {
const struct mlxsw_sp_ipip_ops **ipip_ops_arr;
u32 adj_discard_index;
bool adj_discard_index_valid;
+ struct mlxsw_sp_router_nve_decap nve_decap_config;
};
struct mlxsw_sp_rif_ipip_lb;
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 06/12] mlxsw: spectrum_router: Introduce router lock
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (4 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 05/12] mlxsw: spectrum_router: Store NVE decapsulation configuration in router Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 07/12] mlxsw: spectrum_router: Take router lock from inside routing code Ido Schimmel
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Introduce a mutex to protect the internal structure of the routing code.
A single lock is added instead of a more fine-grained and complicated
locking scheme because there is not a lot of concurrency in the routing
code.
The main motivation is remove the dependence on RTNL lock, which is
currently used by both the process pushing routes to the kernel and the
workqueue pushing the routes to the underlying device.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 4 ++++
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index fe8c1f386651..e2ddb6164f6c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -17,6 +17,7 @@
#include <linux/refcount.h>
#include <linux/jhash.h>
#include <linux/net_namespace.h>
+#include <linux/mutex.h>
#include <net/netevent.h>
#include <net/neighbour.h>
#include <net/arp.h>
@@ -8065,6 +8066,7 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
router = kzalloc(sizeof(*mlxsw_sp->router), GFP_KERNEL);
if (!router)
return -ENOMEM;
+ mutex_init(&router->lock);
mlxsw_sp->router = router;
router->mlxsw_sp = mlxsw_sp;
@@ -8168,6 +8170,7 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp,
err_register_inet6addr_notifier:
unregister_inetaddr_notifier(&router->inetaddr_nb);
err_register_inetaddr_notifier:
+ mutex_destroy(&mlxsw_sp->router->lock);
kfree(mlxsw_sp->router);
return err;
}
@@ -8188,5 +8191,6 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
__mlxsw_sp_router_fini(mlxsw_sp);
unregister_inet6addr_notifier(&mlxsw_sp->router->inet6addr_nb);
unregister_inetaddr_notifier(&mlxsw_sp->router->inetaddr_nb);
+ mutex_destroy(&mlxsw_sp->router->lock);
kfree(mlxsw_sp->router);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index 3c99db1f434b..8418dc3ae967 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -47,6 +47,7 @@ struct mlxsw_sp_router {
u32 adj_discard_index;
bool adj_discard_index_valid;
struct mlxsw_sp_router_nve_decap nve_decap_config;
+ struct mutex lock; /* Protects shared router resources */
};
struct mlxsw_sp_rif_ipip_lb;
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 07/12] mlxsw: spectrum_router: Take router lock from inside routing code
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (5 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 06/12] mlxsw: spectrum_router: Introduce router lock Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 08/12] mlxsw: spectrum_dpipe: Take router lock from dpipe code Ido Schimmel
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
There are several work items in the routing code that currently rely on
RTNL lock to guard against concurrent changes. Have these work items
acquire the router lock in preparation for the removal for RTNL lock
from the routing code.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_router.c | 24 +++++++++++++------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index e2ddb6164f6c..601fa4a1abbb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2278,10 +2278,9 @@ __mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp,
int i, num_rec;
int err;
- /* Make sure the neighbour's netdev isn't removed in the
- * process.
- */
rtnl_lock();
+ /* Ensure the RIF we read from the device does not change mid-dump. */
+ mutex_lock(&mlxsw_sp->router->lock);
do {
mlxsw_reg_rauhtd_pack(rauhtd_pl, type);
err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(rauhtd),
@@ -2295,6 +2294,7 @@ __mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_router_neigh_rec_process(mlxsw_sp, rauhtd_pl,
i);
} while (mlxsw_sp_router_rauhtd_is_full(rauhtd_pl));
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return err;
@@ -2326,14 +2326,15 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp)
{
struct mlxsw_sp_neigh_entry *neigh_entry;
- /* Take RTNL mutex here to prevent lists from changes */
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
list_for_each_entry(neigh_entry, &mlxsw_sp->router->nexthop_neighs_list,
nexthop_neighs_list_node)
/* If this neigh have nexthops, make the kernel think this neigh
* is active regardless of the traffic.
*/
neigh_event_send(neigh_entry->key.n, NULL);
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
}
@@ -2374,14 +2375,14 @@ static void mlxsw_sp_router_probe_unresolved_nexthops(struct work_struct *work)
* the nexthop wouldn't get offloaded until the neighbor is resolved
* but it wouldn't get resolved ever in case traffic is flowing in HW
* using different nexthop.
- *
- * Take RTNL mutex here to prevent lists from changes.
*/
rtnl_lock();
+ mutex_lock(&router->lock);
list_for_each_entry(neigh_entry, &router->nexthop_neighs_list,
nexthop_neighs_list_node)
if (!neigh_entry->connected)
neigh_event_send(neigh_entry->key.n, NULL);
+ mutex_unlock(&router->lock);
rtnl_unlock();
mlxsw_core_schedule_dw(&router->nexthop_probe_dw,
@@ -2521,6 +2522,7 @@ static void mlxsw_sp_router_neigh_event_work(struct work_struct *work)
read_unlock_bh(&n->lock);
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
entry_connected = nud_state & NUD_VALID && !dead;
@@ -2542,6 +2544,7 @@ static void mlxsw_sp_router_neigh_event_work(struct work_struct *work)
mlxsw_sp_neigh_entry_destroy(mlxsw_sp, neigh_entry);
out:
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
neigh_release(n);
kfree(net_work);
@@ -5949,8 +5952,8 @@ static void mlxsw_sp_router_fib4_event_work(struct work_struct *work)
struct mlxsw_sp *mlxsw_sp = fib_work->mlxsw_sp;
int err;
- /* Protect internal structures from changes */
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
switch (fib_work->event) {
@@ -5972,6 +5975,7 @@ static void mlxsw_sp_router_fib4_event_work(struct work_struct *work)
fib_info_put(fib_work->fnh_info.fib_nh->nh_parent);
break;
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
kfree(fib_work);
}
@@ -5984,6 +5988,7 @@ static void mlxsw_sp_router_fib6_event_work(struct work_struct *work)
int err;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
switch (fib_work->event) {
@@ -6010,6 +6015,7 @@ static void mlxsw_sp_router_fib6_event_work(struct work_struct *work)
mlxsw_sp_router_fib6_work_fini(&fib_work->fib6_work);
break;
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
kfree(fib_work);
}
@@ -6023,6 +6029,7 @@ static void mlxsw_sp_router_fibmr_event_work(struct work_struct *work)
int err;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
switch (fib_work->event) {
case FIB_EVENT_ENTRY_REPLACE: /* fall through */
case FIB_EVENT_ENTRY_ADD:
@@ -6051,6 +6058,7 @@ static void mlxsw_sp_router_fibmr_event_work(struct work_struct *work)
dev_put(fib_work->ven_info.dev);
break;
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
kfree(fib_work);
}
@@ -7063,6 +7071,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
struct mlxsw_sp_rif *rif;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!mlxsw_sp_rif_should_config(rif, dev, event))
@@ -7070,6 +7079,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
__mlxsw_sp_inetaddr_event(mlxsw_sp, dev, event, NULL);
out:
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
dev_put(dev);
kfree(inet6addr_work);
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 08/12] mlxsw: spectrum_dpipe: Take router lock from dpipe code
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (6 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 07/12] mlxsw: spectrum_router: Take router lock from inside routing code Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 09/12] mlxsw: spectrum_router: Take router lock from netdev listener Ido Schimmel
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The dpipe code traverses internal router structures such as neighbours
and adjacency entries and dumps them to user space via netlink. Up until
now the routing code did not have its own locks and relied on RTNL lock
to serialize access. This is going to change with the introduction of
the router lock.
Take the router lock in the code paths where RTNL lock is currently
taken so that the latter could be removed by subsequent patches.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_dpipe.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 2dc0978428e6..63fc1f56ef00 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017-2018 Mellanox Technologies. All rights reserved */
#include <linux/kernel.h>
+#include <linux/mutex.h>
#include <net/devlink.h>
#include "spectrum.h"
@@ -211,6 +212,7 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
rif_count = MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS);
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
i = 0;
start_again:
err = devlink_dpipe_entry_ctx_prepare(dump_ctx);
@@ -241,6 +243,7 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
devlink_dpipe_entry_ctx_close(dump_ctx);
if (i != rif_count)
goto start_again;
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
devlink_dpipe_entry_clear(&entry);
@@ -248,6 +251,7 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
err_entry_append:
err_entry_get:
err_ctx_prepare:
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
devlink_dpipe_entry_clear(&entry);
return err;
@@ -259,6 +263,7 @@ static int mlxsw_sp_dpipe_table_erif_counters_update(void *priv, bool enable)
int i;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
@@ -271,6 +276,7 @@ static int mlxsw_sp_dpipe_table_erif_counters_update(void *priv, bool enable)
mlxsw_sp_rif_counter_free(mlxsw_sp, rif,
MLXSW_SP_RIF_COUNTER_EGRESS);
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return 0;
}
@@ -547,6 +553,7 @@ mlxsw_sp_dpipe_table_host_entries_get(struct mlxsw_sp *mlxsw_sp,
int err;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
i = 0;
rif_count = MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS);
start_again:
@@ -602,11 +609,13 @@ mlxsw_sp_dpipe_table_host_entries_get(struct mlxsw_sp *mlxsw_sp,
if (i != rif_count)
goto start_again;
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return 0;
err_ctx_prepare:
err_entry_append:
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return err;
}
@@ -663,6 +672,7 @@ mlxsw_sp_dpipe_table_host_counters_update(struct mlxsw_sp *mlxsw_sp,
int i;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
struct mlxsw_sp_neigh_entry *neigh_entry;
@@ -684,6 +694,7 @@ mlxsw_sp_dpipe_table_host_counters_update(struct mlxsw_sp *mlxsw_sp,
enable);
}
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
}
@@ -702,6 +713,7 @@ mlxsw_sp_dpipe_table_host_size_get(struct mlxsw_sp *mlxsw_sp, int type)
int i;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
struct mlxsw_sp_neigh_entry *neigh_entry;
@@ -721,6 +733,7 @@ mlxsw_sp_dpipe_table_host_size_get(struct mlxsw_sp *mlxsw_sp, int type)
size++;
}
}
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return size;
@@ -1094,6 +1107,7 @@ mlxsw_sp_dpipe_table_adj_entries_get(struct mlxsw_sp *mlxsw_sp,
int err;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
nh_count_max = mlxsw_sp_dpipe_table_adj_size(mlxsw_sp);
start_again:
err = devlink_dpipe_entry_ctx_prepare(dump_ctx);
@@ -1130,12 +1144,14 @@ mlxsw_sp_dpipe_table_adj_entries_get(struct mlxsw_sp *mlxsw_sp,
devlink_dpipe_entry_ctx_close(dump_ctx);
if (nh_count != nh_count_max)
goto start_again;
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return 0;
err_ctx_prepare:
err_entry_append:
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return err;
}
@@ -1207,7 +1223,9 @@ mlxsw_sp_dpipe_table_adj_size_get(void *priv)
u64 size;
rtnl_lock();
+ mutex_lock(&mlxsw_sp->router->lock);
size = mlxsw_sp_dpipe_table_adj_size(mlxsw_sp);
+ mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
return size;
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 09/12] mlxsw: spectrum_router: Take router lock from netdev listener
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (7 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 08/12] mlxsw: spectrum_dpipe: Take router lock from dpipe code Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 10/12] mlxsw: spectrum_router: Take router lock from inetaddr listeners Ido Schimmel
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
One entry point into the routing code is from the netdev listener block.
Some netdev events require access to internal router structures. For
example, changing the MTU of a netdev requires looking-up the backing
RIF and adjusting its MTU.
In order to serialize access to shared router structures, take the
router lock when processing netdev events that require access to it.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.h | 2 +-
.../ethernet/mellanox/mlxsw/spectrum_router.c | 66 ++++++++++++-------
2 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 66cfe27e1f9b..bb02a0361bfd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -548,7 +548,7 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
struct netdev_notifier_changeupper_info *info);
bool mlxsw_sp_netdev_is_ipip_ol(const struct mlxsw_sp *mlxsw_sp,
const struct net_device *dev);
-bool mlxsw_sp_netdev_is_ipip_ul(const struct mlxsw_sp *mlxsw_sp,
+bool mlxsw_sp_netdev_is_ipip_ul(struct mlxsw_sp *mlxsw_sp,
const struct net_device *dev);
int mlxsw_sp_netdevice_ipip_ol_event(struct mlxsw_sp *mlxsw_sp,
struct net_device *l3_dev,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 601fa4a1abbb..7ad5cb5c2d3e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1346,10 +1346,16 @@ mlxsw_sp_ipip_entry_find_by_ul_dev(const struct mlxsw_sp *mlxsw_sp,
return NULL;
}
-bool mlxsw_sp_netdev_is_ipip_ul(const struct mlxsw_sp *mlxsw_sp,
+bool mlxsw_sp_netdev_is_ipip_ul(struct mlxsw_sp *mlxsw_sp,
const struct net_device *dev)
{
- return mlxsw_sp_ipip_entry_find_by_ul_dev(mlxsw_sp, dev, NULL);
+ bool is_ipip_ul;
+
+ mutex_lock(&mlxsw_sp->router->lock);
+ is_ipip_ul = mlxsw_sp_ipip_entry_find_by_ul_dev(mlxsw_sp, dev, NULL);
+ mutex_unlock(&mlxsw_sp->router->lock);
+
+ return is_ipip_ul;
}
static bool mlxsw_sp_netdevice_ipip_can_offload(struct mlxsw_sp *mlxsw_sp,
@@ -1721,35 +1727,41 @@ int mlxsw_sp_netdevice_ipip_ol_event(struct mlxsw_sp *mlxsw_sp,
{
struct netdev_notifier_changeupper_info *chup;
struct netlink_ext_ack *extack;
+ int err = 0;
+ mutex_lock(&mlxsw_sp->router->lock);
switch (event) {
case NETDEV_REGISTER:
- return mlxsw_sp_netdevice_ipip_ol_reg_event(mlxsw_sp, ol_dev);
+ err = mlxsw_sp_netdevice_ipip_ol_reg_event(mlxsw_sp, ol_dev);
+ break;
case NETDEV_UNREGISTER:
mlxsw_sp_netdevice_ipip_ol_unreg_event(mlxsw_sp, ol_dev);
- return 0;
+ break;
case NETDEV_UP:
mlxsw_sp_netdevice_ipip_ol_up_event(mlxsw_sp, ol_dev);
- return 0;
+ break;
case NETDEV_DOWN:
mlxsw_sp_netdevice_ipip_ol_down_event(mlxsw_sp, ol_dev);
- return 0;
+ break;
case NETDEV_CHANGEUPPER:
chup = container_of(info, typeof(*chup), info);
extack = info->extack;
if (netif_is_l3_master(chup->upper_dev))
- return mlxsw_sp_netdevice_ipip_ol_vrf_event(mlxsw_sp,
- ol_dev,
- extack);
- return 0;
+ err = mlxsw_sp_netdevice_ipip_ol_vrf_event(mlxsw_sp,
+ ol_dev,
+ extack);
+ break;
case NETDEV_CHANGE:
extack = info->extack;
- return mlxsw_sp_netdevice_ipip_ol_change_event(mlxsw_sp,
- ol_dev, extack);
+ err = mlxsw_sp_netdevice_ipip_ol_change_event(mlxsw_sp,
+ ol_dev, extack);
+ break;
case NETDEV_CHANGEMTU:
- return mlxsw_sp_netdevice_ipip_ol_update_mtu(mlxsw_sp, ol_dev);
+ err = mlxsw_sp_netdevice_ipip_ol_update_mtu(mlxsw_sp, ol_dev);
+ break;
}
- return 0;
+ mutex_unlock(&mlxsw_sp->router->lock);
+ return err;
}
static int
@@ -1793,8 +1805,9 @@ mlxsw_sp_netdevice_ipip_ul_event(struct mlxsw_sp *mlxsw_sp,
struct netdev_notifier_info *info)
{
struct mlxsw_sp_ipip_entry *ipip_entry = NULL;
- int err;
+ int err = 0;
+ mutex_lock(&mlxsw_sp->router->lock);
while ((ipip_entry = mlxsw_sp_ipip_entry_find_by_ul_dev(mlxsw_sp,
ul_dev,
ipip_entry))) {
@@ -1807,7 +1820,7 @@ mlxsw_sp_netdevice_ipip_ul_event(struct mlxsw_sp *mlxsw_sp,
if (err) {
mlxsw_sp_ipip_demote_tunnel_by_ul_netdev(mlxsw_sp,
ul_dev);
- return err;
+ break;
}
if (demote_this) {
@@ -1824,8 +1837,9 @@ mlxsw_sp_netdevice_ipip_ul_event(struct mlxsw_sp *mlxsw_sp,
ipip_entry = prev;
}
}
+ mutex_unlock(&mlxsw_sp->router->lock);
- return 0;
+ return err;
}
int mlxsw_sp_router_nve_promote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
@@ -7223,24 +7237,30 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev,
{
struct mlxsw_sp *mlxsw_sp;
struct mlxsw_sp_rif *rif;
+ int err = 0;
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
return 0;
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!rif)
- return 0;
+ goto out;
switch (event) {
case NETDEV_CHANGEMTU: /* fall through */
case NETDEV_CHANGEADDR:
- return mlxsw_sp_router_port_change_event(mlxsw_sp, rif);
+ err = mlxsw_sp_router_port_change_event(mlxsw_sp, rif);
+ break;
case NETDEV_PRE_CHANGEADDR:
- return mlxsw_sp_router_port_pre_changeaddr_event(rif, ptr);
+ err = mlxsw_sp_router_port_pre_changeaddr_event(rif, ptr);
+ break;
}
- return 0;
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
+ return err;
}
static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
@@ -7283,9 +7303,10 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
if (!mlxsw_sp || netif_is_macvlan(l3_dev))
return 0;
+ mutex_lock(&mlxsw_sp->router->lock);
switch (event) {
case NETDEV_PRECHANGEUPPER:
- return 0;
+ break;
case NETDEV_CHANGEUPPER:
if (info->linking) {
struct netlink_ext_ack *extack;
@@ -7297,6 +7318,7 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
}
break;
}
+ mutex_unlock(&mlxsw_sp->router->lock);
return err;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 10/12] mlxsw: spectrum_router: Take router lock from inetaddr listeners
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (8 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 09/12] mlxsw: spectrum_router: Take router lock from netdev listener Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 11/12] mlxsw: spectrum_router: Take router lock from exported helpers Ido Schimmel
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Another entry point into the routing code is from inetaddr listeners.
The driver registers listeners to IPv4 and IPv6 inetaddr notification
chains in order to understand when a RIF needs to be created or
destroyed.
Serialize access to shared router structures from these listeners by
taking the router lock when processing inetaddr events.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 7ad5cb5c2d3e..61d323d8b91d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -7029,15 +7029,17 @@ static int mlxsw_sp_inetaddr_event(struct notifier_block *nb,
/* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
if (event == NETDEV_UP)
- goto out;
+ return NOTIFY_DONE;
router = container_of(nb, struct mlxsw_sp_router, inetaddr_nb);
+ mutex_lock(&router->lock);
rif = mlxsw_sp_rif_find_by_dev(router->mlxsw_sp, dev);
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
err = __mlxsw_sp_inetaddr_event(router->mlxsw_sp, dev, event, NULL);
out:
+ mutex_unlock(&router->lock);
return notifier_from_errno(err);
}
@@ -7052,8 +7054,9 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
- goto out;
+ return NOTIFY_DONE;
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
@@ -7065,6 +7068,7 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
err = __mlxsw_sp_inetaddr_event(mlxsw_sp, dev, event, ivi->extack);
out:
+ mutex_unlock(&mlxsw_sp->router->lock);
return notifier_from_errno(err);
}
@@ -7138,8 +7142,9 @@ int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
- goto out;
+ return NOTIFY_DONE;
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
@@ -7151,6 +7156,7 @@ int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
err = __mlxsw_sp_inetaddr_event(mlxsw_sp, dev, event, i6vi->extack);
out:
+ mutex_unlock(&mlxsw_sp->router->lock);
return notifier_from_errno(err);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 11/12] mlxsw: spectrum_router: Take router lock from exported helpers
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (9 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 10/12] mlxsw: spectrum_router: Take router lock from inetaddr listeners Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 12/12] mlxsw: spectrum: Remove RTNL where possible Ido Schimmel
2020-02-23 5:25 ` [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path David Miller
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The routing code exports some helper functions that can be called from
other driver modules such as the bridge. These helpers are never called
with the router lock already held and therefore need to take it in order
to serialize access to shared router structures.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_router.c | 80 ++++++++++++++-----
1 file changed, 58 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 61d323d8b91d..0f5ecb47d0c2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -731,13 +731,18 @@ int mlxsw_sp_router_tb_id_vr_id(struct mlxsw_sp *mlxsw_sp, u32 tb_id,
u16 *vr_id)
{
struct mlxsw_sp_vr *vr;
+ int err = 0;
+ mutex_lock(&mlxsw_sp->router->lock);
vr = mlxsw_sp_vr_find(mlxsw_sp, tb_id);
- if (!vr)
- return -ESRCH;
+ if (!vr) {
+ err = -ESRCH;
+ goto out;
+ }
*vr_id = vr->id;
-
- return 0;
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
+ return err;
}
static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
@@ -1850,10 +1855,14 @@ int mlxsw_sp_router_nve_promote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
enum mlxsw_sp_fib_entry_type type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
struct mlxsw_sp_router *router = mlxsw_sp->router;
struct mlxsw_sp_fib_entry *fib_entry;
- int err;
+ int err = 0;
- if (WARN_ON_ONCE(router->nve_decap_config.valid))
- return -EINVAL;
+ mutex_lock(&mlxsw_sp->router->lock);
+
+ if (WARN_ON_ONCE(router->nve_decap_config.valid)) {
+ err = -EINVAL;
+ goto out;
+ }
router->nve_decap_config.ul_tb_id = ul_tb_id;
router->nve_decap_config.tunnel_index = tunnel_index;
@@ -1868,7 +1877,7 @@ int mlxsw_sp_router_nve_promote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
ul_proto, ul_sip,
type);
if (!fib_entry)
- return 0;
+ goto out;
fib_entry->decap.tunnel_index = tunnel_index;
fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_NVE_DECAP;
@@ -1877,11 +1886,13 @@ int mlxsw_sp_router_nve_promote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
if (err)
goto err_fib_entry_update;
- return 0;
+ goto out;
err_fib_entry_update:
fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
mlxsw_sp_fib_entry_update(mlxsw_sp, fib_entry);
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
return err;
}
@@ -1893,8 +1904,10 @@ void mlxsw_sp_router_nve_demote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
struct mlxsw_sp_router *router = mlxsw_sp->router;
struct mlxsw_sp_fib_entry *fib_entry;
+ mutex_lock(&mlxsw_sp->router->lock);
+
if (WARN_ON_ONCE(!router->nve_decap_config.valid))
- return;
+ goto out;
router->nve_decap_config.valid = false;
@@ -1902,10 +1915,12 @@ void mlxsw_sp_router_nve_demote_decap(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
ul_proto, ul_sip,
type);
if (!fib_entry)
- return;
+ goto out;
fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
mlxsw_sp_fib_entry_update(mlxsw_sp, fib_entry);
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
}
static bool mlxsw_sp_router_nve_is_decap(struct mlxsw_sp *mlxsw_sp,
@@ -6298,7 +6313,13 @@ mlxsw_sp_rif_find_by_dev(const struct mlxsw_sp *mlxsw_sp,
bool mlxsw_sp_rif_exists(struct mlxsw_sp *mlxsw_sp,
const struct net_device *dev)
{
- return !!mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ struct mlxsw_sp_rif *rif;
+
+ mutex_lock(&mlxsw_sp->router->lock);
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ mutex_unlock(&mlxsw_sp->router->lock);
+
+ return rif;
}
u16 mlxsw_sp_rif_vid(struct mlxsw_sp *mlxsw_sp, const struct net_device *dev)
@@ -6306,6 +6327,7 @@ u16 mlxsw_sp_rif_vid(struct mlxsw_sp *mlxsw_sp, const struct net_device *dev)
struct mlxsw_sp_rif *rif;
u16 vid = 0;
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!rif)
goto out;
@@ -6319,6 +6341,7 @@ u16 mlxsw_sp_rif_vid(struct mlxsw_sp *mlxsw_sp, const struct net_device *dev)
vid = mlxsw_sp_fid_8021q_vid(rif->fid);
out:
+ mutex_unlock(&mlxsw_sp->router->lock);
return vid;
}
@@ -6600,10 +6623,13 @@ void mlxsw_sp_rif_destroy_by_dev(struct mlxsw_sp *mlxsw_sp,
{
struct mlxsw_sp_rif *rif;
+ mutex_lock(&mlxsw_sp->router->lock);
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
if (!rif)
- return;
+ goto out;
mlxsw_sp_rif_destroy(rif);
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
}
static void
@@ -6725,7 +6751,11 @@ __mlxsw_sp_port_vlan_router_leave(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
void
mlxsw_sp_port_vlan_router_leave(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
{
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port_vlan->mlxsw_sp_port->mlxsw_sp;
+
+ mutex_lock(&mlxsw_sp->router->lock);
__mlxsw_sp_port_vlan_router_leave(mlxsw_sp_port_vlan);
+ mutex_unlock(&mlxsw_sp->router->lock);
}
static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
@@ -6947,7 +6977,9 @@ static void __mlxsw_sp_rif_macvlan_del(struct mlxsw_sp *mlxsw_sp,
void mlxsw_sp_rif_macvlan_del(struct mlxsw_sp *mlxsw_sp,
const struct net_device *macvlan_dev)
{
+ mutex_lock(&mlxsw_sp->router->lock);
__mlxsw_sp_rif_macvlan_del(mlxsw_sp, macvlan_dev);
+ mutex_unlock(&mlxsw_sp->router->lock);
}
static int mlxsw_sp_inetaddr_macvlan_event(struct mlxsw_sp *mlxsw_sp,
@@ -7833,28 +7865,32 @@ int mlxsw_sp_router_ul_rif_get(struct mlxsw_sp *mlxsw_sp, u32 ul_tb_id,
u16 *ul_rif_index)
{
struct mlxsw_sp_rif *ul_rif;
+ int err = 0;
- ASSERT_RTNL();
-
+ mutex_lock(&mlxsw_sp->router->lock);
ul_rif = mlxsw_sp_ul_rif_get(mlxsw_sp, ul_tb_id, NULL);
- if (IS_ERR(ul_rif))
- return PTR_ERR(ul_rif);
+ if (IS_ERR(ul_rif)) {
+ err = PTR_ERR(ul_rif);
+ goto out;
+ }
*ul_rif_index = ul_rif->rif_index;
-
- return 0;
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
+ return err;
}
void mlxsw_sp_router_ul_rif_put(struct mlxsw_sp *mlxsw_sp, u16 ul_rif_index)
{
struct mlxsw_sp_rif *ul_rif;
- ASSERT_RTNL();
-
+ mutex_lock(&mlxsw_sp->router->lock);
ul_rif = mlxsw_sp->router->rifs[ul_rif_index];
if (WARN_ON(!ul_rif))
- return;
+ goto out;
mlxsw_sp_ul_rif_put(ul_rif);
+out:
+ mutex_unlock(&mlxsw_sp->router->lock);
}
static int
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next 12/12] mlxsw: spectrum: Remove RTNL where possible
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (10 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 11/12] mlxsw: spectrum_router: Take router lock from exported helpers Ido Schimmel
@ 2020-02-21 17:54 ` Ido Schimmel
2020-02-23 5:25 ` [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path David Miller
12 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2020-02-21 17:54 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
After introducing the router lock in previous patches and making sure it
protects internal router structures, we no longer need to rely on RTNL
to serialize access to these structures.
Remove RTNL from call sites that no longer require it.
Two calls sites that keep taking the lock are
mlxsw_sp_router_fibmr_event_work() and mlxsw_sp_inet6addr_event_work().
The first calls into ACL code that still assumes RTNL is taken. The
second potentially calls into the FID code that also relies on RTNL.
Removing RTNL from these two call sites is the subject of future work.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_dpipe.c | 17 -----------------
.../ethernet/mellanox/mlxsw/spectrum_router.c | 12 ------------
2 files changed, 29 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 63fc1f56ef00..daf029931b5f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -211,7 +211,6 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
return err;
rif_count = MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS);
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
i = 0;
start_again:
@@ -244,7 +243,6 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
if (i != rif_count)
goto start_again;
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
devlink_dpipe_entry_clear(&entry);
return 0;
@@ -252,7 +250,6 @@ mlxsw_sp_dpipe_table_erif_entries_dump(void *priv, bool counters_enabled,
err_entry_get:
err_ctx_prepare:
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
devlink_dpipe_entry_clear(&entry);
return err;
}
@@ -262,7 +259,6 @@ static int mlxsw_sp_dpipe_table_erif_counters_update(void *priv, bool enable)
struct mlxsw_sp *mlxsw_sp = priv;
int i;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
@@ -277,7 +273,6 @@ static int mlxsw_sp_dpipe_table_erif_counters_update(void *priv, bool enable)
MLXSW_SP_RIF_COUNTER_EGRESS);
}
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return 0;
}
@@ -552,7 +547,6 @@ mlxsw_sp_dpipe_table_host_entries_get(struct mlxsw_sp *mlxsw_sp,
int i, j;
int err;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
i = 0;
rif_count = MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS);
@@ -610,13 +604,11 @@ mlxsw_sp_dpipe_table_host_entries_get(struct mlxsw_sp *mlxsw_sp,
goto start_again;
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return 0;
err_ctx_prepare:
err_entry_append:
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return err;
}
@@ -671,7 +663,6 @@ mlxsw_sp_dpipe_table_host_counters_update(struct mlxsw_sp *mlxsw_sp,
{
int i;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
@@ -695,7 +686,6 @@ mlxsw_sp_dpipe_table_host_counters_update(struct mlxsw_sp *mlxsw_sp,
}
}
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
}
static int mlxsw_sp_dpipe_table_host4_counters_update(void *priv, bool enable)
@@ -712,7 +702,6 @@ mlxsw_sp_dpipe_table_host_size_get(struct mlxsw_sp *mlxsw_sp, int type)
u64 size = 0;
int i;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS); i++) {
struct mlxsw_sp_rif *rif = mlxsw_sp_rif_by_index(mlxsw_sp, i);
@@ -734,7 +723,6 @@ mlxsw_sp_dpipe_table_host_size_get(struct mlxsw_sp *mlxsw_sp, int type)
}
}
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return size;
}
@@ -1106,7 +1094,6 @@ mlxsw_sp_dpipe_table_adj_entries_get(struct mlxsw_sp *mlxsw_sp,
int j;
int err;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
nh_count_max = mlxsw_sp_dpipe_table_adj_size(mlxsw_sp);
start_again:
@@ -1145,14 +1132,12 @@ mlxsw_sp_dpipe_table_adj_entries_get(struct mlxsw_sp *mlxsw_sp,
if (nh_count != nh_count_max)
goto start_again;
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return 0;
err_ctx_prepare:
err_entry_append:
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return err;
}
@@ -1222,11 +1207,9 @@ mlxsw_sp_dpipe_table_adj_size_get(void *priv)
struct mlxsw_sp *mlxsw_sp = priv;
u64 size;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
size = mlxsw_sp_dpipe_table_adj_size(mlxsw_sp);
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return size;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 0f5ecb47d0c2..b527387ccf80 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2307,7 +2307,6 @@ __mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp,
int i, num_rec;
int err;
- rtnl_lock();
/* Ensure the RIF we read from the device does not change mid-dump. */
mutex_lock(&mlxsw_sp->router->lock);
do {
@@ -2324,7 +2323,6 @@ __mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp,
i);
} while (mlxsw_sp_router_rauhtd_is_full(rauhtd_pl));
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
return err;
}
@@ -2355,7 +2353,6 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp)
{
struct mlxsw_sp_neigh_entry *neigh_entry;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
list_for_each_entry(neigh_entry, &mlxsw_sp->router->nexthop_neighs_list,
nexthop_neighs_list_node)
@@ -2364,7 +2361,6 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp)
*/
neigh_event_send(neigh_entry->key.n, NULL);
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
}
static void
@@ -2405,14 +2401,12 @@ static void mlxsw_sp_router_probe_unresolved_nexthops(struct work_struct *work)
* but it wouldn't get resolved ever in case traffic is flowing in HW
* using different nexthop.
*/
- rtnl_lock();
mutex_lock(&router->lock);
list_for_each_entry(neigh_entry, &router->nexthop_neighs_list,
nexthop_neighs_list_node)
if (!neigh_entry->connected)
neigh_event_send(neigh_entry->key.n, NULL);
mutex_unlock(&router->lock);
- rtnl_unlock();
mlxsw_core_schedule_dw(&router->nexthop_probe_dw,
MLXSW_SP_UNRESOLVED_NH_PROBE_INTERVAL);
@@ -2550,7 +2544,6 @@ static void mlxsw_sp_router_neigh_event_work(struct work_struct *work)
dead = n->dead;
read_unlock_bh(&n->lock);
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
@@ -2574,7 +2567,6 @@ static void mlxsw_sp_router_neigh_event_work(struct work_struct *work)
out:
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
neigh_release(n);
kfree(net_work);
}
@@ -5981,7 +5973,6 @@ static void mlxsw_sp_router_fib4_event_work(struct work_struct *work)
struct mlxsw_sp *mlxsw_sp = fib_work->mlxsw_sp;
int err;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
@@ -6005,7 +5996,6 @@ static void mlxsw_sp_router_fib4_event_work(struct work_struct *work)
break;
}
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
kfree(fib_work);
}
@@ -6016,7 +6006,6 @@ static void mlxsw_sp_router_fib6_event_work(struct work_struct *work)
struct mlxsw_sp *mlxsw_sp = fib_work->mlxsw_sp;
int err;
- rtnl_lock();
mutex_lock(&mlxsw_sp->router->lock);
mlxsw_sp_span_respin(mlxsw_sp);
@@ -6045,7 +6034,6 @@ static void mlxsw_sp_router_fib6_event_work(struct work_struct *work)
break;
}
mutex_unlock(&mlxsw_sp->router->lock);
- rtnl_unlock();
kfree(fib_work);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
` (11 preceding siblings ...)
2020-02-21 17:54 ` [PATCH net-next 12/12] mlxsw: spectrum: Remove RTNL where possible Ido Schimmel
@ 2020-02-23 5:25 ` David Miller
12 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2020-02-23 5:25 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, mlxsw, idosch
From: Ido Schimmel <idosch@idosch.org>
Date: Fri, 21 Feb 2020 19:54:03 +0200
> From: Ido Schimmel <idosch@mellanox.com>
>
> This patch set removes RTNL from the route insertion path in mlxsw in
> order to reduce the control plane latency: the time it takes to push
> routes from user space to the kernel and mlxsw.
>
> Up until now mlxsw did not have a lock to protect its shared router data
> structures and instead relied on RTNL. While this was simple and worked,
> it resulted in large control plane latencies as RTNL was heavily
> contended - by both the task receiving the netlink messages from user
> space and the mlxsw workqueue that programs the routes to the device.
>
> By removing RTNL and introducing a new router mutex, this patch set
> reduces the control plane latency by ~80%. A single mutex is added as
> inside mlxsw there is not a lot of concurrency. In addition, a more
> fine-grained locking scheme is much more error-prone.
>
> Patches #1-#6 are preparations. They add needed locking in NVE and
> multicast routing code instead of relying on RTNL
> Patch #7 introduces the new mutex
> Patches #8-#12 gradually take the lock in various entry points into the
> routing code
> Patch #13 removes RTNL in places where it is no longer required
Series applied, thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-23 5:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-21 17:54 [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 02/12] mlxsw: spectrum_mr: Protect multicast table list with a lock Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 03/12] mlxsw: spectrum_mr: Protect multicast route " Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 04/12] mlxsw: spectrum_router: Expose router struct to internal users Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 05/12] mlxsw: spectrum_router: Store NVE decapsulation configuration in router Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 06/12] mlxsw: spectrum_router: Introduce router lock Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 07/12] mlxsw: spectrum_router: Take router lock from inside routing code Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 08/12] mlxsw: spectrum_dpipe: Take router lock from dpipe code Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 09/12] mlxsw: spectrum_router: Take router lock from netdev listener Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 10/12] mlxsw: spectrum_router: Take router lock from inetaddr listeners Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 11/12] mlxsw: spectrum_router: Take router lock from exported helpers Ido Schimmel
2020-02-21 17:54 ` [PATCH net-next 12/12] mlxsw: spectrum: Remove RTNL where possible Ido Schimmel
2020-02-23 5:25 ` [PATCH net-next 00/12] mlxsw: Remove RTNL from route insertion path David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).