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: <CAKErNvrfirDasvmDDbGDu=301tOE43RaTW9jVjBL8=pngPH6YQ@mail.gmail.com>
Date: Thu, 20 Mar 2025 09:43:39 +0200
From: Maxim Mikityanskiy <maxtram95@...il.com>
To: Tariq Toukan <ttoukan.linux@...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 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.

> > 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;
>