net/mlx5e: Extend mod header entry with reference counter
authorVlad Buslov <vladbu@mellanox.com>
Fri, 1 Jun 2018 18:47:43 +0000 (21:47 +0300)
committerSaeed Mahameed <saeedm@mellanox.com>
Fri, 9 Aug 2019 21:54:09 +0000 (14:54 -0700)
List of flows attached to mod header entry is used as implicit reference
counter (mod header entry is deallocated when list becomes free) and as a
mechanism to obtain mod header entry that flow is attached to (through list
head). This is not safe when concurrent modification of list of flows
attached to mod header entry is possible. Proper atomic reference counter
is required to support concurrent access.

As a preparation for extending mod header with reference counting, extract
code that lookups and deletes mod header entry into standalone put/get
helpers. In order to remove this dependency on external locking, extend mod
header entry with reference counter to manage its lifetime and extend flow
structure with direct pointer to mod header entry that flow is attached to.

To remove code duplication between legacy and switchdev mode
implementations that both support mod_hdr functionality, store mod_hdr
table in dedicated structure used by both fdb and kernel namespaces. New
table structure is extended with table lock by one of the following patches
in this series. Implement helper function to get correct mod_hdr table
depending on flow namespace.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Jianbo Liu <jianbol@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
include/linux/mlx5/fs.h

index 100506a3dd58922cd43800322b015110f0a922d8..ca2161b42c7ff5fd10564b0c36c16f49f5e80d92 100644 (file)
@@ -16,7 +16,7 @@ struct mlx5e_tc_table {
 
        struct rhashtable               ht;
 
-       DECLARE_HASHTABLE(mod_hdr_tbl, 8);
+       struct mod_hdr_tbl mod_hdr;
        struct mutex hairpin_tbl_lock; /* protects hairpin_tbl */
        DECLARE_HASHTABLE(hairpin_tbl, 8);
 
index b6a91e3054c0f8f547c4d4c1597a42cd8ba00884..fe1b04aa910a07b08ac96d966f00fb17c435251d 100644 (file)
@@ -119,6 +119,7 @@ struct mlx5e_tc_flow {
         */
        struct encap_flow_item encaps[MLX5_MAX_FLOW_FWD_VPORTS];
        struct mlx5e_tc_flow    *peer_flow;
+       struct mlx5e_mod_hdr_entry *mh; /* attached mod header instance */
        struct list_head        mod_hdr; /* flows sharing the same mod hdr ID */
        struct mlx5e_hairpin_entry *hpe; /* attached hairpin instance */
        struct list_head        hairpin; /* flows sharing the same hairpin */
@@ -194,6 +195,8 @@ struct mlx5e_mod_hdr_entry {
        struct mod_hdr_key key;
 
        u32 mod_hdr_id;
+
+       refcount_t refcnt;
 };
 
 #define MLX5_MH_ACT_SZ MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto)
@@ -284,14 +287,51 @@ static inline int cmp_mod_hdr_info(struct mod_hdr_key *a,
        return memcmp(a->actions, b->actions, a->num_actions * MLX5_MH_ACT_SZ);
 }
 
+static struct mod_hdr_tbl *
+get_mod_hdr_table(struct mlx5e_priv *priv, int namespace)
+{
+       struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
+
+       return namespace == MLX5_FLOW_NAMESPACE_FDB ? &esw->offloads.mod_hdr :
+               &priv->fs.tc.mod_hdr;
+}
+
+static struct mlx5e_mod_hdr_entry *
+mlx5e_mod_hdr_get(struct mod_hdr_tbl *tbl, struct mod_hdr_key *key, u32 hash_key)
+{
+       struct mlx5e_mod_hdr_entry *mh, *found = NULL;
+
+       hash_for_each_possible(tbl->hlist, mh, mod_hdr_hlist, hash_key) {
+               if (!cmp_mod_hdr_info(&mh->key, key)) {
+                       refcount_inc(&mh->refcnt);
+                       found = mh;
+                       break;
+               }
+       }
+
+       return found;
+}
+
+static void mlx5e_mod_hdr_put(struct mlx5e_priv *priv,
+                             struct mlx5e_mod_hdr_entry *mh)
+{
+       if (!refcount_dec_and_test(&mh->refcnt))
+               return;
+
+       WARN_ON(!list_empty(&mh->flows));
+       mlx5_modify_header_dealloc(priv->mdev, mh->mod_hdr_id);
+       hash_del(&mh->mod_hdr_hlist);
+       kfree(mh);
+}
+
 static int mlx5e_attach_mod_hdr(struct mlx5e_priv *priv,
                                struct mlx5e_tc_flow *flow,
                                struct mlx5e_tc_flow_parse_attr *parse_attr)
 {
-       struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
+       bool is_eswitch_flow = mlx5e_is_eswitch_flow(flow);
        int num_actions, actions_size, namespace, err;
-       bool found = false, is_eswitch_flow;
        struct mlx5e_mod_hdr_entry *mh;
+       struct mod_hdr_tbl *tbl;
        struct mod_hdr_key key;
        u32 hash_key;
 
@@ -303,28 +343,12 @@ static int mlx5e_attach_mod_hdr(struct mlx5e_priv *priv,
 
        hash_key = hash_mod_hdr_info(&key);
 
-       is_eswitch_flow = mlx5e_is_eswitch_flow(flow);
-       if (is_eswitch_flow) {
-               namespace = MLX5_FLOW_NAMESPACE_FDB;
-               hash_for_each_possible(esw->offloads.mod_hdr_tbl, mh,
-                                      mod_hdr_hlist, hash_key) {
-                       if (!cmp_mod_hdr_info(&mh->key, &key)) {
-                               found = true;
-                               break;
-                       }
-               }
-       } else {
-               namespace = MLX5_FLOW_NAMESPACE_KERNEL;
-               hash_for_each_possible(priv->fs.tc.mod_hdr_tbl, mh,
-                                      mod_hdr_hlist, hash_key) {
-                       if (!cmp_mod_hdr_info(&mh->key, &key)) {
-                               found = true;
-                               break;
-                       }
-               }
-       }
+       namespace = is_eswitch_flow ?
+               MLX5_FLOW_NAMESPACE_FDB : MLX5_FLOW_NAMESPACE_KERNEL;
+       tbl = get_mod_hdr_table(priv, namespace);
 
-       if (found)
+       mh = mlx5e_mod_hdr_get(tbl, &key, hash_key);
+       if (mh)
                goto attach_flow;
 
        mh = kzalloc(sizeof(*mh) + actions_size, GFP_KERNEL);
@@ -335,6 +359,7 @@ static int mlx5e_attach_mod_hdr(struct mlx5e_priv *priv,
        memcpy(mh->key.actions, key.actions, actions_size);
        mh->key.num_actions = num_actions;
        INIT_LIST_HEAD(&mh->flows);
+       refcount_set(&mh->refcnt, 1);
 
        err = mlx5_modify_header_alloc(priv->mdev, namespace,
                                       mh->key.num_actions,
@@ -343,12 +368,10 @@ static int mlx5e_attach_mod_hdr(struct mlx5e_priv *priv,
        if (err)
                goto out_err;
 
-       if (is_eswitch_flow)
-               hash_add(esw->offloads.mod_hdr_tbl, &mh->mod_hdr_hlist, hash_key);
-       else
-               hash_add(priv->fs.tc.mod_hdr_tbl, &mh->mod_hdr_hlist, hash_key);
+       hash_add(tbl->hlist, &mh->mod_hdr_hlist, hash_key);
 
 attach_flow:
+       flow->mh = mh;
        list_add(&flow->mod_hdr, &mh->flows);
        if (is_eswitch_flow)
                flow->esw_attr->mod_hdr_id = mh->mod_hdr_id;
@@ -365,23 +388,14 @@ out_err:
 static void mlx5e_detach_mod_hdr(struct mlx5e_priv *priv,
                                 struct mlx5e_tc_flow *flow)
 {
-       struct list_head *next = flow->mod_hdr.next;
-
        /* flow wasn't fully initialized */
-       if (list_empty(&flow->mod_hdr))
+       if (!flow->mh)
                return;
 
        list_del(&flow->mod_hdr);
 
-       if (list_empty(next)) {
-               struct mlx5e_mod_hdr_entry *mh;
-
-               mh = list_entry(next, struct mlx5e_mod_hdr_entry, flows);
-
-               mlx5_modify_header_dealloc(priv->mdev, mh->mod_hdr_id);
-               hash_del(&mh->mod_hdr_hlist);
-               kfree(mh);
-       }
+       mlx5e_mod_hdr_put(priv, flow->mh);
+       flow->mh = NULL;
 }
 
 static
@@ -3844,7 +3858,7 @@ int mlx5e_tc_nic_init(struct mlx5e_priv *priv)
        int err;
 
        mutex_init(&tc->t_lock);
-       hash_init(tc->mod_hdr_tbl);
+       hash_init(tc->mod_hdr.hlist);
        mutex_init(&tc->hairpin_tbl_lock);
        hash_init(tc->hairpin_tbl);
 
index 5fbebee7254d1899ee99192bd87224c983df2877..5ce3c81e308341b076922da93aec0f3d8906d75c 100644 (file)
@@ -2000,7 +2000,7 @@ int mlx5_eswitch_init(struct mlx5_core_dev *dev)
                goto abort;
 
        hash_init(esw->offloads.encap_tbl);
-       hash_init(esw->offloads.mod_hdr_tbl);
+       hash_init(esw->offloads.mod_hdr.hlist);
        atomic64_set(&esw->offloads.num_flows, 0);
        mutex_init(&esw->state_lock);
 
index 804912e38dee21036faba79eb7925ccf7499d82c..fd63ba4ed0da33ac65426f648b49266282f15ce5 100644 (file)
@@ -182,7 +182,7 @@ struct mlx5_esw_offload {
        struct list_head peer_flows;
        struct mutex peer_mutex;
        DECLARE_HASHTABLE(encap_tbl, 8);
-       DECLARE_HASHTABLE(mod_hdr_tbl, 8);
+       struct mod_hdr_tbl mod_hdr;
        DECLARE_HASHTABLE(termtbl_tbl, 8);
        struct mutex termtbl_mutex; /* protects termtbl hash */
        const struct mlx5_eswitch_rep_ops *rep_ops[NUM_REP_TYPES];
index f049af3f3cd838c85d87e02d5e13f5a39d1ffec0..96650a33aa919392c5ae48ce7d2b1cd9f0ba256a 100644 (file)
@@ -126,6 +126,10 @@ struct mlx5_flow_destination {
        };
 };
 
+struct mod_hdr_tbl {
+       DECLARE_HASHTABLE(hlist, 8);
+};
+
 struct mlx5_flow_namespace *
 mlx5_get_fdb_sub_ns(struct mlx5_core_dev *dev, int n);
 struct mlx5_flow_namespace *