Skip to content

Commit

Permalink
net/mlx5: CT: Change idr to xarray to protect parallel tuple id alloc…
Browse files Browse the repository at this point in the history
…ation

After allowing parallel tuple insertion, we get the following trace:

[ 5505.142249] ------------[ cut here ]------------
[ 5505.148155] WARNING: CPU: 21 PID: 13313 at lib/radix-tree.c:581 delete_node+0x16c/0x180
[ 5505.295553] CPU: 21 PID: 13313 Comm: kworker/u50:22 Tainted: G           OE     5.6.0+ #78
[ 5505.304824] Hardware name: Supermicro Super Server/X10DRT-P, BIOS 2.0b 03/30/2017
[ 5505.313740] Workqueue: nf_flow_table_offload flow_offload_work_handler [nf_flow_table]
[ 5505.323257] RIP: 0010:delete_node+0x16c/0x180
[ 5505.349862] RSP: 0018:ffffb19184eb7b30 EFLAGS: 00010282
[ 5505.356785] RAX: 0000000000000000 RBX: ffff904ac95b86d8 RCX: ffff904b6f938838
[ 5505.365190] RDX: 0000000000000000 RSI: ffff904ac954b908 RDI: ffff904ac954b920
[ 5505.373628] RBP: ffff904b4ac13060 R08: 0000000000000001 R09: 0000000000000000
[ 5505.382155] R10: 0000000000000000 R11: 0000000000000040 R12: 0000000000000000
[ 5505.390527] R13: ffffb19184eb7bfc R14: ffff904b6bef5800 R15: ffff90482c1203c0
[ 5505.399246] FS:  0000000000000000(0000) GS:ffff904c2fc80000(0000) knlGS:0000000000000000
[ 5505.408621] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5505.415739] CR2: 00007f5d27006010 CR3: 0000000058c10006 CR4: 00000000001626e0
[ 5505.424547] Call Trace:
[ 5505.428429]  idr_alloc_u32+0x7b/0xc0
[ 5505.433803]  mlx5_tc_ct_entry_add_rule+0xbf/0x950 [mlx5_core]
[ 5505.441354]  ? mlx5_fc_create+0x23c/0x370 [mlx5_core]
[ 5505.448225]  mlx5_tc_ct_block_flow_offload+0x874/0x10b0 [mlx5_core]
[ 5505.456278]  ? mlx5_tc_ct_block_flow_offload+0x63d/0x10b0 [mlx5_core]
[ 5505.464532]  nf_flow_offload_tuple.isra.21+0xc5/0x140 [nf_flow_table]
[ 5505.472286]  ? __kmalloc+0x217/0x2f0
[ 5505.477093]  ? flow_rule_alloc+0x1c/0x30
[ 5505.482117]  flow_offload_work_handler+0x1d0/0x290 [nf_flow_table]
[ 5505.489674]  ? process_one_work+0x17c/0x580
[ 5505.494922]  process_one_work+0x202/0x580
[ 5505.500082]  ? process_one_work+0x17c/0x580
[ 5505.505696]  worker_thread+0x4c/0x3f0
[ 5505.510458]  kthread+0x103/0x140
[ 5505.514989]  ? process_one_work+0x580/0x580
[ 5505.520616]  ? kthread_bind+0x10/0x10
[ 5505.525837]  ret_from_fork+0x3a/0x50
[ 5505.570841] ---[ end trace 07995de9c56d6831 ]---

This happens from parallel deletes/adds to idr, as idr isn't protected.
Fix that by using xarray as the tuple_ids allocator instead of idr.

Fixes: 7da182a ("netfilter: flowtable: Use work entry per offload command")
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
  • Loading branch information
Paul Blakey authored and Saeed Mahameed committed Apr 20, 2020
1 parent a019b36 commit 70840b6
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <net/flow_offload.h>
#include <net/netfilter/nf_flow_table.h>
#include <linux/workqueue.h>
#include <linux/xarray.h>

#include "esw/chains.h"
#include "en/tc_ct.h"
Expand All @@ -35,7 +36,7 @@ struct mlx5_tc_ct_priv {
struct mlx5_eswitch *esw;
const struct net_device *netdev;
struct idr fte_ids;
struct idr tuple_ids;
struct xarray tuple_ids;
struct rhashtable zone_ht;
struct mlx5_flow_table *ct;
struct mlx5_flow_table *ct_nat;
Expand Down Expand Up @@ -238,7 +239,7 @@ mlx5_tc_ct_entry_del_rule(struct mlx5_tc_ct_priv *ct_priv,

mlx5_eswitch_del_offloaded_rule(esw, zone_rule->rule, attr);
mlx5_modify_header_dealloc(esw->dev, attr->modify_hdr);
idr_remove(&ct_priv->tuple_ids, zone_rule->tupleid);
xa_erase(&ct_priv->tuple_ids, zone_rule->tupleid);
}

static void
Expand Down Expand Up @@ -483,7 +484,7 @@ mlx5_tc_ct_entry_add_rule(struct mlx5_tc_ct_priv *ct_priv,
struct mlx5_esw_flow_attr *attr = &zone_rule->attr;
struct mlx5_eswitch *esw = ct_priv->esw;
struct mlx5_flow_spec *spec = NULL;
u32 tupleid = 1;
u32 tupleid;
int err;

zone_rule->nat = nat;
Expand All @@ -493,12 +494,12 @@ mlx5_tc_ct_entry_add_rule(struct mlx5_tc_ct_priv *ct_priv,
return -ENOMEM;

/* Get tuple unique id */
err = idr_alloc_u32(&ct_priv->tuple_ids, zone_rule, &tupleid,
TUPLE_ID_MAX, GFP_KERNEL);
err = xa_alloc(&ct_priv->tuple_ids, &tupleid, zone_rule,
XA_LIMIT(1, TUPLE_ID_MAX), GFP_KERNEL);
if (err) {
netdev_warn(ct_priv->netdev,
"Failed to allocate tuple id, err: %d\n", err);
goto err_idr_alloc;
goto err_xa_alloc;
}
zone_rule->tupleid = tupleid;

Expand Down Expand Up @@ -539,8 +540,8 @@ mlx5_tc_ct_entry_add_rule(struct mlx5_tc_ct_priv *ct_priv,
err_rule:
mlx5_modify_header_dealloc(esw->dev, attr->modify_hdr);
err_mod_hdr:
idr_remove(&ct_priv->tuple_ids, zone_rule->tupleid);
err_idr_alloc:
xa_erase(&ct_priv->tuple_ids, zone_rule->tupleid);
err_xa_alloc:
kfree(spec);
return err;
}
Expand Down Expand Up @@ -1299,7 +1300,7 @@ mlx5_tc_ct_init(struct mlx5_rep_uplink_priv *uplink_priv)
}

idr_init(&ct_priv->fte_ids);
idr_init(&ct_priv->tuple_ids);
xa_init_flags(&ct_priv->tuple_ids, XA_FLAGS_ALLOC1);
mutex_init(&ct_priv->control_lock);
rhashtable_init(&ct_priv->zone_ht, &zone_params);

Expand Down Expand Up @@ -1334,7 +1335,7 @@ mlx5_tc_ct_clean(struct mlx5_rep_uplink_priv *uplink_priv)

rhashtable_destroy(&ct_priv->zone_ht);
mutex_destroy(&ct_priv->control_lock);
idr_destroy(&ct_priv->tuple_ids);
xa_destroy(&ct_priv->tuple_ids);
idr_destroy(&ct_priv->fte_ids);
kfree(ct_priv);

Expand All @@ -1352,7 +1353,7 @@ mlx5e_tc_ct_restore_flow(struct mlx5_rep_uplink_priv *uplink_priv,
if (!ct_priv || !tupleid)
return true;

zone_rule = idr_find(&ct_priv->tuple_ids, tupleid);
zone_rule = xa_load(&ct_priv->tuple_ids, tupleid);
if (!zone_rule)
return false;

Expand Down

0 comments on commit 70840b6

Please sign in to comment.