[PATCH net-next 0/2] ppp: Remove PPPOX_ZOMBIE socket state

[PATCH net-next 0/2] ppp: Remove PPPOX_ZOMBIE socket state

[Guillaume Nault]

Several issues have been found lately wrt. the PPPOX_ZOMBIE socket
state. This state is now only set upon reception of a PADT to stop
further transmissions. However this is redundant with the PADT
workqueue mechanism introduced by 287f3a943fef ("pppoe: Use workqueue
to die properly when a PADT is received").

We can thus simplify pppox socket state handling by getting rid of
PPPOX_ZOMBIE entirely.


Guillaume Nault (2):
  ppp: don't set sk_state to PPPOX_ZOMBIE in pppoe_disc_rcv()
  ppp: remove PPPOX_ZOMBIE socket state

 drivers/net/ppp/pppoe.c  | 26 ++++----------------------
 drivers/net/ppp/pppox.c  |  2 +-
 include/linux/if_pppox.h |  1 -
 3 files changed, 5 insertions(+), 24 deletions(-)

-- 
2.6.2

[PATCH net-next 1/2] ppp: don't set sk_state to PPPOX_ZOMBIE in pppoe_disc_rcv()

[Guillaume Nault]

Since 287f3a943fef ("pppoe: Use workqueue to die properly when a PADT
is received"), pppoe_disc_rcv() disconnects the socket by scheduling
pppoe_unbind_sock_work(). This is enough to stop socket transmission
and makes the PPPOX_ZOMBIE state uncessary.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 drivers/net/ppp/pppoe.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 5e0b432..1dedfbf 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -500,27 +500,9 @@ static int pppoe_disc_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	pn = pppoe_pernet(dev_net(dev));
 	po = get_item(pn, ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
-	if (po) {
-		struct sock *sk = sk_pppox(po);
-
-		bh_lock_sock(sk);
-
-		/* If the user has locked the socket, just ignore
-		 * the packet.  With the way two rcv protocols hook into
-		 * one socket family type, we cannot (easily) distinguish
-		 * what kind of SKB it is during backlog rcv.
-		 */
-		if (sock_owned_by_user(sk) == 0) {
-			/* We're no longer connect at the PPPOE layer,
-			 * and must wait for ppp channel to disconnect us.
-			 */
-			sk->sk_state = PPPOX_ZOMBIE;
-		}
-
-		bh_unlock_sock(sk);
+	if (po)
 		if (!schedule_work(&po->proto.pppoe.padt_work))
-			sock_put(sk);
-	}
+			sock_put(sk_pppox(po));
 
 abort:
 	kfree_skb(skb);
-- 
2.6.2

[PATCH net-next 2/2] ppp: remove PPPOX_ZOMBIE socket state

[Guillaume Nault]

PPPOX_ZOMBIE is never set anymore.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 drivers/net/ppp/pppoe.c  | 4 ++--
 drivers/net/ppp/pppox.c  | 2 +-
 include/linux/if_pppox.h | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 1dedfbf..277e682 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -311,7 +311,7 @@ static void pppoe_flush_dev(struct net_device *dev)
 			lock_sock(sk);
 
 			if (po->pppoe_dev == dev &&
-			    sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND | PPPOX_ZOMBIE)) {
+			    sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
 				pppox_unbind_sock(sk);
 				sk->sk_state_change(sk);
 				po->pppoe_dev = NULL;
@@ -775,7 +775,7 @@ static int pppoe_ioctl(struct socket *sock, unsigned int cmd,
 		struct pppox_sock *relay_po;
 
 		err = -EBUSY;
-		if (sk->sk_state & (PPPOX_BOUND | PPPOX_ZOMBIE | PPPOX_DEAD))
+		if (sk->sk_state & (PPPOX_BOUND | PPPOX_DEAD))
 			break;
 
 		err = -ENOTCONN;
diff --git a/drivers/net/ppp/pppox.c b/drivers/net/ppp/pppox.c
index 0e1b306..0200de7 100644
--- a/drivers/net/ppp/pppox.c
+++ b/drivers/net/ppp/pppox.c
@@ -58,7 +58,7 @@ void pppox_unbind_sock(struct sock *sk)
 {
 	/* Clear connection to ppp device, if attached. */
 
-	if (sk->sk_state & (PPPOX_BOUND | PPPOX_CONNECTED | PPPOX_ZOMBIE)) {
+	if (sk->sk_state & (PPPOX_BOUND | PPPOX_CONNECTED)) {
 		ppp_unregister_channel(&pppox_sk(sk)->chan);
 		sk->sk_state = PPPOX_DEAD;
 	}
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index b49cf92..ba7a9b0 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -91,7 +91,6 @@ enum {
     PPPOX_CONNECTED	= 1,  /* connection established ==TCP_ESTABLISHED */
     PPPOX_BOUND		= 2,  /* bound to ppp device */
     PPPOX_RELAY		= 4,  /* forwarding is enabled */
-    PPPOX_ZOMBIE	= 8,  /* dead, but still bound to ppp device */
     PPPOX_DEAD		= 16  /* dead, useless, please clean me up!*/
 };
 
-- 
2.6.2

Re: [PATCH net-next 0/2] ppp: Remove PPPOX_ZOMBIE socket state

[David Miller]

From: Guillaume Nault <g.nault@alphalink.fr>
Date: Thu, 19 Nov 2015 12:52:30 +0100

> Several issues have been found lately wrt. the PPPOX_ZOMBIE socket
> state. This state is now only set upon reception of a PADT to stop
> further transmissions. However this is redundant with the PADT
> workqueue mechanism introduced by 287f3a943fef ("pppoe: Use workqueue
> to die properly when a PADT is received").
> 
> We can thus simplify pppox socket state handling by getting rid of
> PPPOX_ZOMBIE entirely.

Nice, applied to net-next, thanks!