diff mbox series

[v2,net-next,6/9] l2tp: use get_next APIs for management requests and procfs/debugfs

Message ID 0ed95752e184f213260e84b4ff3ee4f4bedeed9e.1723011569.git.jchapman@katalix.com (mailing list archive)
State Accepted
Commit 1f4c3dce9112d23145b4f8bbfd3793a1c4c517ab
Delegated to: Netdev Maintainers
Headers show
Series l2tp: misc improvements | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 29 this patch: 29
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-08-07--09-00 (tests: 707)

Commit Message

James Chapman Aug. 7, 2024, 6:54 a.m. UTC 
l2tp netlink and procfs/debugfs iterate over tunnel and session lists
to obtain data. They currently use very inefficient get_nth functions
to do so. Replace these with get_next.

For netlink, use nl cb->ctx[] for passing state instead of the
obsolete cb->args[].

l2tp_tunnel_get_nth and l2tp_session_get_nth are no longer used so
they can be removed.

Signed-off-by: James Chapman <jchapman@katalix.com>
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
 net/l2tp/l2tp_core.c    | 40 ----------------------------------------
 net/l2tp/l2tp_core.h    |  2 --
 net/l2tp/l2tp_debugfs.c | 16 +++++++++-------
 net/l2tp/l2tp_netlink.c | 34 +++++++++++++++++++++-------------
 net/l2tp/l2tp_ppp.c     | 16 +++++++++-------
 5 files changed, 39 insertions(+), 69 deletions(-)

Comments

Cong Wang Aug. 11, 2024, 6:36 p.m. UTC | #1 
On Wed, Aug 07, 2024 at 07:54:49AM +0100, James Chapman wrote:
> diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
> index cc464982a7d9..0fabacffc3f3 100644
> --- a/net/l2tp/l2tp_core.h
> +++ b/net/l2tp/l2tp_core.h
> @@ -219,14 +219,12 @@ void l2tp_session_dec_refcount(struct l2tp_session *session);
>   * the caller must ensure that the reference is dropped appropriately.
>   */
>  struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
> -struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
>  struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key);
>  
>  struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id);
>  struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id);
>  struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver,
>  				      u32 tunnel_id, u32 session_id);
> -struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
>  struct l2tp_session *l2tp_session_get_next(const struct net *net, struct sock *sk, int pver,
>  					   u32 tunnel_id, unsigned long *key);
>  struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
> diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
> index 8755ae521154..b2134b57ed18 100644
> --- a/net/l2tp/l2tp_debugfs.c
> +++ b/net/l2tp/l2tp_debugfs.c
> @@ -34,8 +34,8 @@ static struct dentry *rootdir;
>  struct l2tp_dfs_seq_data {
>  	struct net	*net;
>  	netns_tracker	ns_tracker;
> -	int tunnel_idx;			/* current tunnel */
> -	int session_idx;		/* index of session within current tunnel */
> +	unsigned long tkey;		/* lookup key of current tunnel */
> +	unsigned long skey;		/* lookup key of current session */

Any reason to change the type from int to unsigned long?

Asking because tunnel ID remains to be 32bit unsigned int as a part of
UAPI.

Thanks.
James Chapman Aug. 12, 2024, 8:14 a.m. UTC | #2 
On 11/08/2024 19:36, Cong Wang wrote:
> On Wed, Aug 07, 2024 at 07:54:49AM +0100, James Chapman wrote:
>> diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
>> index cc464982a7d9..0fabacffc3f3 100644
>> --- a/net/l2tp/l2tp_core.h
>> +++ b/net/l2tp/l2tp_core.h
>> @@ -219,14 +219,12 @@ void l2tp_session_dec_refcount(struct l2tp_session *session);
>>    * the caller must ensure that the reference is dropped appropriately.
>>    */
>>   struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
>> -struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
>>   struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key);
>>   
>>   struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id);
>>   struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id);
>>   struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver,
>>   				      u32 tunnel_id, u32 session_id);
>> -struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
>>   struct l2tp_session *l2tp_session_get_next(const struct net *net, struct sock *sk, int pver,
>>   					   u32 tunnel_id, unsigned long *key);
>>   struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
>> diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
>> index 8755ae521154..b2134b57ed18 100644
>> --- a/net/l2tp/l2tp_debugfs.c
>> +++ b/net/l2tp/l2tp_debugfs.c
>> @@ -34,8 +34,8 @@ static struct dentry *rootdir;
>>   struct l2tp_dfs_seq_data {
>>   	struct net	*net;
>>   	netns_tracker	ns_tracker;
>> -	int tunnel_idx;			/* current tunnel */
>> -	int session_idx;		/* index of session within current tunnel */
>> +	unsigned long tkey;		/* lookup key of current tunnel */
>> +	unsigned long skey;		/* lookup key of current session */
> 
> Any reason to change the type from int to unsigned long?
> 
> Asking because tunnel ID remains to be 32bit unsigned int as a part of
> UAPI.
> 
> Thanks.

It's used as the key in and potentially modified by idr_get_next_ul calls.
Cong Wang Aug. 14, 2024, 6:05 p.m. UTC | #3 
On Mon, Aug 12, 2024 at 09:14:42AM +0100, James Chapman wrote:
> On 11/08/2024 19:36, Cong Wang wrote:
> > On Wed, Aug 07, 2024 at 07:54:49AM +0100, James Chapman wrote:
> > > diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
> > > index cc464982a7d9..0fabacffc3f3 100644
> > > --- a/net/l2tp/l2tp_core.h
> > > +++ b/net/l2tp/l2tp_core.h
> > > @@ -219,14 +219,12 @@ void l2tp_session_dec_refcount(struct l2tp_session *session);
> > >    * the caller must ensure that the reference is dropped appropriately.
> > >    */
> > >   struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
> > > -struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
> > >   struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key);
> > >   struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id);
> > >   struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id);
> > >   struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver,
> > >   				      u32 tunnel_id, u32 session_id);
> > > -struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
> > >   struct l2tp_session *l2tp_session_get_next(const struct net *net, struct sock *sk, int pver,
> > >   					   u32 tunnel_id, unsigned long *key);
> > >   struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
> > > diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
> > > index 8755ae521154..b2134b57ed18 100644
> > > --- a/net/l2tp/l2tp_debugfs.c
> > > +++ b/net/l2tp/l2tp_debugfs.c
> > > @@ -34,8 +34,8 @@ static struct dentry *rootdir;
> > >   struct l2tp_dfs_seq_data {
> > >   	struct net	*net;
> > >   	netns_tracker	ns_tracker;
> > > -	int tunnel_idx;			/* current tunnel */
> > > -	int session_idx;		/* index of session within current tunnel */
> > > +	unsigned long tkey;		/* lookup key of current tunnel */
> > > +	unsigned long skey;		/* lookup key of current session */
> > 
> > Any reason to change the type from int to unsigned long?
> > 
> > Asking because tunnel ID remains to be 32bit unsigned int as a part of
> > UAPI.
> > 
> > Thanks.
> 
> It's used as the key in and potentially modified by idr_get_next_ul calls.

idr_get_next() takes an `int` ID. So the reason is not this API, it must be
something else.
James Chapman Aug. 15, 2024, 8:21 a.m. UTC | #4 
On 14/08/2024 19:05, Cong Wang wrote:
> On Mon, Aug 12, 2024 at 09:14:42AM +0100, James Chapman wrote:
>> On 11/08/2024 19:36, Cong Wang wrote:
>>> On Wed, Aug 07, 2024 at 07:54:49AM +0100, James Chapman wrote:
>>>> diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
>>>> index cc464982a7d9..0fabacffc3f3 100644
>>>> --- a/net/l2tp/l2tp_core.h
>>>> +++ b/net/l2tp/l2tp_core.h
>>>> @@ -219,14 +219,12 @@ void l2tp_session_dec_refcount(struct l2tp_session *session);
>>>>     * the caller must ensure that the reference is dropped appropriately.
>>>>     */
>>>>    struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
>>>> -struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
>>>>    struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key);
>>>>    struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id);
>>>>    struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id);
>>>>    struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver,
>>>>    				      u32 tunnel_id, u32 session_id);
>>>> -struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
>>>>    struct l2tp_session *l2tp_session_get_next(const struct net *net, struct sock *sk, int pver,
>>>>    					   u32 tunnel_id, unsigned long *key);
>>>>    struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
>>>> diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
>>>> index 8755ae521154..b2134b57ed18 100644
>>>> --- a/net/l2tp/l2tp_debugfs.c
>>>> +++ b/net/l2tp/l2tp_debugfs.c
>>>> @@ -34,8 +34,8 @@ static struct dentry *rootdir;
>>>>    struct l2tp_dfs_seq_data {
>>>>    	struct net	*net;
>>>>    	netns_tracker	ns_tracker;
>>>> -	int tunnel_idx;			/* current tunnel */
>>>> -	int session_idx;		/* index of session within current tunnel */
>>>> +	unsigned long tkey;		/* lookup key of current tunnel */
>>>> +	unsigned long skey;		/* lookup key of current session */
>>>
>>> Any reason to change the type from int to unsigned long?
>>>
>>> Asking because tunnel ID remains to be 32bit unsigned int as a part of
>>> UAPI.
>>>
>>> Thanks.
>>
>> It's used as the key in and potentially modified by idr_get_next_ul calls.
> 
> idr_get_next() takes an `int` ID. So the reason is not this API, it must be
> something else.
> 

l2tp doesn't use idr_get_next; it uses idr_get_next_ul which takes an 
unsigned long. An int isn't big enough for a u32 tunnel ID value in 
32-bit architectures without potentially going negative.

The previous code used an int tunnel_idx as input for 
l2tp_tunnel_get_nth. This is replaced by l2tp_tunnel_get_next which gets 
the next item from an entry given by a key where key is a tunnel ID. The 
next item is a tunnel with the next highest tunnel ID, hence going 
negative would cause problems.
diff mbox series

Patch

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index c3d6aa5309c7..c0d525fc85e1 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -236,27 +236,6 @@  struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id)
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_get);
 
-struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth)
-{
-	struct l2tp_net *pn = l2tp_pernet(net);
-	unsigned long tunnel_id, tmp;
-	struct l2tp_tunnel *tunnel;
-	int count = 0;
-
-	rcu_read_lock_bh();
-	idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) {
-		if (tunnel && ++count > nth &&
-		    refcount_inc_not_zero(&tunnel->ref_count)) {
-			rcu_read_unlock_bh();
-			return tunnel;
-		}
-	}
-	rcu_read_unlock_bh();
-
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(l2tp_tunnel_get_nth);
-
 struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key)
 {
 	struct l2tp_net *pn = l2tp_pernet(net);
@@ -350,25 +329,6 @@  struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, in
 }
 EXPORT_SYMBOL_GPL(l2tp_session_get);
 
-struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth)
-{
-	struct l2tp_session *session;
-	int count = 0;
-
-	rcu_read_lock_bh();
-	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;
-}
-EXPORT_SYMBOL_GPL(l2tp_session_get_nth);
-
 static struct l2tp_session *l2tp_v2_session_get_next(const struct net *net,
 						     u16 tid,
 						     unsigned long *key)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index cc464982a7d9..0fabacffc3f3 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -219,14 +219,12 @@  void l2tp_session_dec_refcount(struct l2tp_session *session);
  * the caller must ensure that the reference is dropped appropriately.
  */
 struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
-struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
 struct l2tp_tunnel *l2tp_tunnel_get_next(const struct net *net, unsigned long *key);
 
 struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id);
 struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id);
 struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver,
 				      u32 tunnel_id, u32 session_id);
-struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
 struct l2tp_session *l2tp_session_get_next(const struct net *net, struct sock *sk, int pver,
 					   u32 tunnel_id, unsigned long *key);
 struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index 8755ae521154..b2134b57ed18 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -34,8 +34,8 @@  static struct dentry *rootdir;
 struct l2tp_dfs_seq_data {
 	struct net	*net;
 	netns_tracker	ns_tracker;
-	int tunnel_idx;			/* current tunnel */
-	int session_idx;		/* index of session within current tunnel */
+	unsigned long tkey;		/* lookup key of current tunnel */
+	unsigned long skey;		/* lookup key of current session */
 	struct l2tp_tunnel *tunnel;
 	struct l2tp_session *session;	/* NULL means get next tunnel */
 };
@@ -46,8 +46,8 @@  static void l2tp_dfs_next_tunnel(struct l2tp_dfs_seq_data *pd)
 	if (pd->tunnel)
 		l2tp_tunnel_dec_refcount(pd->tunnel);
 
-	pd->tunnel = l2tp_tunnel_get_nth(pd->net, pd->tunnel_idx);
-	pd->tunnel_idx++;
+	pd->tunnel = l2tp_tunnel_get_next(pd->net, &pd->tkey);
+	pd->tkey++;
 }
 
 static void l2tp_dfs_next_session(struct l2tp_dfs_seq_data *pd)
@@ -56,11 +56,13 @@  static void l2tp_dfs_next_session(struct l2tp_dfs_seq_data *pd)
 	if (pd->session)
 		l2tp_session_dec_refcount(pd->session);
 
-	pd->session = l2tp_session_get_nth(pd->tunnel, pd->session_idx);
-	pd->session_idx++;
+	pd->session = l2tp_session_get_next(pd->net, pd->tunnel->sock,
+					    pd->tunnel->version,
+					    pd->tunnel->tunnel_id, &pd->skey);
+	pd->skey++;
 
 	if (!pd->session) {
-		pd->session_idx = 0;
+		pd->skey = 0;
 		l2tp_dfs_next_tunnel(pd);
 	}
 }
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index fc43ecbd128c..0598b97a0bca 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -491,14 +491,20 @@  static int l2tp_nl_cmd_tunnel_get(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
+struct l2tp_nl_cb_data {
+	unsigned long tkey;
+	unsigned long skey;
+};
+
 static int l2tp_nl_cmd_tunnel_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	int ti = cb->args[0];
+	struct l2tp_nl_cb_data *cbd = (void *)&cb->ctx[0];
+	unsigned long key = cbd->tkey;
 	struct l2tp_tunnel *tunnel;
 	struct net *net = sock_net(skb->sk);
 
 	for (;;) {
-		tunnel = l2tp_tunnel_get_nth(net, ti);
+		tunnel = l2tp_tunnel_get_next(net, &key);
 		if (!tunnel)
 			goto out;
 
@@ -510,11 +516,11 @@  static int l2tp_nl_cmd_tunnel_dump(struct sk_buff *skb, struct netlink_callback
 		}
 		l2tp_tunnel_dec_refcount(tunnel);
 
-		ti++;
+		key++;
 	}
 
 out:
-	cb->args[0] = ti;
+	cbd->tkey = key;
 
 	return skb->len;
 }
@@ -832,25 +838,27 @@  static int l2tp_nl_cmd_session_get(struct sk_buff *skb, struct genl_info *info)
 
 static int l2tp_nl_cmd_session_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct l2tp_nl_cb_data *cbd = (void *)&cb->ctx[0];
 	struct net *net = sock_net(skb->sk);
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel = NULL;
-	int ti = cb->args[0];
-	int si = cb->args[1];
+	unsigned long tkey = cbd->tkey;
+	unsigned long skey = cbd->skey;
 
 	for (;;) {
 		if (!tunnel) {
-			tunnel = l2tp_tunnel_get_nth(net, ti);
+			tunnel = l2tp_tunnel_get_next(net, &tkey);
 			if (!tunnel)
 				goto out;
 		}
 
-		session = l2tp_session_get_nth(tunnel, si);
+		session = l2tp_session_get_next(net, tunnel->sock, tunnel->version,
+						tunnel->tunnel_id, &skey);
 		if (!session) {
-			ti++;
+			tkey++;
 			l2tp_tunnel_dec_refcount(tunnel);
 			tunnel = NULL;
-			si = 0;
+			skey = 0;
 			continue;
 		}
 
@@ -863,12 +871,12 @@  static int l2tp_nl_cmd_session_dump(struct sk_buff *skb, struct netlink_callback
 		}
 		l2tp_session_dec_refcount(session);
 
-		si++;
+		skey++;
 	}
 
 out:
-	cb->args[0] = ti;
-	cb->args[1] = si;
+	cbd->tkey = tkey;
+	cbd->skey = skey;
 
 	return skb->len;
 }
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index c25dd8e36074..8459e5159430 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1397,8 +1397,8 @@  static int pppol2tp_getsockopt(struct socket *sock, int level, int optname,
 
 struct pppol2tp_seq_data {
 	struct seq_net_private p;
-	int tunnel_idx;			/* current tunnel */
-	int session_idx;		/* index of session within current tunnel */
+	unsigned long tkey;		/* lookup key of current tunnel */
+	unsigned long skey;		/* lookup key of current session */
 	struct l2tp_tunnel *tunnel;
 	struct l2tp_session *session;	/* NULL means get next tunnel */
 };
@@ -1410,8 +1410,8 @@  static void pppol2tp_next_tunnel(struct net *net, struct pppol2tp_seq_data *pd)
 		l2tp_tunnel_dec_refcount(pd->tunnel);
 
 	for (;;) {
-		pd->tunnel = l2tp_tunnel_get_nth(net, pd->tunnel_idx);
-		pd->tunnel_idx++;
+		pd->tunnel = l2tp_tunnel_get_next(net, &pd->tkey);
+		pd->tkey++;
 
 		/* Only accept L2TPv2 tunnels */
 		if (!pd->tunnel || pd->tunnel->version == 2)
@@ -1427,11 +1427,13 @@  static void pppol2tp_next_session(struct net *net, struct pppol2tp_seq_data *pd)
 	if (pd->session)
 		l2tp_session_dec_refcount(pd->session);
 
-	pd->session = l2tp_session_get_nth(pd->tunnel, pd->session_idx);
-	pd->session_idx++;
+	pd->session = l2tp_session_get_next(net, pd->tunnel->sock,
+					    pd->tunnel->version,
+					    pd->tunnel->tunnel_id, &pd->skey);
+	pd->skey++;
 
 	if (!pd->session) {
-		pd->session_idx = 0;
+		pd->skey = 0;
 		pppol2tp_next_tunnel(net, pd);
 	}
 }