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

[Ophir Munk <ophirmu@mellanox.com>]

Please find comments inline.

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, October 03, 2018 9:56 PM
> To: Ophir Munk <ophirmu@mellanox.com>
> Cc: dev@dpdk.org; Asaf Penso <asafp@mellanox.com>; Shahaf Shuler
> <shahafs@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.
> >
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5_flow.c | 7 ++++---
> 
> Please rebase the code on top of the latest dpdk-next-net-mlx.
> Actually, it is better to rebase on top of PR#878.
> 
> Thanks,
> Yongseok

I have rebased the code on master branch dated 18.11 28-Oct-18.
After the rebase I have discovered more seg fault scenarios which are fixed in V2.

> 
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > 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 {
> >
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> > 					  &rss->level,
> > 					  "tunnel RSS is not supported");
> > -	if (rss->key_len < MLX5_RSS_HASH_KEY_LEN)
> > +	/* key_len 0 means using default RSS key */
> > +	if (rss->key_len > 0 && rss->key_len < MLX5_RSS_HASH_KEY_LEN)
> > 		return rte_flow_error_set(error, ENOTSUP,
> >
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> > 					  &rss->key_len,
> > @@ -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);
> > 	flow->rss.types = rss->types;
> > 	flow->rss.level = rss->level;
> > 	flow->fate |= MLX5_FLOW_FATE_RSS;
> > @@ -2948,7 +2949,7 @@ struct mlx5_flow_tunnel_info {
> > 					     flow->rss.queue_num);
> > 			if (!hrxq)
> > 				hrxq = mlx5_hrxq_new(dev, flow->key,
> > -
> MLX5_RSS_HASH_KEY_LEN,
> > +						     flow->rss.key_len,
> > 						     verbs->hash_fields,
> > 						     (*flow->queue),
> > 						     flow->rss.queue_num,
> > --
> > 1.8.3.1
> >