Re: [PATCH net-next 1/5] net: ethtool: only allow set_rxnfc with rss + ring_cookie if driver opts in

[Ahmed Zaki <ahmed.zaki@intel.com>]

On 2024-11-25 7:42 a.m., Edward Cree wrote:
> On 25/11/2024 14:20, Ahmed Zaki wrote:
>> On 2024-11-25 7:10 a.m., Gal Pressman wrote:
>>> On 25/11/2024 15:21, Edward Cree wrote:
>>>> Also, the check below it, dealing with sym-xor, looks like it's only
>>>>    relevant to ETHTOOL_SRXFH, since info.data is garbage for other commands.
>>>>    Ahmed, is my understanding correct there?
>>>>
>>>
>>> Speaking of the below check, the sanity check depends on the order of
>>> operations, for example:
>>> 1. Enable symmetric xor
>>> 2. Request hash on src only
>>> = Error as expected, however:
>>
>> Correct. The check below is to make sure that no ntuple that does not cover symmetric fields is added if symm-xor is enabled.
> But symm-xor is about hashing, and is only relevant to traffic being
>   directed by RSS.  The user should still be allowed to, and the NIC
>   should be able to handle, setting an ntuple filter (SRXCLSRLINS)
>   that is asymmetric, to override the symmetric hashing for selected
>   traffic.

I agree, and in its first version, the sym-xor series was setting 
sym-xor per ntuple, not per netdev. So the NIC can support different RSS 
functions for different filters. Unfortunately, this was then changed to 
be per-netdev during review. At that point, these checks were added in 
nxfc path.

> symm-xor should only constrain RXFH settings.  And in fact even if
>   you wanted to block asymm ntuple filters, the current code does not
>   do that, since the info.data fields it looks at aren't populated for
>   ntuple filters (whose filter fields are defined by info.fs).
> So the xfrm check should be under `if (info.cmd == ETHTOOL_SRXFH)`.

If it is not, then it is a bug. I will try to test later this week and 
send a fix if needed.