Thread (20 messages) 20 messages, 4 authors, 2015-10-22

Re: [PATCH net] ppp: don't override sk->sk_state in pppoe_flush_dev()

From: Matt Bennett <hidden>
Date: 2015-10-22 00:15:12

On Tue, 2015-10-13 at 05:13 +0300, Denys Fedoryshchenko wrote:
On 2015-10-07 15:12, Guillaume Nault wrote:
quoted
On Mon, Oct 05, 2015 at 02:08:44PM +0200, Guillaume Nault wrote:
quoted
 	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 (!schedule_work(&po->proto.pppoe.padt_work))
 			sock_put(sk);
 	}
Finally, I think I'll keep this approach for net-next, to completely
remove PPPOX_ZOMBIE.
For now, let's just avoid any assumption about the relationship between
the PPPOX_ZOMBIE state and the value of po->pppoe_dev, as suggested by
Matt.

Denys, can you let me know if your issue goes away with the following
patch?
---
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 2ed7506..5e0b432 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -589,7 +589,7 @@ static int pppoe_release(struct socket *sock)

 	po = pppox_sk(sk);

-	if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND | PPPOX_ZOMBIE)) {
+	if (po->pppoe_dev) {
 		dev_put(po->pppoe_dev);
 		po->pppoe_dev = NULL;
 	}
I just got OK to upgrade server yesterday, for now around 12 hours 
working fine. I need 1-2 more days, and maybe will upgrade few more 
servers to say for sure, if it is ok or not.
Sorry for delay, just it is production servers and at current situation 
they cannot tolerate significant downtime.
Any update on whether this issue is fixed with the suggested patch?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help