netdev - Re: [PATCH net-next 08/12] amd-xgbe: Add ethtool show/set channels support

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: <6346850c-ea57-ef4e-db0a-78d29fd360b3@amd.com>
Date:   Tue, 22 May 2018 08:24:09 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Jakub Kicinski <kubakici@...pl>
Cc:     netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: [PATCH net-next 08/12] amd-xgbe: Add ethtool show/set channels
 support

On 5/22/2018 12:35 AM, Jakub Kicinski wrote:
> On Mon, 21 May 2018 16:59:37 -0500, Tom Lendacky wrote:
>> +	rx = combined + channels->rx_count;
>> +	tx = combined + channels->tx_count;
>> +	netdev_notice(netdev, "final channel count assignment: combined=%u, rx-only=%u, tx-only=%u\n",
>> +		      min(rx, tx), rx - min(rx, tx), tx - min(rx, tx));
> 
> If user requests combined 0 rx 8 tx 8 they will end up with combined 8
> rx 0 tx 0.  Is that expected?

Yes, which is the reason that I issue the final channel count message. I
debated on how to do all this and looked at other drivers as well as the
ethtool man page and decided on this logic.

> 
> The man page clearly sayeth:
> 
>        -L --set-channels
>               Changes the numbers of channels of the specified network device.
> 
>            rx N   Changes the number of channels with only receive queues.
> 
>            tx N   Changes the number of channels with only transmit queues.
> 
>            other N
>                   Changes the number of channels used only for other  purposes
>                   e.g. link interrupts or SR-IOV co-ordination.
> 
>            combined N
>                   Changes the number of multi-purpose channels.
> 
> Note the use of word *only*.  There are drivers in tree which adhere to
> this interpretation and dutifully allocate separate IRQs if RX and TX
> channels are requested separately.

The amd-xgbe driver is not designed to allocate separate IRQs for Rx and
Tx.  In general, there is one IRQ for a channel of which Tx and Rx are
shared.  You can have more Tx channels than Rx channels or vice-versa, but
the min() of those numbers will be a combined Tx/Rx with the excess being
Tx or Rx only: e.g. combined 0 tx 8 rx 10 results in 8 combined channels
plus two Rx only channels.

I thought this was the most reasonable way to do this, please let me know
if there's a strong objection to this.

Thanks,
Tom

> 
> Which is not to claim that majority of existing drivers adhere to this
> wording :)
>