netdev - Re: [net-next 02/16] ixgbe: fix spinlock recursion with netpoll and busy poll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 14 Mar 2014 19:08:09 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	"Tantilov\, Emil S" <emil.s.tantilov@...el.com>
Cc:	David Miller <davem@...emloft.net>,
	"Kirsher\, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
	"gospo\@redhat.com" <gospo@...hat.com>,
	"sassmann\@redhat.com" <sassmann@...hat.com>,
	"asharma\@fb.com" <asharma@...com>,
	"stable\@vger.kernel.org" <stable@...r.kernel.org>,
	"Keller\, Jacob E" <jacob.e.keller@...el.com>
Subject: Re: [net-next 02/16] ixgbe: fix spinlock recursion with netpoll and busy poll

"Tantilov, Emil S" <emil.s.tantilov@...el.com> writes:

>>-----Original Message-----
>>From: David Miller [mailto:davem@...emloft.net]
>>Sent: Friday, March 14, 2014 11:59 AM
>>To: Kirsher, Jeffrey T
>>Cc: Tantilov, Emil S; netdev@...r.kernel.org;
>>gospo@...hat.com; sassmann@...hat.com; asharma@...com;
>>stable@...r.kernel.org; Keller, Jacob E
>>Subject: Re: [net-next 02/16] ixgbe: fix spinlock recursion
>>with netpoll and busy poll
>>
>>From: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
>>Date: Fri, 14 Mar 2014 02:47:12 -0700
>>
>>> From: Emil Tantilov <emil.s.tantilov@...el.com>
>>>
>>> This patch resolves a hang with busy poll when used with netconsole.
>>>
>>> The main change is the check for netpoll packets in ixgbe_poll() which
>>> prevents a call to spin_lock_bh() while interrupts are disabled.
>>>
>>> In addition it removes the call to netif_rx() since netif_receive_skb()
>>> can deal with netpoll packets and also replaced the global adapter flag
>>> with per-q_vector bool that indicates Rx packet from netpoll which
>>> should help with performance.
>>>
>>> CC: Arun Sharma <asharma@...com>
>>> CC: stable <stable@...r.kernel.org>
>>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>>> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
>>> Tested-by: Phil Schmitt <phillip.j.schmitt@...el.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
>>
>>Eric Biederman and co. are working to make this completely unnecessary.
>>
>>What is going to happen is that poll will be called with a budget of
>>zero, and no RX processing will thus occur, for netpoll.
>>
>>Nothing in the RX path can handle being invoked from hardware
>>interrupt context, so we have to prevent it completely in all
>>drivers that support netpoll.
>
> I noticed Eric's patches and this is obviously the better way to deal
> with it, but once the netpoll_rx patches are accepted will they make
> it into previous stable releases? Currently kernels between 3.11 and
> 3.13 (assuming Eric's changes make it into 3.14) will hang when
> netpoll and busy poll are used together.

A quick comment on this patch.  IXGBE_FLAG_IN_NETPOLL in ixgbe (before
your refinement) is harmless but complete nonsense.  The napi poll
methods do not get called from ndo_poll_controller.  So the call of
netif_rx is bunk.

Before my patches go in netpoll will cause any packets processed with
napi_gro_receive or netif_receive_skb while netconsole is sending a
packet to be dropped, so netif_rx (which is not called) should be
unnecessary.

As for the netpoll hang taking the lock. I expect that the fix for
handling receving packets with netpoll is simply to disable interrupts
in the ixgbe_qv_lock_ family of functions, so you can handle netpoll
from hard irq context.

Of course that won't be needed as soon David merges my patches and but shrug.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html