netdev - Re: [PATCH net] net/mlx5e: Fix ethtool -N flow-type ip4 to RSS context

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: <978c7574-b4d2-4b36-968c-d8d46c588964@gmail.com>
Date: Thu, 20 Mar 2025 10:09:04 +0200
From: Tariq Toukan <ttoukan.linux@...il.com>
To: Maxim Mikityanskiy <maxtram95@...il.com>
Cc: Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, Maxim Mikityanskiy <maxim@...valent.com>,
 Tariq Toukan <tariqt@...dia.com>
Subject: Re: [PATCH net] net/mlx5e: Fix ethtool -N flow-type ip4 to RSS
 context



On 20/03/2025 9:43, Maxim Mikityanskiy wrote:
> On Thu, 20 Mar 2025 at 09:32, Tariq Toukan <ttoukan.linux@...il.com> wrote:
>>
>>
>>
>> On 19/03/2025 14:45, Maxim Mikityanskiy wrote:
>>> There commands can be used to add an RSS context and steer some traffic
>>> into it:
>>>
>>>       # ethtool -X eth0 context new
>>>       New RSS context is 1
>>>       # ethtool -N eth0 flow-type ip4 dst-ip 1.1.1.1 context 1
>>>       Added rule with ID 1023
>>>
>>> However, the second command fails with EINVAL on mlx5e:
>>>
>>>       # ethtool -N eth0 flow-type ip4 dst-ip 1.1.1.1 context 1
>>>       rmgr: Cannot insert RX class rule: Invalid argument
>>>       Cannot insert classification rule
>>>
>>> It happens when flow_get_tirn calls flow_type_to_traffic_type with
>>> flow_type = IP_USER_FLOW or IPV6_USER_FLOW. That function only handles
>>> IPV4_FLOW and IPV6_FLOW cases, but unlike all other cases which are
>>> common for hash and spec, IPv4 and IPv6 defines different contants for
>>> hash and for spec:
>>>
>>>       #define  TCP_V4_FLOW     0x01    /* hash or spec (tcp_ip4_spec) */
>>>       #define  UDP_V4_FLOW     0x02    /* hash or spec (udp_ip4_spec) */
>>>       ...
>>>       #define  IPV4_USER_FLOW  0x0d    /* spec only (usr_ip4_spec) */
>>>       #define  IP_USER_FLOW    IPV4_USER_FLOW
>>>       #define  IPV6_USER_FLOW  0x0e    /* spec only (usr_ip6_spec; nfc only) */
>>>       #define  IPV4_FLOW       0x10    /* hash only */
>>>       #define  IPV6_FLOW       0x11    /* hash only */
>>>
>>> Extend the switch in flow_type_to_traffic_type to support both, which
>>> fixes the failing ethtool -N command with flow-type ip4 or ip6.
>>>
>>
>> Hi Maxim,
>> Thanks for your patch!
>>
>>> Fixes: 248d3b4c9a39 ("net/mlx5e: Support flow classification into RSS contexts")
>>
>> Seems that the issue originates in commit 756c41603a18 ("net/mlx5e:
>> ethtool, Support user configuration for RX hash fields"),
> 
> Not really; commit 756c41603a18 configures the hash (not flow
> direction), and IPV4_FLOW/IPV6_FLOW are already correct constants for
> IP-based hashes. Moreover, we don't support them anyway, see
> mlx5e_set_rss_hash_opt:
> 
>      /*  RSS does not support anything other than hashing to queues
>       *  on src IP, dest IP, TCP/UDP src port and TCP/UDP dest
>       *  port.
>       */
>      if (flow_type != TCP_V4_FLOW &&
>          flow_type != TCP_V6_FLOW &&
>          flow_type != UDP_V4_FLOW &&
>          flow_type != UDP_V6_FLOW)
>          return -EOPNOTSUPP;
> 
>> when directly
>> classifying into an RQ, before the multi RSS context support.
> 
> Direct classification into an RQ actually works before my fix, because
> it goes to another branch in flow_get_tirn, that doesn't call
> flow_type_to_traffic_type. It's only steering to an RSS context that
> was broken for flow-type ip4/ip6.
> 

Thanks for your patch.
LGTM.
Reviewed-by: Tariq Toukan <tariqt@...dia.com>

Regards,
Tariq

>>> Signed-off-by: Maxim Mikityanskiy <maxim@...valent.com>
>>> ---
>>>    drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
>>> index 773624bb2c5d..d68230a7b9f4 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
>>> @@ -884,8 +884,10 @@ static int flow_type_to_traffic_type(u32 flow_type)
>>>        case ESP_V6_FLOW:
>>>                return MLX5_TT_IPV6_IPSEC_ESP;
>>>        case IPV4_FLOW:
>>> +     case IP_USER_FLOW:
>>>                return MLX5_TT_IPV4;
>>>        case IPV6_FLOW:
>>> +     case IPV6_USER_FLOW:
>>>                return MLX5_TT_IPV6;
>>>        default:
>>>                return -EINVAL;
>>