From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754576AbcARKLZ (ORCPT ); Mon, 18 Jan 2016 05:11:25 -0500 Received: from mailout4.samsung.com ([203.254.224.34]:52406 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754444AbcARKLX (ORCPT ); Mon, 18 Jan 2016 05:11:23 -0500 X-AuditID: cbfee68d-f79646d000001355-70-569cba395bc4 Date: Mon, 18 Jan 2016 10:11:05 +0000 (GMT) From: Vaneet Narang Subject: Re: [PATCH] af_packet: Raw socket destruction warning fix To: Daniel Borkmann , Maninder Singh Cc: "davem@davemloft.net" , "willemb@google.com" , "edumazet@google.com" , "eyal.birger@gmail.com" , "tklauser@distanz.ch" , "fruggeri@aristanetworks.com" , "dwmw2@infradead.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , PANKAJ MISHRA , Geon-ho Kim , Hak-Bong Lee Reply-to: v.narang@samsung.com MIME-version: 1.0 X-MTR: 20160118100902189@v.narang Msgkey: 20160118100902189@v.narang X-EPLocale: en_US.windows-1252 X-Priority: 3 X-EPWebmail-Msg-Type: personal X-EPWebmail-Reply-Demand: 0 X-EPApproval-Locale: X-EPHeader: ML X-MLAttribute: X-RootMTR: 20160118100902189@v.narang X-ParentMTR: X-ArchiveUser: X-CPGSPASS: N X-ConfirmMail: N,general Content-type: text/plain; charset=windows-1252 MIME-version: 1.0 Message-id: <1385051583.176751453111860930.JavaMail.weblogic@ep2mlwas01a> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrGIsWRmVeSWpSXmKPExsWyRsSkStdy15wwg633dCwu75rD5sDo8XmT XABjFJdNSmpOZllqkb5dAlfGyrYFLAV7pCpmf1/E3sD4QLKLkZNDSEBZovPaNVYQW0LAROJ2 Vw+ULSZx4d56ti5GLqCapYwSDde3wBXda7vKDJGYwyjRcnQXC0iCRUBVYvr//8wgNpuAtsSb f71gcWEBJ4k33T/AmkUEoiSe9xxlAWlmFrjIIjHr53t2iDPkJP4u+AXWzCsgKHFy5hMWiG2K Equ/rGGDiCtJnHjfzQYRl5NYMvUyE4TNKzGj/SkLTHza1zXMELa0xPlZGxhh3ln8/TFUnF/i 2O0dUL0CElPPHISqUZO4P+88lM0nsWbhWxaY+l2nljPD7Lq/ZS5Ur4TE1pYnYI8xA905pfsh O4RtIHFk0RxWdL/wCnhKrOy7DdXbyyHx8JDIBEalWUjKZiEZNQvJKGQ1CxhZVjGKphYkFxQn pRcZ6hUn5haX5qXrJefnbmIEJofT/5717mC8fcD6EKMAB6MSD6/D2dlhQqyJZcWVuYcYTYHx NJFZSjQ5H5iC8kriDY3NjCxMTUyNjcwtzZTEeRWlfgYLCaQnlqRmp6YWpBbFF5XmpBYfYmTi 4JRqYPRm1viYb7dKaYvLkdv50etnVraLTgzPL7+8au0yV/HqQ9cWtzQIu8b9Ezf/4fSd20Zc rPPBs7gyn6+/rxscfPTXyeHa+RCzFOvfhltuL44UPrfr4pnMExr+xnedVgeJxMec6OXtmcER u9vpa0mB3nLJfzL/aibp8c9Z9WxbW5ta8uc9lsLP9imxFGckGmoxFxUnAgBJAqd5CQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBKsWRmVeSWpSXmKPExsVy+t/tPl3LXXPCDDbMVLa4vGsOmwOjx+dN cgGMUWk2GamJKalFCql5yfkpmXnptkrewfHO8aZmBoa6hpYW5koKeYm5qbZKLj4Bum6ZOUBD lRTKEnNKgUIBicXFSvp2NkX5pSWpChn5xSW2StGG5kZ6RgZ6pkZ6hqaxVoYGBkamQDUJaRkr 2xawFOyRqpj9fRF7A+MDyS5GTg4hAWWJzmvXWEFsCQETiXttV5khbDGJC/fWs3UxcgHVzGGU aDm6iwUkwSKgKjH9/3+wIjYBbYk3/3rB4sICThJvun+ADRIRiJJ43nOUBaSZWeAii8Ssn+/Z IbbJSfxd8AusmVdAUOLkzCcsENsUJVZ/WcMGEVeSOPG+mw0iLiexZOplJgibV2JG+1MWmPi0 r2ugLpWWOD9rAyPM1Yu/P4aK80scu70DqldAYuqZg1A1ahL3552Hsvkk1ix8ywJTv+vUcmaY Xfe3zIXqlZDY2vIE7DFmoDundD9kh7ANJI4smsOK7hdeAU+JlX23mSYwys5CkpqFpH0WknZk NQsYWVYxiqYWJBcUJ6VXGOoVJ+YWl+al6yXn525iBKeiZwt3MH45b32IUYCDUYmH1+Hs7DAh 1sSy4srcQ4wSHMxKIrzB6+eECfGmJFZWpRblxxeV5qQWH2I0BUbbRGYp0eR8YJrMK4k3NDYx NzU2tTAwNDc3UxLnvb3PL0xIID2xJDU7NbUgtQimj4mDU6qBsTX/B9uZGe/O6Rh+jkv4Of+D V/X58ubj6ttaE2oWWccH785oPb1U4UBzhbn5QukTn17v+OOxrObtZs2oL7N7XE5zu0bnrT3h sP7IynZFc6c/Ry4d3RFdZcuV1btEq8zC+eESqQDvluWHcl+c7RWbGLdbqzY/dvpEjv2+38/e TI06Ujpn3eFtrUosxRmJhlrMRcWJACYG2i9bAwAA DLP-Filter: Pass X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u0IABWw1009567 Hi, >> __do_softirq >> run_ksoftirqd >> >> Signed-off-by: Vaneet Narang >> Signed-off-by: Maninder Singh > Thanks for the fix. While it fixes the WARN_ON(), I believe some more > investigation is needed here on why it is happening: > > We call first into packet_release(), which removes the socket hook from > the kernel (unregister_prot_hook()), later calls synchronize_net() to > make sure no more skbs will come in. The receive queue is purged right > after the synchronize_net() already. > > packet_sock_destruct() will be called afterwards, when there are no more > refs on the socket anymore and no af_packet skbs in tx waiting for completion. > Only then, in sk_destruct(), we'll call into packet_sock_destruct(). > > So, eventually double purging the sk_receive_queue seems not the right > thing to do at first look, and w/o any deeper analysis in the commit description. > > Could you look a bit further into the issue? Do you have a reproducer to > trigger it? It is Suspend Resume scenario and in this case close(sock_id) is not called and hence packet_release is also not called. In case of suspend, driver power down its ethernet port and release all the sk_buff stored in RX and TX ring. driver calls dev_kfree_skb_any to release all the sk_buff in tx ring and if last tx buff of socket is called then packet_sock_destruct() will be invoked and will result in warning if and recevive sk_buff is still in receive queue. Driver calls dev_kfree_skb_any->dev_kfree_skb_irq and it adds buffer in completion queue to free and raises softirq NET_TX_SOFTIRQ net_tx_action->__kfree_skb->skb_release_all->skb_release_head_state->sock_wfree-> __sk_free->packet_sock_destruct Also purging of receive queue has been taken care in other protocols. // IP protocol void inet_sock_destruct(struct sock *sk) { struct inet_sock *inet = inet_sk(sk); __skb_queue_purge(&sk->sk_receive_queue); // Purge Receive queue __skb_queue_purge(&sk->sk_error_queue); .... WARN_ON(atomic_read(&sk->sk_rmem_alloc)); WARN_ON(atomic_read(&sk->sk_wmem_alloc)); } So i think it should be done in Raw sockets also. >> --- >> net/packet/af_packet.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c >> index 81b4b81..bcb37ba 100644 Thanks Vaneet Narang