Re: [PATCH net] ppp: don't override sk->sk_state in pppoe_flush_dev()
From: Guillaume Nault <hidden>
Date: 2015-10-13 07:24:12
On Tue, Oct 13, 2015 at 05:13:54AM +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.
That's ok. I'll send an official patch when you consider the issue to be definitely fixed.