Netdev List
 help / color / mirror / Atom feed
* [RFC net-next] pppoe: remove session socket receive support
@ 2026-08-06  9:16 Qingfang Deng
  2026-08-09 14:58 ` Pali Rohár
  0 siblings, 1 reply; 5+ messages in thread
From: Qingfang Deng @ 2026-08-06  9:16 UTC (permalink / raw)
  To: Guillaume Nault, Pali Rohár, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Qingfang Deng,
	Kees Cook, Asim Viladi Oglu Manizada, Eric Woudstra,
	Felix Fietkau, netdev, linux-kernel
  Cc: Paul Mackerras, Michal Ostrowski

No known userspace PPPoE implementation reads from an AF_PPPOX
socket. In particular, pppd uses the socket only to establish a
session and obtain a generic PPP channel with PPPIOCGCHAN. Packet I/O
then uses /dev/ppp.

Before this change, packets received before PPPIOCGCHAN set
PPPOX_BOUND were queued on sk_receive_queue. Since no known
implementation consumed them, they remained there until socket
destruction and unnecessarily consumed memory.

Pass every matched session packet directly to ppp_input(). On a
successful connect, the channel is registered before the socket lock
is released. Generic PPP can therefore queue early packets until
userspace attaches /dev/ppp, preserving packets that arrive between
connect() and PPPIOCGCHAN.

Replace pppoe_recvmsg() with sock_no_recvmsg(). Like PPTP, omit the
poll callback because the socket no longer receives packets. Remove
pppoe_destruct() because the receive queue is no longer used.

Assisted-by: Codex:GPT-5.6
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ppp/pppoe.c | 50 ++---------------------------------------
 1 file changed, 2 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index bf7414b46a26..e3d425943c76 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -350,23 +350,9 @@ static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
 {
 	struct pppox_sock *po = pppox_sk(sk);
 
-	/* Backlog receive. Semantics of backlog rcv preclude any code from
-	 * executing in lock_sock()/release_sock() bounds; meaning sk->sk_state
-	 * can't change.
-	 */
-
-	if (sk->sk_state & PPPOX_BOUND) {
-		ppp_input(&po->chan, skb);
-	} else {
-		if (sock_queue_rcv_skb(sk, skb))
-			goto abort_kfree;
-	}
+	ppp_input(&po->chan, skb);
 
 	return NET_RX_SUCCESS;
-
-abort_kfree:
-	kfree_skb(skb);
-	return NET_RX_DROP;
 }
 
 /************************************************************************
@@ -498,11 +484,6 @@ static struct proto pppoe_sk_proto __read_mostly = {
 	.obj_size = sizeof(struct pppox_sock),
 };
 
-static void pppoe_destruct(struct sock *sk)
-{
-	skb_queue_purge(&sk->sk_receive_queue);
-}
-
 /***********************************************************************
  *
  * Initialize a new struct sock.
@@ -523,7 +504,6 @@ static int pppoe_create(struct net *net, struct socket *sock, int kern)
 	sock->ops	= &pppoe_ops;
 
 	sk->sk_backlog_rcv	= pppoe_rcv_core;
-	sk->sk_destruct		= pppoe_destruct;
 	sk->sk_state		= PPPOX_NONE;
 	sk->sk_type		= SOCK_STREAM;
 	sk->sk_family		= PF_PPPOX;
@@ -921,31 +901,6 @@ static const struct ppp_channel_ops pppoe_chan_ops = {
 	.fill_forward_path = pppoe_fill_forward_path,
 };
 
-static int pppoe_recvmsg(struct socket *sock, struct msghdr *m,
-			 size_t total_len, int flags)
-{
-	struct sock *sk = sock->sk;
-	struct sk_buff *skb;
-	int error = 0;
-
-	if (sk->sk_state & PPPOX_BOUND)
-		return -EIO;
-
-	skb = skb_recv_datagram(sk, flags, &error);
-	if (!skb)
-		return error;
-
-	total_len = min_t(size_t, total_len, skb->len);
-	error = skb_copy_datagram_msg(skb, 0, m, total_len);
-	if (error == 0) {
-		consume_skb(skb);
-		return total_len;
-	}
-
-	kfree_skb(skb);
-	return error;
-}
-
 #ifdef CONFIG_PROC_FS
 static int pppoe_seq_show(struct seq_file *seq, void *v)
 {
@@ -1046,11 +1001,10 @@ static const struct proto_ops pppoe_ops = {
 	.socketpair	= sock_no_socketpair,
 	.accept		= sock_no_accept,
 	.getname	= pppoe_getname,
-	.poll		= datagram_poll,
 	.listen		= sock_no_listen,
 	.shutdown	= sock_no_shutdown,
 	.sendmsg	= pppoe_sendmsg,
-	.recvmsg	= pppoe_recvmsg,
+	.recvmsg	= sock_no_recvmsg,
 	.mmap		= sock_no_mmap,
 	.ioctl		= pppox_ioctl,
 #ifdef CONFIG_COMPAT
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC net-next] pppoe: remove session socket receive support
  2026-08-06  9:16 [RFC net-next] pppoe: remove session socket receive support Qingfang Deng
@ 2026-08-09 14:58 ` Pali Rohár
  2026-08-09 16:17   ` Qingfang Deng
  0 siblings, 1 reply; 5+ messages in thread
From: Pali Rohár @ 2026-08-09 14:58 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Guillaume Nault, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Kees Cook, Asim Viladi Oglu Manizada,
	Eric Woudstra, Felix Fietkau, netdev, linux-kernel,
	Paul Mackerras, Michal Ostrowski

Hello, I would like to point out that "no know open source
userspace implementation" does not have to mean that there cannot be
some closed source or not publicly available. But I agree that
probability is too low.

Anyway, could you describe what is this functionality (which is being
removed) doing? Or how to use it? From the code and description I have
not figure out how it is suppose to be used or for what kind of scenario
or use case can be it hypothetically used.

Would be nice to know what exactly is being removing and what userspace
applications cannot do after applying this change.

On Thursday 06 August 2026 17:16:24 Qingfang Deng wrote:
> No known userspace PPPoE implementation reads from an AF_PPPOX
> socket. In particular, pppd uses the socket only to establish a
> session and obtain a generic PPP channel with PPPIOCGCHAN. Packet I/O
> then uses /dev/ppp.
> 
> Before this change, packets received before PPPIOCGCHAN set
> PPPOX_BOUND were queued on sk_receive_queue. Since no known
> implementation consumed them, they remained there until socket
> destruction and unnecessarily consumed memory.
> 
> Pass every matched session packet directly to ppp_input(). On a
> successful connect, the channel is registered before the socket lock
> is released. Generic PPP can therefore queue early packets until
> userspace attaches /dev/ppp, preserving packets that arrive between
> connect() and PPPIOCGCHAN.
> 
> Replace pppoe_recvmsg() with sock_no_recvmsg(). Like PPTP, omit the
> poll callback because the socket no longer receives packets. Remove
> pppoe_destruct() because the receive queue is no longer used.
> 
> Assisted-by: Codex:GPT-5.6
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
>  drivers/net/ppp/pppoe.c | 50 ++---------------------------------------
>  1 file changed, 2 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index bf7414b46a26..e3d425943c76 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -350,23 +350,9 @@ static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct pppox_sock *po = pppox_sk(sk);
>  
> -	/* Backlog receive. Semantics of backlog rcv preclude any code from
> -	 * executing in lock_sock()/release_sock() bounds; meaning sk->sk_state
> -	 * can't change.
> -	 */
> -
> -	if (sk->sk_state & PPPOX_BOUND) {
> -		ppp_input(&po->chan, skb);
> -	} else {
> -		if (sock_queue_rcv_skb(sk, skb))
> -			goto abort_kfree;
> -	}
> +	ppp_input(&po->chan, skb);
>  
>  	return NET_RX_SUCCESS;
> -
> -abort_kfree:
> -	kfree_skb(skb);
> -	return NET_RX_DROP;
>  }
>  
>  /************************************************************************
> @@ -498,11 +484,6 @@ static struct proto pppoe_sk_proto __read_mostly = {
>  	.obj_size = sizeof(struct pppox_sock),
>  };
>  
> -static void pppoe_destruct(struct sock *sk)
> -{
> -	skb_queue_purge(&sk->sk_receive_queue);
> -}
> -
>  /***********************************************************************
>   *
>   * Initialize a new struct sock.
> @@ -523,7 +504,6 @@ static int pppoe_create(struct net *net, struct socket *sock, int kern)
>  	sock->ops	= &pppoe_ops;
>  
>  	sk->sk_backlog_rcv	= pppoe_rcv_core;
> -	sk->sk_destruct		= pppoe_destruct;
>  	sk->sk_state		= PPPOX_NONE;
>  	sk->sk_type		= SOCK_STREAM;
>  	sk->sk_family		= PF_PPPOX;
> @@ -921,31 +901,6 @@ static const struct ppp_channel_ops pppoe_chan_ops = {
>  	.fill_forward_path = pppoe_fill_forward_path,
>  };
>  
> -static int pppoe_recvmsg(struct socket *sock, struct msghdr *m,
> -			 size_t total_len, int flags)
> -{
> -	struct sock *sk = sock->sk;
> -	struct sk_buff *skb;
> -	int error = 0;
> -
> -	if (sk->sk_state & PPPOX_BOUND)
> -		return -EIO;
> -
> -	skb = skb_recv_datagram(sk, flags, &error);
> -	if (!skb)
> -		return error;
> -
> -	total_len = min_t(size_t, total_len, skb->len);
> -	error = skb_copy_datagram_msg(skb, 0, m, total_len);
> -	if (error == 0) {
> -		consume_skb(skb);
> -		return total_len;
> -	}
> -
> -	kfree_skb(skb);
> -	return error;
> -}
> -
>  #ifdef CONFIG_PROC_FS
>  static int pppoe_seq_show(struct seq_file *seq, void *v)
>  {
> @@ -1046,11 +1001,10 @@ static const struct proto_ops pppoe_ops = {
>  	.socketpair	= sock_no_socketpair,
>  	.accept		= sock_no_accept,
>  	.getname	= pppoe_getname,
> -	.poll		= datagram_poll,
>  	.listen		= sock_no_listen,
>  	.shutdown	= sock_no_shutdown,
>  	.sendmsg	= pppoe_sendmsg,
> -	.recvmsg	= pppoe_recvmsg,
> +	.recvmsg	= sock_no_recvmsg,
>  	.mmap		= sock_no_mmap,
>  	.ioctl		= pppox_ioctl,
>  #ifdef CONFIG_COMPAT
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC net-next] pppoe: remove session socket receive support
  2026-08-09 14:58 ` Pali Rohár
@ 2026-08-09 16:17   ` Qingfang Deng
  2026-08-09 22:59     ` Pali Rohár
  0 siblings, 1 reply; 5+ messages in thread
From: Qingfang Deng @ 2026-08-09 16:17 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Guillaume Nault, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Kees Cook, Asim Viladi Oglu Manizada,
	Eric Woudstra, Felix Fietkau, netdev, linux-kernel,
	Paul Mackerras, Michal Ostrowski

Hi Pali,

On 8/9/2026 10:58 PM, Pali Rohár wrote:
> Hello, I would like to point out that "no know open source
> userspace implementation" does not have to mean that there cannot be
> some closed source or not publicly available. But I agree that
> probability is too low.
> 
> Anyway, could you describe what is this functionality (which is being
> removed) doing? Or how to use it? From the code and description I have
> not figure out how it is suppose to be used or for what kind of scenario
> or use case can be it hypothetically used.
> 
> Would be nice to know what exactly is being removing and what userspace
> applications cannot do after applying this change.

pppoe_recvmsg()/sendmsg() can be used to receive/send PPPoE session 
packets without the generic layer. I just found out that the RP-PPPoE 
package, maintained by Dianne Skoll, actually provides a standalone 
PPPoE client that does not depend on pppd and instead uses these 
syscalls. So long as the package is still being maintained, these 
syscalls should not be removed.

The state transition still needs to be fixed, though. It's done by the 
PPPIOCGCHAN ioctl. Note that despite the "Getter" semantic, it actually 
alters a socket's internal state by setting the PPPOX_BOUND bit, to 
indicate that the socket is now "bound" to the generic PPP layer. The 
code is carried over from the very first version of the PPPoX driver and 
the intention of this behavior was not known. A fix may move the 
transition to a non-getter ioctl and drain the socket receive queue once 
a socket is bound.

Best regards,
Qingfang

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC net-next] pppoe: remove session socket receive support
  2026-08-09 16:17   ` Qingfang Deng
@ 2026-08-09 22:59     ` Pali Rohár
  2026-08-10  2:28       ` Qingfang Deng
  0 siblings, 1 reply; 5+ messages in thread
From: Pali Rohár @ 2026-08-09 22:59 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Guillaume Nault, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Kees Cook, Asim Viladi Oglu Manizada,
	Eric Woudstra, Felix Fietkau, netdev, linux-kernel,
	Paul Mackerras, Michal Ostrowski

On Monday 10 August 2026 00:17:14 Qingfang Deng wrote:
> Hi Pali,
> 
> On 8/9/2026 10:58 PM, Pali Rohár wrote:
> > Hello, I would like to point out that "no know open source
> > userspace implementation" does not have to mean that there cannot be
> > some closed source or not publicly available. But I agree that
> > probability is too low.
> > 
> > Anyway, could you describe what is this functionality (which is being
> > removed) doing? Or how to use it? From the code and description I have
> > not figure out how it is suppose to be used or for what kind of scenario
> > or use case can be it hypothetically used.
> > 
> > Would be nice to know what exactly is being removing and what userspace
> > applications cannot do after applying this change.
> 
> pppoe_recvmsg()/sendmsg() can be used to receive/send PPPoE session packets
> without the generic layer. I just found out that the RP-PPPoE package,
> maintained by Dianne Skoll, actually provides a standalone PPPoE client that
> does not depend on pppd and instead uses these syscalls. So long as the
> package is still being maintained, these syscalls should not be removed.

Yes, that is right. I completely forgot that there are real userspace
implementations of PPPoE which are in use. RP-PPPoE is one of them.
RP-PPPoE provides plugin for pppd, but that plugin does not use kernel
PPPoE module.

> The state transition still needs to be fixed, though. It's done by the
> PPPIOCGCHAN ioctl. Note that despite the "Getter" semantic, it actually
> alters a socket's internal state by setting the PPPOX_BOUND bit, to indicate
> that the socket is now "bound" to the generic PPP layer. The code is carried
> over from the very first version of the PPPoX driver and the intention of
> this behavior was not known. A fix may move the transition to a non-getter
> ioctl and drain the socket receive queue once a socket is bound.
> 
> Best regards,
> Qingfang

I see. Maybe you could ask developers of the first version of driver if
they remember something. But probably nobody would remember old things.

Clearing the queue after the transition happens sounds like a good idea.
If I understand correctly, it does not make sense to get packets into
userspace once the socket is bound to PPP layer.

I have feeling that kernel PPP layer and its API do fully fit into the
API for network devices and netlink layer for userspace.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC net-next] pppoe: remove session socket receive support
  2026-08-09 22:59     ` Pali Rohár
@ 2026-08-10  2:28       ` Qingfang Deng
  0 siblings, 0 replies; 5+ messages in thread
From: Qingfang Deng @ 2026-08-10  2:28 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Guillaume Nault, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Kees Cook, Asim Viladi Oglu Manizada,
	Eric Woudstra, Felix Fietkau, netdev, linux-kernel,
	Paul Mackerras, Michal Ostrowski

On 2026/8/10 6:59, Pali Rohár wrote:
> I see. Maybe you could ask developers of the first version of driver if
> they remember something. But probably nobody would remember old things.

I wish I could, but the maintainer of PPPoX has gone AWOL for quite a 
long time.. (commit 2fd68c7ea2ae)

> Clearing the queue after the transition happens sounds like a good idea.
> If I understand correctly, it does not make sense to get packets into
> userspace once the socket is bound to PPP layer.
>
> I have feeling that kernel PPP layer and its API do fully fit into the
> API for network devices and netlink layer for userspace.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-10  2:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  9:16 [RFC net-next] pppoe: remove session socket receive support Qingfang Deng
2026-08-09 14:58 ` Pali Rohár
2026-08-09 16:17   ` Qingfang Deng
2026-08-09 22:59     ` Pali Rohár
2026-08-10  2:28       ` Qingfang Deng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox