netdev - Re: question: tg3 driver/nics and inconsistent RX ring count

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>] [day] [month] [year] [list]

Date:	Fri, 29 Jul 2016 16:07:48 +0530
From:	Siva Reddy Kallam <siva.kallam@...adcom.com>
To:	Michael Chan <michael.chan@...adcom.com>
Cc:	Michal Soltys <soltys@....info>,
	Alexander Duyck <alexander.duyck@...il.com>,
	Linux Netdev List <netdev@...r.kernel.org>,
	mcarlson@...adcom.com
Subject: Re: question: tg3 driver/nics and inconsistent RX ring count

On Wed, Jul 27, 2016 at 4:25 AM, Michael Chan <michael.chan@...adcom.com> wrote:
> On Tue, Jul 26, 2016 at 1:32 PM, Michal Soltys <soltys@....info> wrote:
>> On 2016-07-26 22:06, Alexander Duyck wrote:
>>> On Tue, Jul 26, 2016 at 12:52 PM, Michal Soltys <soltys@....info> wrote:
>>>> Hi,
>>>>
>>>> I have a few of BCM5720 and BCM5719 kinds sitting in Dell R320 and R520
>>>> servers - and all of them have certain peculiarity: they claim to have
>>>> up to 4 TX and RX rings (and this can be set/verified just fine through
>>>> ethtool -l/-L, with driver defaulting to 4 rings), indirection table
>>>> (ethtool -x) looks fine as well:
>>>>
>>>> RX flow hash indirection table for eth1b with 3 RX ring(s):
>>>>     0:      0     1     2     3     0     1     2     3
>>>>     8:      0     1     2     3     0     1     2     3
>>>> ......
>>>>
>>>> But this "3 RX ring(s)" is actually a real limit if I try to adjust
>>>> anything, for example all those commands would fail:
>>>>
>>>> ethtool -X eth1b equal 4
>>>> ethtool -X eth1b weight 1 1 1 1
>>>>
>>>> But would work fine for 3 and less rings. This was quickly tested with
>>>> different kernel/ethtool combinations, from old 3.16 to 4.6.y with same
>>>> exact results. Nothing fancier (-N/-U) is supported either.
>>>>
>>>> Any hints/comments about the cause of this and/or possible workarounds ?
>>>
>>> Well a quick look at the driver code makes it seem the problem lies in
>>> tg3_get_rxnfc.  I suspect the bug is related to the following lines:
>>>
>>>                 /* The first interrupt vector only
>>>                  * handles link interrupts.
>>>                  */
>>>                 info->data -= 1;
>>>
>>> I'm not sure what the number of interrupt vectors has to do with the
>>> number of rings.  Perhaps someone more familiar with the driver can
>>> point out why you would subtract 1 from tp->rxq_cnt to get the number
>>> of queues when it seems like it should be tp->rxq_cnt.
>>>
>>> Hope that helps.
>>>
>>> - Alex
>>>
>>
>> Ah thanks, seems to be the case then. Quick git blame shows it's been
>> since the very introduction of RSS indirection configurability (ca.
>> 2011) in this driver, 90415477bf1356f72acc34063ff52441fc10a754.
>>
>> I've CCed the author, maybe he will be able to shed some light.
>
> Matt is no longer working here.  The driver should support up to 5
> MSIX vectors and 4 RSS rings.  It looks like the code to subtract 1 in
> tg3_get_rxnfc() is not correct.  Siva will look further into this.
> Thanks for reporting the issue.
Yes, the code to subtract 1 in tg3_get_rxnfc looks incorrect. we will
upstream the patch for removing this code.