netdev - Re: [PATCH]: Fix myri10ge NAPI oops & warnings

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:	Wed, 31 Oct 2007 18:54:00 -0400
From:	Andrew Gallatin <gallatin@...i.com>
To:	Stephen Hemminger <shemminger@...ux-foundation.org>
CC:	jeff@...zik.org, netdev@...r.kernel.org
Subject: Re: [PATCH]: Fix myri10ge NAPI oops & warnings

Stephen Hemminger wrote:
> On Wed, 31 Oct 2007 17:40:06 -0400
> Andrew Gallatin <gallatin@...i.com> wrote:
> 
>> When testing the myri10ge driver with 2.6.24-rc1, I found
>> that the machine crashed under heavy load:
>>
>> Unable to handle kernel paging request at 0000000000100108 RIP:
>>   [<ffffffff803cc8dd>] net_rx_action+0x11b/0x184
>>
>> The address corresponds to the list_move_tail() in
>> netif_rx_complete():
>>                      if (unlikely(work == weight))
>>                              list_move_tail(&n->poll_list, list);
>>
>> Eventually, I traced the crashes to calling netif_rx_complete() with
>> work_done == budget.  From looking at other drivers, it appears that
>> one should only call netif_rx_complete() when work_done < budget.
>>
>> To fix it, I changed the test in myri10ge_poll() so that it refers
>> to to work_done rather than looking at the rx ring status.  If
>> work_done is < budget, then that implies we have no more packets to
>> process. Any races will be resolved by the NIC when the write to
>> irq_claim is made.
>>
>> In myri10ge_clean_rx_done(), if we ever exceeded our budget, it would
>> report a work_done one larger than was acutally done.  This is because
>> the increment was done in the conditional, so work_done would be
>> incremented regardless of whether or not the test passed or failed.
>> This would lead to the WARN_ON_ONCE(work > weight); warning in
>> net_rx_action triggering.  I've moved the increment of work_done
>> inside the loop.  Note that this would only be a problem when we had
>> exceeded our budget.
>>
>> Signed off by: Andrew Gallatin <gallatin@...i.com>
>>
>> Andrew Gallatin Myricom Inc
>>
>>
> 
> Yes, this looks right.
> How could the check in netif_rx_complete be changed to catch this better?

I'm sorry, but I don't really know.  I'm afraid that I am
rather clueless about the new NAPI, and found this by
stumbling around in the dark. Naively, it seems like some
global option to override all napi weights (eg, to 1) would
be helpful so that if a driver had this problem, it could
be easily reproduced by the first packet received.  At least
I found setting our weight to one made the bug rather
obvious to me.

Drew


-
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