netdev - Re: [PATCH net-next] ethtool: Fail number of channels change when it conflicts with rxnfc

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: <ceeadd10-51c4-9afb-58b4-ff113b8829b9@nvidia.com>
Date:   Wed, 2 Nov 2022 14:44:36 +0200
From:   Gal Pressman <gal@...dia.com>
To:     Jacob Keller <jacob.e.keller@...el.com>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Tariq Toukan <tariqt@...dia.com>
Subject: Re: [PATCH net-next] ethtool: Fail number of channels change when it
 conflicts with rxnfc

On 01/11/2022 18:50, Jacob Keller wrote:
>
>
> On 10/31/2022 6:23 PM, Jakub Kicinski wrote:
>> On Mon, 31 Oct 2022 12:00:16 +0200 Gal Pressman wrote:
>>> Similar to what we do with the hash indirection table [1], when network
>>> flow classification rules are forwarding traffic to channels greater
>>> than the requested number of channels, fail the operation.
>>> Without this, traffic could be directed to channels which no longer
>>> exist (dropped) after changing number of channels.
>>>
>>> [1] commit d4ab4286276f ("ethtool: correctly ensure {GS}CHANNELS
>>> doesn't conflict with GS{RXFH}")
>>
>> Have you made sure there are no magic encodings of queue numbers this
>> would break? I seem to recall some vendors used magic queue values to
>> redirect to VFs before TC and switchdev. If that's the case we'd need
>> to locate the drivers that do that and flag them so we can enforce this
>> only going forward?
>
> I believe these all use the same encoding defined by
> ethtool_get_flow_spec_ring and ethtool_get_flow_spec_vf, at least
> that's what ixgbe uses.
>
> This sets the lower 32 bits as the queue index and the next 8 bits as
> the VF identifier as defined by ETHTOOL_RX_FLOW_SPEC_RING and
> ETHTOOL_RX_FLOW_SPEC_RING_VF.
>
> It looks like this change should just exempt ring_cookie with
> ethtool_get_flow_spec_vf as non-zero?
>
> We maybe ought to mark this whole thing as deprecated now given the
> advances in TC.

Oh, I was not aware of this encoding scheme, shouldn't VF rules be added
on the VF interface?
What is this used for?

How does the PF verify the rules are in range for the VF queues?
Anyway, I'll go ahead and verify that VF == 0 in the if statement.

Thanks for the review!