l2tp: replace hlist with simple list for per-tunnel session list

The per-tunnel session list is no longer used by the
datapath. However, we still need a list of sessions in the tunnel for
l2tp_session_get_nth, which is used by management code. (An
alternative might be to walk each session IDR list, matching only
sessions of a given tunnel.)

Replace the per-tunnel hlist with a per-tunnel list. In functions
which walk a list of sessions of a tunnel, walk this list instead.

Signed-off-by: James Chapman <jchapman@katalix.com>
Reviewed-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 3ce689331542514c94935f95d6c6e6dd12ec9685..be4bcbf291a172f4641ae75e7e8ae92dce3be76f 100644 (file)
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -39,7 +39,6 @@
 #include <linux/ip.h>
 #include <linux/udp.h>
 #include <linux/l2tp.h>
-#include <linux/hash.h>
 #include <linux/sort.h>
 #include <linux/file.h>
 #include <linux/nsproxy.h>
@@ -137,18 +136,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net)
        return net_generic(net, l2tp_net_id);
 }
 
-/* Session hash list.
- * The session_id SHOULD be random according to RFC2661, but several
- * L2TP implementations (Cisco and Microsoft) use incrementing
- * session_ids.  So we do a real hash on the session_id, rather than a
- * simple bitmask.
- */
-static inline struct hlist_head *
-l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id)
-{
-       return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)];
-}
-
 static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)
 {
        trace_free_tunnel(tunnel);
@@ -306,21 +293,17 @@ EXPORT_SYMBOL_GPL(l2tp_session_get);
 
 struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth)
 {
-       int hash;
        struct l2tp_session *session;
        int count = 0;
 
        rcu_read_lock_bh();
-       for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
-               hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) {
-                       if (++count > nth) {
-                               l2tp_session_inc_refcount(session);
-                               rcu_read_unlock_bh();
-                               return session;
-                       }
+       list_for_each_entry_rcu(session, &tunnel->session_list, list) {
+               if (++count > nth) {
+                       l2tp_session_inc_refcount(session);
+                       rcu_read_unlock_bh();
+                       return session;
                }
        }
-
        rcu_read_unlock_bh();
 
        return NULL;
@@ -334,21 +317,23 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
                                                const char *ifname)
 {
        struct l2tp_net *pn = l2tp_pernet(net);
-       unsigned long session_id, tmp;
+       unsigned long tunnel_id, tmp;
        struct l2tp_session *session;
+       struct l2tp_tunnel *tunnel;
 
        rcu_read_lock_bh();
-       idr_for_each_entry_ul(&pn->l2tp_v3_session_idr, session, tmp, session_id) {
-               if (session) {
-                       if (!strcmp(session->ifname, ifname)) {
-                               l2tp_session_inc_refcount(session);
-                               rcu_read_unlock_bh();
-
-                               return session;
+       idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) {
+               if (tunnel) {
+                       list_for_each_entry_rcu(session, &tunnel->session_list, list) {
+                               if (!strcmp(session->ifname, ifname)) {
+                                       l2tp_session_inc_refcount(session);
+                                       rcu_read_unlock_bh();
+
+                                       return session;
+                               }
                        }
                }
        }
-
        rcu_read_unlock_bh();
 
        return NULL;
@@ -452,25 +437,15 @@ int l2tp_session_register(struct l2tp_session *session,
                          struct l2tp_tunnel *tunnel)
 {
        struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
-       struct l2tp_session *session_walk;
-       struct hlist_head *head;
        u32 session_key;
        int err;
 
-       head = l2tp_session_id_hash(tunnel, session->session_id);
-
-       spin_lock_bh(&tunnel->hlist_lock);
+       spin_lock_bh(&tunnel->list_lock);
        if (!tunnel->acpt_newsess) {
                err = -ENODEV;
                goto err_tlock;
        }
 
-       hlist_for_each_entry(session_walk, head, hlist)
-               if (session_walk->session_id == session->session_id) {
-                       err = -EEXIST;
-                       goto err_tlock;
-               }
-
        if (tunnel->version == L2TP_HDR_VER_3) {
                session_key = session->session_id;
                spin_lock_bh(&pn->l2tp_session_idr_lock);
@@ -506,8 +481,8 @@ int l2tp_session_register(struct l2tp_session *session,
 
        l2tp_tunnel_inc_refcount(tunnel);
 
-       hlist_add_head_rcu(&session->hlist, head);
-       spin_unlock_bh(&tunnel->hlist_lock);
+       list_add(&session->list, &tunnel->session_list);
+       spin_unlock_bh(&tunnel->list_lock);
 
        spin_lock_bh(&pn->l2tp_session_idr_lock);
        if (tunnel->version == L2TP_HDR_VER_3)
@@ -521,7 +496,7 @@ int l2tp_session_register(struct l2tp_session *session,
        return 0;
 
 err_tlock:
-       spin_unlock_bh(&tunnel->hlist_lock);
+       spin_unlock_bh(&tunnel->list_lock);
 
        return err;
 }
@@ -1275,20 +1250,19 @@ end:
        return;
 }
 
-/* Remove an l2tp session from l2tp_core's hash lists. */
+/* Remove an l2tp session from l2tp_core's lists. */
 static void l2tp_session_unhash(struct l2tp_session *session)
 {
        struct l2tp_tunnel *tunnel = session->tunnel;
 
-       /* Remove the session from core hashes */
        if (tunnel) {
                struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
                struct l2tp_session *removed = session;
 
-               /* Remove from the per-tunnel hash */
-               spin_lock_bh(&tunnel->hlist_lock);
-               hlist_del_init_rcu(&session->hlist);
-               spin_unlock_bh(&tunnel->hlist_lock);
+               /* Remove from the per-tunnel list */
+               spin_lock_bh(&tunnel->list_lock);
+               list_del_init(&session->list);
+               spin_unlock_bh(&tunnel->list_lock);
 
                /* Remove from per-net IDR */
                spin_lock_bh(&pn->l2tp_session_idr_lock);
@@ -1316,28 +1290,19 @@ static void l2tp_session_unhash(struct l2tp_session *session)
 static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
 {
        struct l2tp_session *session;
-       int hash;
+       struct list_head __rcu *pos;
+       struct list_head *tmp;
 
-       spin_lock_bh(&tunnel->hlist_lock);
+       spin_lock_bh(&tunnel->list_lock);
        tunnel->acpt_newsess = false;
-       for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
-again:
-               hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) {
-                       hlist_del_init_rcu(&session->hlist);
-
-                       spin_unlock_bh(&tunnel->hlist_lock);
-                       l2tp_session_delete(session);
-                       spin_lock_bh(&tunnel->hlist_lock);
-
-                       /* Now restart from the beginning of this hash
-                        * chain.  We always remove a session from the
-                        * list so we are guaranteed to make forward
-                        * progress.
-                        */
-                       goto again;
-               }
+       list_for_each_safe(pos, tmp, &tunnel->session_list) {
+               session = list_entry(pos, struct l2tp_session, list);
+               list_del_init(&session->list);
+               spin_unlock_bh(&tunnel->list_lock);
+               l2tp_session_delete(session);
+               spin_lock_bh(&tunnel->list_lock);
        }
-       spin_unlock_bh(&tunnel->hlist_lock);
+       spin_unlock_bh(&tunnel->list_lock);
 }
 
 /* Tunnel socket destroy hook for UDP encapsulation */
@@ -1531,8 +1496,9 @@ int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id,
 
        tunnel->magic = L2TP_TUNNEL_MAGIC;
        sprintf(&tunnel->name[0], "tunl %u", tunnel_id);
-       spin_lock_init(&tunnel->hlist_lock);
+       spin_lock_init(&tunnel->list_lock);
        tunnel->acpt_newsess = true;
+       INIT_LIST_HEAD(&tunnel->session_list);
 
        tunnel->encap = encap;
 
@@ -1732,6 +1698,7 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
                session->hlist_key = l2tp_v3_session_hashkey(tunnel->sock, session->session_id);
                INIT_HLIST_NODE(&session->hlist);
                INIT_LIST_HEAD(&session->clist);
+               INIT_LIST_HEAD(&session->list);
 
                if (cfg) {
                        session->pwtype = cfg->pw_type;

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index bfff69f2e0a232e09b056f3eb1c67779f3413c36..8ac81bc1bc6fa33d3cbb43da70c74c4c6d373de8 100644 (file)
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -19,10 +19,6 @@
 #define L2TP_TUNNEL_MAGIC      0x42114DDA
 #define L2TP_SESSION_MAGIC     0x0C04EB7D
 
-/* Per tunnel session hash table size */
-#define L2TP_HASH_BITS 4
-#define L2TP_HASH_SIZE BIT(L2TP_HASH_BITS)
-
 struct sk_buff;
 
 struct l2tp_stats {
@@ -65,8 +61,7 @@ struct l2tp_session_coll_list {
 
 /* Represents a session (pseudowire) instance.
  * Tracks runtime state including cookies, dataplane packet sequencing, and IO statistics.
- * Is linked into a per-tunnel session hashlist; and in the case of an L2TPv3 session into
- * an additional per-net ("global") hashlist.
+ * Is linked into a per-tunnel session list and a per-net ("global") IDR tree.
  */
 #define L2TP_SESSION_NAME_MAX 32
 struct l2tp_session {
@@ -90,6 +85,7 @@ struct l2tp_session {
        u32                     nr_oos;         /* NR of last OOS packet */
        int                     nr_oos_count;   /* for OOS recovery */
        int                     nr_oos_count_max;
+       struct list_head        list;           /* per-tunnel list node */
        refcount_t              ref_count;
        struct hlist_node       hlist;          /* per-net session hlist */
        unsigned long           hlist_key;      /* key for session hlist */
@@ -118,7 +114,7 @@ struct l2tp_session {
        /* Session close handler.
         * Each pseudowire implementation may implement this callback in order to carry
         * out pseudowire-specific shutdown actions.
-        * The callback is called by core after unhashing the session and purging its
+        * The callback is called by core after unlisting the session and purging its
         * reorder queue.
         */
        void (*session_close)(struct l2tp_session *session);
@@ -154,7 +150,7 @@ struct l2tp_tunnel_cfg {
 /* Represents a tunnel instance.
  * Tracks runtime state including IO statistics.
  * Holds the tunnel socket (either passed from userspace or directly created by the kernel).
- * Maintains a hashlist of sessions belonging to the tunnel instance.
+ * Maintains a list of sessions belonging to the tunnel instance.
  * Is linked into a per-net list of tunnels.
  */
 #define L2TP_TUNNEL_NAME_MAX 20
@@ -164,12 +160,11 @@ struct l2tp_tunnel {
        unsigned long           dead;
 
        struct rcu_head rcu;
-       spinlock_t              hlist_lock;     /* write-protection for session_hlist */
+       spinlock_t              list_lock;      /* write-protection for session_list */
        bool                    acpt_newsess;   /* indicates whether this tunnel accepts
-                                                * new sessions. Protected by hlist_lock.
+                                                * new sessions. Protected by list_lock.
                                                 */
-       struct hlist_head       session_hlist[L2TP_HASH_SIZE];
-                                               /* hashed list of sessions, hashed by id */
+       struct list_head        session_list;   /* list of sessions */
        u32                     tunnel_id;
        u32                     peer_tunnel_id;
        int                     version;        /* 2=>L2TPv2, 3=>L2TPv3 */

diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index 4595b56d175d9174ef0bd58abc7848388b00da42..8755ae521154d77eb5bd106fefc25e728641102e 100644 (file)
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -123,17 +123,14 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
        struct l2tp_tunnel *tunnel = v;
        struct l2tp_session *session;
        int session_count = 0;
-       int hash;
 
        rcu_read_lock_bh();
-       for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
-               hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) {
-                       /* Session ID of zero is a dummy/reserved value used by pppol2tp */
-                       if (session->session_id == 0)
-                               continue;
+       list_for_each_entry_rcu(session, &tunnel->session_list, list) {
+               /* Session ID of zero is a dummy/reserved value used by pppol2tp */
+               if (session->session_id == 0)
+                       continue;
 
-                       session_count++;
-               }
+               session_count++;
        }
        rcu_read_unlock_bh();