netdev - RE: [PATCH net-next 05/10] bnxt_en: Fix ethtool -l|-L inconsistent channel counts.

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

Message-ID: <BL2PR07MB2306246BCB25E3C323A3F3A88DF40@BL2PR07MB2306.namprd07.prod.outlook.com>
Date:   Mon, 19 Sep 2016 09:51:12 +0000
From:   "Mintz, Yuval" <Yuval.Mintz@...ium.com>
To:     Michael Chan <michael.chan@...adcom.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 05/10] bnxt_en: Fix ethtool -l|-L inconsistent
 channel counts.

> >> >> The existing code is inconsistent in reporting and accepting the
> >> >> combined channel count.  bnxt_get_channels() reports maximum
> >> >> combined as the maximum rx count.  bnxt_set_channels() accepts
> >> >> combined count that cannot be bigger than max rx or max tx.
> >> >>
> >> >> For example, if max rx = 2 and max tx = 1, we report max supported
> >> >> combined to be 2.  But if the user tries to set combined to 2, it
> >> >> will fail because 2 is bigger than max tx which is 1.
> >> >>
> >> >> Fix the code to be consistent.  Max allowed combined = max(max_rx,
> >> max_tx).
> >> >> We will accept a combined channel count <= max(max_rx, max_tx).
> >> >
> >> > Don't you mean the 'max allowed combined = min(max_rx, max_tx)'.
> >> > How does using 'max' change the faulty scenario you've described?
> >>
> >> I'm fixing the inconsistency described in the first 2 paragraphs.
> >> The driver logic allows a combined ring to be rx or tx only.  In the
> >> above example, we allow combined to be set to 2.  The 2nd combined ring
> supports rx only.
> >
> > Then what makes it a combined channel?
> > Sounds to me like in the above scenario you should have claimed support for:
> >   - max rx == 2
> >   - max tx == 1
> >   - max combined == 1
> 
> Our implementation will report:
> 
> max rx 2
> max tx 1
> max combined 2
> 
> If the user chooses 2 rx and 1 tx, he will use 3 msix vectors (3 completion rings,
> etc).  If the user chooses 2 combined (1 with rx/tx,
> 1 with rx only), he will use 2 msix vectors (2 completion rings, etc).
> With a large number of NPAR functions and SRIOV functions, the number of
> rings available per function may not be symmetrical.  We just want maximum
> flexibility to make use of all available resources in 2 different modes (one that
> uses more resources and one that uses less).

Sounds like the user should chose 1 combined in 1 rx in this scenario.