netdev - [net-next 04/11] net/mlx5e: Refactor mlx5e_neigh_update_table->encap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190821232806.21847-5-saeedm@mellanox.com>
Date:   Wed, 21 Aug 2019 23:28:40 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "David S. Miller" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Vlad Buslov <vladbu@...lanox.com>,
        Jianbo Liu <jianbol@...lanox.com>,
        Roi Dayan <roid@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>
Subject: [net-next 04/11] net/mlx5e: Refactor
 mlx5e_neigh_update_table->encap_lock

From: Vlad Buslov <vladbu@...lanox.com>

To remove dependency on rtnl lock, always take neigh update encap lock when
modifying neigh update hash table and list. Originally, this lock was only
used to synchronize with netevent handler function, which is called from bh
context and cannot use rtnl lock for synchronization. Take lock in encap
entry attach function to prevent concurrent modifications of neigh update
hash table and list.

Taking the encap lock when creating new nhe introduces a problem that we
need to allocate new entry with sleeping GFP_KERNEL flag while holding a
spinlock. However, since previous patch in this series has already
converted lookup in netevent handler function to user rcu read lock instead
of encap lock, we can safely convert the lock type to mutex.

Signed-off-by: Vlad Buslov <vladbu@...lanox.com>
Reviewed-by: Jianbo Liu <jianbol@...lanox.com>
Reviewed-by: Roi Dayan <roid@...lanox.com>
Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rep.c    | 17 ++++++++++++-----
 .../net/ethernet/mellanox/mlx5/core/en_rep.h    |  3 ++-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index a294dc6b5a0c..218772d5c062 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -973,7 +973,7 @@ static int mlx5e_rep_neigh_init(struct mlx5e_rep_priv *rpriv)
 		return err;
 
 	INIT_LIST_HEAD(&neigh_update->neigh_list);
-	spin_lock_init(&neigh_update->encap_lock);
+	mutex_init(&neigh_update->encap_lock);
 	INIT_DELAYED_WORK(&neigh_update->neigh_stats_work,
 			  mlx5e_rep_neigh_stats_work);
 	mlx5e_rep_neigh_update_init_interval(rpriv);
@@ -1000,6 +1000,7 @@ static void mlx5e_rep_neigh_cleanup(struct mlx5e_rep_priv *rpriv)
 
 	cancel_delayed_work_sync(&rpriv->neigh_update.neigh_stats_work);
 
+	mutex_destroy(&neigh_update->encap_lock);
 	rhashtable_destroy(&neigh_update->neigh_ht);
 }
 
@@ -1024,18 +1025,18 @@ static void mlx5e_rep_neigh_entry_remove(struct mlx5e_neigh_hash_entry *nhe)
 {
 	struct mlx5e_rep_priv *rpriv = nhe->priv->ppriv;
 
-	spin_lock_bh(&rpriv->neigh_update.encap_lock);
+	mutex_lock(&rpriv->neigh_update.encap_lock);
 
 	list_del_rcu(&nhe->neigh_list);
 
 	rhashtable_remove_fast(&rpriv->neigh_update.neigh_ht,
 			       &nhe->rhash_node,
 			       mlx5e_neigh_ht_params);
-	spin_unlock_bh(&rpriv->neigh_update.encap_lock);
+	mutex_unlock(&rpriv->neigh_update.encap_lock);
 }
 
-/* This function must only be called under RTNL lock or under the
- * representor's encap_lock in case RTNL mutex can't be held.
+/* This function must only be called under the representor's encap_lock or
+ * inside rcu read lock section.
  */
 static struct mlx5e_neigh_hash_entry *
 mlx5e_rep_neigh_entry_lookup(struct mlx5e_priv *priv,
@@ -1088,17 +1089,23 @@ int mlx5e_rep_encap_entry_attach(struct mlx5e_priv *priv,
 	err = mlx5_tun_entropy_refcount_inc(tun_entropy, e->reformat_type);
 	if (err)
 		return err;
+
+	mutex_lock(&rpriv->neigh_update.encap_lock);
 	nhe = mlx5e_rep_neigh_entry_lookup(priv, &e->m_neigh);
 	if (!nhe) {
 		err = mlx5e_rep_neigh_entry_create(priv, e, &nhe);
 		if (err) {
+			mutex_unlock(&rpriv->neigh_update.encap_lock);
 			mlx5_tun_entropy_refcount_dec(tun_entropy,
 						      e->reformat_type);
 			return err;
 		}
 	}
+
 	e->nhe = nhe;
 	list_add(&e->encap_list, &nhe->encap_list);
+	mutex_unlock(&rpriv->neigh_update.encap_lock);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index d057e401b0de..8fa27832bd81 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -35,6 +35,7 @@
 
 #include <net/ip_tunnels.h>
 #include <linux/rhashtable.h>
+#include <linux/mutex.h>
 #include "eswitch.h"
 #include "en.h"
 #include "lib/port_tun.h"
@@ -48,7 +49,7 @@ struct mlx5e_neigh_update_table {
 	 */
 	struct list_head	neigh_list;
 	/* protect lookup/remove operations */
-	spinlock_t              encap_lock;
+	struct mutex		encap_lock;
 	struct notifier_block   netevent_nb;
 	struct delayed_work     neigh_stats_work;
 	unsigned long           min_interval; /* jiffies */
-- 
2.21.0