[dpdk-dev] [PATCH v2 07/15] net/mlx5: support tunnel RSS level

[Nélio Laranjeiro]

On Mon, Apr 16, 2018 at 10:06:06AM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro at 6wind.com>
> > Sent: Monday, April 16, 2018 4:09 PM
> > To: Xueming(Steven) Li <xuemingl at mellanox.com>
> > Cc: Shahaf Shuler <shahafs at mellanox.com>; dev at dpdk.org
> > Subject: Re: [PATCH v2 07/15] net/mlx5: support tunnel RSS level
> > 
> > On Mon, Apr 16, 2018 at 07:46:08AM +0000, Xueming(Steven) Li wrote:
> > >[...]
> > > > > > > @@ -1386,6 +1386,8 @@ mlx5_ind_table_ibv_verify(struct
> > > > > > > rte_eth_dev
> > > > *dev)
> > > > > > >   *   Number of queues.
> > > > > > >   * @param tunnel
> > > > > > >   *   Tunnel type.
> > > > > > > + * @param rss_level
> > > > > > > + *   RSS hash on tunnel level.
> > > > > > >   *
> > > > > > >   * @return
> > > > > > >   *   The Verbs object initialised, NULL otherwise and rte_errno
> > is
> > > > set.
> > > > > > > @@ -1394,13 +1396,17 @@ struct mlx5_hrxq *
> > > > > > > mlx5_hrxq_new(struct rte_eth_dev *dev,
> > > > > > >  	      const uint8_t *rss_key, uint32_t rss_key_len,
> > > > > > >  	      uint64_t hash_fields,
> > > > > > > -	      const uint16_t *queues, uint32_t queues_n, uint32_t
> > > > tunnel)
> > > > > > > +	      const uint16_t *queues, uint32_t queues_n,
> > > > > > > +	      uint32_t tunnel, uint32_t rss_level)
> > > > > >
> > > > > > tunnel and rss_level seems to be redundant here.
> > > > > >
> > > > > > rss_level > 1 is equivalent to tunnel, there is no need to have
> > both.
> > > > >
> > > > > There is a case of tunnel and outer rss(1).
> > > >
> > > > Why cannot it be handled by a regular Hash Rx queue, i.e. what is
> > > > the benefit of creating a tunnel hash Rx queue to make the same job
> > > > as a legacy one?
> > >
> > > Tunnel checksum, ptype and rss offloading demand a QP to be created by
> > > DV api with tunnel offload flags.
> > 
> > I was expecting such answer, such information should be present in the
> > function documentation, can you add it?
> 
> You mean https://dpdk.org/doc/guides/nics/overview.html?
> "Inner L3 checksum" and "Inner L4 checksum" defined. 
> I added "Inner RSS" per your suggestion, The only thing missing is 
> "Innner packet type", make sense?

No I mean adding in this function doxygen documentation the fact than
tunnel is to have the checksum offload whereas the rss_level will be to
enable the RSS in the inner.

Thanks,

-- 
Nélio Laranjeiro
6WIND