Re: [PATCH] net/mlx5: set RSS key len 0 to indicate default RSS

[Ophir Munk <ophirmu@mellanox.com>]

> -----Original Message-----
> From: Shahaf Shuler
> Sent: Sunday, October 07, 2018 2:21 PM
> To: Yongseok Koh <yskoh@mellanox.com>; Ophir Munk
> <ophirmu@mellanox.com>
> Cc: dev@dpdk.org; Asaf Penso <asafp@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>
> Subject: RE: [PATCH] net/mlx5: set RSS key len 0 to indicate default RSS
> 
> > > On Oct 3, 2018, at 10:37 AM, Ophir Munk <ophirmu@mellanox.com>
> > wrote:
> > >
> > > Applications which add an RSS flow must supply an RSS key table and
> > > an RSS key length. If an application needs to add the default RSS
> > > flow it should not care about the exact RSS default key table and its
> length.
> > > By setting key length to 0 - the PMD will know that it should use
> > > the default RSS key table and length.
> 
> Can you refer where in the rte_flow API this is documented?  I couldn't find it
> under struct rte_flow_action_rss  definition.
> I agree it is better for the PMD to behave this way, but this should be
> explained in the API, so for this patch to be accepted we need also doc
> update in rte_flow.h.
> 

Will update documentation in v2

> In my opinion, it is better for the key to be NULL to request from the PMD to
> use the default key, similar to struct rte_eth_rss_conf.
> 

I suggest considering both (NULL key or key_len 0) as a criteria for using default RSS key.
In addition if type==0 it will be considered a default type and mlx5 will use ETH_RSS_IP
Will update in v2

> > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > b/drivers/net/mlx5/mlx5_flow.c index 3f548a9..18eacf5 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -2062,7 +2062,8 @@ struct mlx5_flow_tunnel_info {
> > >
> > > @@ -2106,7 +2107,7 @@ struct mlx5_flow_tunnel_info {
> > > 		memcpy((*flow->queue), rss->queue,
> > > 		       rss->queue_num * sizeof(uint16_t));
> > > 	flow->rss.queue_num = rss->queue_num;
> > > -	memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
> > > +	memcpy(flow->key, rss->key, rss->key_len);
> 
> In Mellanox devices 40B of a key is a must. If you intended to copy 0 bytes, it
> is better to do it in other ways.

rss->key_len can be 0 or 40. Please consider this memcpy() a generic code which fits both cases.

>