[dpdk-dev] net/mlx5: return RSS hash result in mbuf

  Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
  

On Wed, Sep 14, 2016 at 10:16:05AM +0200, Nelio Laranjeiro wrote:
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

> ---
>  drivers/net/mlx5/mlx5_rxq.c  | 1 +
>  drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
>  drivers/net/mlx5/mlx5_rxtx.h | 2 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index f6f4315..65c264b 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -926,6 +926,7 @@ rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl,
>  		.rxq = {
>  			.elts_n = log2above(desc),
>  			.mp = mp,
> +			.rss_hash = !!(priv->rxqs_n > 1),
>  		},
>  	};
>  	struct ibv_exp_wq_attr mod;
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index b91b644..17ae5e4 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1338,12 +1338,16 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			/* Update packet information. */
>  			pkt->packet_type = 0;
>  			pkt->ol_flags = 0;
> +			if (rxq->rss_hash) {
> +				pkt->hash.rss = ntohl(cqe->rx_hash_res);
> +				pkt->ol_flags = PKT_RX_RSS_HASH;
> +			}
>  			if (rxq->csum | rxq->csum_l2tun | rxq->vlan_strip |
>  			    rxq->crc_present) {
>  				if (rxq->csum) {
>  					pkt->packet_type =
>  						rxq_cq_to_pkt_type(cqe);
> -					pkt->ol_flags =
> +					pkt->ol_flags |=
>  						rxq_cq_to_ol_flags(rxq, cqe);
>  				}
>  				if (cqe->l4_hdr_type_etc &
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index 9828aef..e813f38 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -113,6 +113,8 @@ struct rxq {
>  	unsigned int cqe_n:4; /* Log 2 of CQ elements. */
>  	unsigned int elts_n:4; /* Log 2 of Mbufs. */
>  	unsigned int port_id:8;
> +	unsigned int rss_hash:1; /* RSS hash result is enabled. */
> +	unsigned int :9; /* Remaining bits. */
>  	volatile uint32_t *rq_db;
>  	volatile uint32_t *cq_db;
>  	uint16_t rq_ci;
> -- 
> 2.1.4
>
  

On Wed, Sep 14, 2016 at 10:16:05AM +0200, Nelio Laranjeiro wrote:
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_rxq.c  | 1 +
>  drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
>  drivers/net/mlx5/mlx5_rxtx.h | 2 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index f6f4315..65c264b 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -926,6 +926,7 @@ rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl,
>  		.rxq = {
>  			.elts_n = log2above(desc),
>  			.mp = mp,
> +			.rss_hash = !!(priv->rxqs_n > 1),
>  		},

Two comments on this:
1. I don't think the !! is needed, since this is a comparison which will result
   in 0 or 1, rather than an int value that needs to be clamped to those values.
2. Why limit the use of RSS to when there is more than one RX queues? Sometimes
   it might be useful to have a precomputed hash for a packet even when not
   spreading packets among RX queues. For example, when using a single RX queue,
   but doing a hash table lookup for each packet, the RSS value could be used
   as the lookup hash.

/Bruce
  

Hi Bruce,

On Tue, Sep 27, 2016 at 03:11:10PM +0100, Bruce Richardson wrote:
> On Wed, Sep 14, 2016 at 10:16:05AM +0200, Nelio Laranjeiro wrote:
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > ---
> >  drivers/net/mlx5/mlx5_rxq.c  | 1 +
> >  drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
> >  drivers/net/mlx5/mlx5_rxtx.h | 2 ++
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> > index f6f4315..65c264b 100644
> > --- a/drivers/net/mlx5/mlx5_rxq.c
> > +++ b/drivers/net/mlx5/mlx5_rxq.c
> > @@ -926,6 +926,7 @@ rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl,
> >  		.rxq = {
> >  			.elts_n = log2above(desc),
> >  			.mp = mp,
> > +			.rss_hash = !!(priv->rxqs_n > 1),
> >  		},
> 
> Two comments on this:
> 1. I don't think the !! is needed, since this is a comparison which will result
>    in 0 or 1, rather than an int value that needs to be clamped to those values.

Ok for this one.

> 2. Why limit the use of RSS to when there is more than one RX queues? Sometimes
>    it might be useful to have a precomputed hash for a packet even when not
>    spreading packets among RX queues. For example, when using a single RX queue,
>    but doing a hash table lookup for each packet, the RSS value could be used
>    as the lookup hash.
> 
> /Bruce

ConnectX-4 NICs does not return a hash value for a single queue as
software does not request RSS in this situation, that is why it is not
filled.

I will send a v2 to also fix a bug discovered since concerning the hash
result which is not retrieve from the right place when Completion Queue
Entry is compressed.

Regards,