[dpdk-dev] [PATCH 0/5] fix Memory region lookups

[dpdk-dev] [PATCH 0/5] fix Memory region lookups

[Shahaf Shuler]

The series contains multiple small patches which address various of issues with
the Memory region lookup on the Transmit side.

Xueming Li
Shahaf Shuler (5):
  net/mlx5: fix Memory Region cache lookup
  net/mlx5: fix secondary process mempool registration
  net/mlx5: assert for un-successful memory registration
  net/mlx5: fix memory registration cache last index
  net/mlx5: fix Memory Region boundary checks

 doc/guides/nics/mlx5.rst     |  7 ++++++-
 drivers/net/mlx5/mlx5_rxtx.h | 28 ++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 7 deletions(-)

-- 
2.12.0

[dpdk-dev] [PATCH 1/5] net/mlx5: fix Memory Region cache lookup

[Shahaf Shuler]

The memory registration (MR) cache contains pointers to mlx5_mr.
The MR cache indexes are filled when a new MR is created. As it is
possible for MR to be created on the flight, an extra validation must be
added to avoid segmentation fault.

Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Cc: stable@dpdk.org
Cc: yskoh@mellanox.com

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 2eb2f0506..a63364d79 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -555,7 +555,8 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
 		return txq->mp2mr[i]->lkey;
 	for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
-		if (unlikely(txq->mp2mr[i]->mr == NULL)) {
+		if (unlikely(txq->mp2mr[i] == NULL ||
+		    txq->mp2mr[i]->mr == NULL)) {
 			/* Unknown MP, add a new MR for it. */
 			break;
 		}
-- 
2.12.0

[dpdk-dev] [PATCH 2/5] net/mlx5: fix secondary process mempool registration

[Shahaf Shuler]

Secondary process is not allowed to register mempools on the flight.

The code will return invalid memory key for such case.

Fixes: 87ec44ce1651 ("net/mlx5: add operations for secondary process")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 doc/guides/nics/mlx5.rst     |  7 ++++++-
 drivers/net/mlx5/mlx5_rxtx.h | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index bdc2216c0..2f860402f 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -106,7 +106,12 @@ Limitations
 
 - Inner RSS for VXLAN frames is not supported yet.
 - Hardware checksum RX offloads for VXLAN inner header are not supported yet.
-- Forked secondary process not supported.
+- For secondary process:
+
+  - Forked secondary process not supported.
+  - All mempools must be initialized before rte_eth_dev_start().
+  - Number of mempools must less than CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE
+
 - Flow pattern without any specific vlan will match for vlan packets as well:
 
   When VLAN spec is not specified in the pattern, the matching rule will be created with VLAN as a wild card.
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index a63364d79..79cdfc793 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -550,6 +550,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	uint16_t i = txq->mr_cache_idx;
 	uintptr_t addr = rte_pktmbuf_mtod(mb, uintptr_t);
 	struct mlx5_mr *mr;
+	struct rte_mempool *mp;
 
 	assert(i < RTE_DIM(txq->mp2mr));
 	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
@@ -563,14 +564,24 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 		if (txq->mp2mr[i]->start <= addr &&
 		    txq->mp2mr[i]->end >= addr) {
 			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
-			assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey) ==
-			       txq->mp2mr[i]->lkey);
+			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+				assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey)
+				       == txq->mp2mr[i]->lkey);
+			}
 			txq->mr_cache_idx = i;
 			return txq->mp2mr[i]->lkey;
 		}
 	}
 	txq->mr_cache_idx = 0;
-	mr = mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
+	mp = mlx5_tx_mb2mp(mb);
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		WARN("Using unregistered mempool 0x%p(%s) in secondary process,"
+		     " please create mempool before rte_eth_dev_start()",
+		     (void *)mp, mp->name);
+		assert(rte_eal_process_type() == RTE_PROC_PRIMARY);
+		return (uint32_t)-1;
+	}
+	mr = mlx5_txq_mp2mr_reg(txq, mp, i);
 	/*
 	 * Request the reference to use in this queue, the original one is
 	 * kept by the control plane.
-- 
2.12.0

Re: [dpdk-dev] [PATCH 2/5] net/mlx5: fix secondary process mempool registration

[Nélio Laranjeiro]

Hi Shahaf,

On Tue, Jan 23, 2018 at 07:08:20PM +0200, Shahaf Shuler wrote:
> Secondary process is not allowed to register mempools on the flight.
> 
> The code will return invalid memory key for such case.
> 
> Fixes: 87ec44ce1651 ("net/mlx5: add operations for secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>  doc/guides/nics/mlx5.rst     |  7 ++++++-
>  drivers/net/mlx5/mlx5_rxtx.h | 17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index bdc2216c0..2f860402f 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -106,7 +106,12 @@ Limitations
>  
>  - Inner RSS for VXLAN frames is not supported yet.
>  - Hardware checksum RX offloads for VXLAN inner header are not supported yet.
> -- Forked secondary process not supported.
> +- For secondary process:
> +
> +  - Forked secondary process not supported.
> +  - All mempools must be initialized before rte_eth_dev_start().
> +  - Number of mempools must less than CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE

Why such limitation?  Registering a new memory is independent of
searching a new one when the cache is too small.

>  - Flow pattern without any specific vlan will match for vlan packets as well:
>  
>    When VLAN spec is not specified in the pattern, the matching rule will be created with VLAN as a wild card.
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index a63364d79..79cdfc793 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -550,6 +550,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
>  	uint16_t i = txq->mr_cache_idx;
>  	uintptr_t addr = rte_pktmbuf_mtod(mb, uintptr_t);
>  	struct mlx5_mr *mr;
> +	struct rte_mempool *mp;
>  
>  	assert(i < RTE_DIM(txq->mp2mr));
>  	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
> @@ -563,14 +564,24 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
>  		if (txq->mp2mr[i]->start <= addr &&
>  		    txq->mp2mr[i]->end >= addr) {
>  			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
> -			assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey) ==
> -			       txq->mp2mr[i]->lkey);
> +			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +				assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey)
> +				       == txq->mp2mr[i]->lkey);
> +			}

This code should be inside priv_txq_mp2mr_reg() to let the secondary
search inside the MR list when the cache is too small.

If it does not find any MR it should fail before calling
priv_mr_new().

>  			txq->mr_cache_idx = i;
>  			return txq->mp2mr[i]->lkey;
>  		}
>  	}
>  	txq->mr_cache_idx = 0;
> -	mr = mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
> +	mp = mlx5_tx_mb2mp(mb);
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		WARN("Using unregistered mempool 0x%p(%s) in secondary process,"
> +		     " please create mempool before rte_eth_dev_start()",
> +		     (void *)mp, mp->name);
> +		assert(rte_eal_process_type() == RTE_PROC_PRIMARY);
> +		return (uint32_t)-1;
> +	}
> +	mr = mlx5_txq_mp2mr_reg(txq, mp, i);
>  	/*
>  	 * Request the reference to use in this queue, the original one is
>  	 * kept by the control plane.
> -- 
> 2.12.0

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH 2/5] net/mlx5: fix secondary process mempool registration

[Shahaf Shuler]

Wednesday, January 24, 2018 10:14 AM, Nélio Laranjeiro:
> Hi Shahaf,
> 
> On Tue, Jan 23, 2018 at 07:08:20PM +0200, Shahaf Shuler wrote:
> > Secondary process is not allowed to register mempools on the flight.
> >
> > The code will return invalid memory key for such case.
> >
> > Fixes: 87ec44ce1651 ("net/mlx5: add operations for secondary process")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > ---
> >  doc/guides/nics/mlx5.rst     |  7 ++++++-
> >  drivers/net/mlx5/mlx5_rxtx.h | 17 ++++++++++++++---
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index
> > bdc2216c0..2f860402f 100644
> > --- a/doc/guides/nics/mlx5.rst
> > +++ b/doc/guides/nics/mlx5.rst
> > @@ -106,7 +106,12 @@ Limitations
> >
> >  - Inner RSS for VXLAN frames is not supported yet.
> >  - Hardware checksum RX offloads for VXLAN inner header are not
> supported yet.
> > -- Forked secondary process not supported.
> > +- For secondary process:
> > +
> > +  - Forked secondary process not supported.
> > +  - All mempools must be initialized before rte_eth_dev_start().
> > +  - Number of mempools must less than
> > + CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE
> 
> Why such limitation?  Registering a new memory is independent of searching
> a new one when the cache is too small.
> 
> >  - Flow pattern without any specific vlan will match for vlan packets as well:
> >
> >    When VLAN spec is not specified in the pattern, the matching rule will be
> created with VLAN as a wild card.
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.h
> > b/drivers/net/mlx5/mlx5_rxtx.h index a63364d79..79cdfc793 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.h
> > +++ b/drivers/net/mlx5/mlx5_rxtx.h
> > @@ -550,6 +550,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct
> rte_mbuf *mb)
> >  	uint16_t i = txq->mr_cache_idx;
> >  	uintptr_t addr = rte_pktmbuf_mtod(mb, uintptr_t);
> >  	struct mlx5_mr *mr;
> > +	struct rte_mempool *mp;
> >
> >  	assert(i < RTE_DIM(txq->mp2mr));
> >  	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >=
> > addr)) @@ -563,14 +564,24 @@ mlx5_tx_mb2mr(struct mlx5_txq_data
> *txq, struct rte_mbuf *mb)
> >  		if (txq->mp2mr[i]->start <= addr &&
> >  		    txq->mp2mr[i]->end >= addr) {
> >  			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
> > -			assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey)
> ==
> > -			       txq->mp2mr[i]->lkey);
> > +			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > +				assert(rte_cpu_to_be_32(txq->mp2mr[i]-
> >mr->lkey)
> > +				       == txq->mp2mr[i]->lkey);
> > +			}
> 
> This code should be inside priv_txq_mp2mr_reg() to let the secondary
> search inside the MR list when the cache is too small.

You probably mean below code and not above. Yes good suggestion.
Regarding the above code - the assert in un-accessible from the secondary process as it tries to access ibv_mr.
I will add another patch to remove this assert completely (in order to save the comparison logic). 

> 
> If it does not find any MR it should fail before calling priv_mr_new().
> 
> >  			txq->mr_cache_idx = i;
> >  			return txq->mp2mr[i]->lkey;
> >  		}
> >  	}
> >  	txq->mr_cache_idx = 0;
> > -	mr = mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
> > +	mp = mlx5_tx_mb2mp(mb);
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +		WARN("Using unregistered mempool 0x%p(%s) in secondary
> process,"
> > +		     " please create mempool before rte_eth_dev_start()",
> > +		     (void *)mp, mp->name);
> > +		assert(rte_eal_process_type() == RTE_PROC_PRIMARY);
> > +		return (uint32_t)-1;
> > +	}
> > +	mr = mlx5_txq_mp2mr_reg(txq, mp, i);
> >  	/*
> >  	 * Request the reference to use in this queue, the original one is
> >  	 * kept by the control plane.
> > --
> > 2.12.0
> 
> --
> Nélio Laranjeiro
> 6WIND

[dpdk-dev] [PATCH 3/5] net/mlx5: assert for un-successful memory registration

[Shahaf Shuler]

Memory registration can fail, add the proper assert for such scenario
for it at least to be visible in debug mode.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 79cdfc793..2934f9fb3 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -589,6 +589,10 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	if (mr) {
 		rte_atomic32_inc(&mr->refcnt);
 		return mr->lkey;
+	} else {
+		WARN("Failed to register mempool 0x%p(%s)",
+		      (void *)mp, mp->name);
+		assert(mr != NULL);
 	}
 	return (uint32_t)-1;
 }
-- 
2.12.0

Re: [dpdk-dev] [PATCH 3/5] net/mlx5: assert for un-successful memory registration

[Nélio Laranjeiro]

Hi Shahaf,

On Tue, Jan 23, 2018 at 07:08:21PM +0200, Shahaf Shuler wrote:
> Memory registration can fail, add the proper assert for such scenario
> for it at least to be visible in debug mode.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_rxtx.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index 79cdfc793..2934f9fb3 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -589,6 +589,10 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
>  	if (mr) {
>  		rte_atomic32_inc(&mr->refcnt);
>  		return mr->lkey;
> +	} else {
> +		WARN("Failed to register mempool 0x%p(%s)",
> +		      (void *)mp, mp->name);
> +		assert(mr != NULL);

This assert seems wrong.

Why this assert, you don't trust the CPU to verify the pointer is NULL?

>  	}
>  	return (uint32_t)-1;
>  }
> -- 
> 2.12.0

Regards,

-- 
Nélio Laranjeiro
6WIND

[dpdk-dev] [PATCH 4/5] net/mlx5: fix memory registration cache last index

[Shahaf Shuler]

In case memory registration cache is full the new mempool will be
inserted in the last index of the array.

Update the last entry being hit to reflect it.

Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Cc: stable@dpdk.org
Cc: yskoh@mellanox.com

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 2934f9fb3..4bedfb89b 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -572,7 +572,6 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 			return txq->mp2mr[i]->lkey;
 		}
 	}
-	txq->mr_cache_idx = 0;
 	mp = mlx5_tx_mb2mp(mb);
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		WARN("Using unregistered mempool 0x%p(%s) in secondary process,"
@@ -588,6 +587,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	 */
 	if (mr) {
 		rte_atomic32_inc(&mr->refcnt);
+		txq->mr_cache_idx = i >= RTE_DIM(txq->mp2mr) ? i - 1 : i;
 		return mr->lkey;
 	} else {
 		WARN("Failed to register mempool 0x%p(%s)",
-- 
2.12.0

[dpdk-dev] [PATCH 5/5] net/mlx5: fix Memory Region boundary checks

[Shahaf Shuler]

Since commit f81ec748434b ("net/mlx5: fix memory region lookup") the
memory regions (MR) are no longer overlaps.

Comparing the end address of the MR should be exclusive, otherwise two
contiguous MRs may cause wrong matching.

Fixes: f81ec748434b ("net/mlx5: fix memory region lookup")
Cc: yskoh@mellanox.com

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 4bedfb89b..692069971 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -553,7 +553,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	struct rte_mempool *mp;
 
 	assert(i < RTE_DIM(txq->mp2mr));
-	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
+	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end > addr))
 		return txq->mp2mr[i]->lkey;
 	for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
 		if (unlikely(txq->mp2mr[i] == NULL ||
-- 
2.12.0

[dpdk-dev] [PATCH v2 0/6] fix Memory region lookups

[Shahaf Shuler]

The series contains multiple small patches which address various of issues with
the Memory region lookup on the Transmit side.

On v2:
 - Addressed Nelio's comments.
 - Extended the "fix Memory Region boundary checks" commit to cover
   identical cases.
 - Fixed the commits log.

Xueming Li
Shahaf Shuler (6):
  net/mlx5: fix Memory Region cache lookup
  net/mlx5: fix secondary process mempool registration
  net/mlx5: remove assert un-accessible from secondary process
  net/mlx5: warn for un-successful memory registration
  net/mlx5: fix Memory Region cache last index
  net/mlx5: fix Memory Region boundary checks

 doc/guides/nics/mlx5.rst     |  6 +++++-
 drivers/net/mlx5/mlx5_mr.c   | 12 ++++++++++--
 drivers/net/mlx5/mlx5_rxtx.h | 16 ++++++++++------
 3 files changed, 25 insertions(+), 9 deletions(-)

-- 
2.12.0

[dpdk-dev] [PATCH v2 1/6] net/mlx5: fix Memory Region cache lookup

[Shahaf Shuler]

The Memory Region (MR) cache contains pointers to mlx5_mr.
The MR cache indexes are filled when a new MR is created. As it is
possible for MR to be created on the flight, an extra validation must be
added to avoid segmentation fault.

Fixes: b0b093845793 ("net/mlx5: use buffer address for LKEY search")
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 2eb2f0506..a63364d79 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -555,7 +555,8 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
 		return txq->mp2mr[i]->lkey;
 	for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
-		if (unlikely(txq->mp2mr[i]->mr == NULL)) {
+		if (unlikely(txq->mp2mr[i] == NULL ||
+		    txq->mp2mr[i]->mr == NULL)) {
 			/* Unknown MP, add a new MR for it. */
 			break;
 		}
-- 
2.12.0

[dpdk-dev] [PATCH v2 2/6] net/mlx5: fix secondary process mempool registration

[Shahaf Shuler]

Secondary process is not allowed to register mempools on the flight.

The code will return invalid memory key for such case.

Fixes: 87ec44ce1651 ("net/mlx5: add operations for secondary process")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 doc/guides/nics/mlx5.rst   |  6 +++++-
 drivers/net/mlx5/mlx5_mr.c | 12 ++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index bdc2216c0..2e6d1e45a 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -106,7 +106,11 @@ Limitations
 
 - Inner RSS for VXLAN frames is not supported yet.
 - Hardware checksum RX offloads for VXLAN inner header are not supported yet.
-- Forked secondary process not supported.
+- For secondary process:
+
+  - Forked secondary process not supported.
+  - All mempools must be initialized before rte_eth_dev_start().
+
 - Flow pattern without any specific vlan will match for vlan packets as well:
 
   When VLAN spec is not specified in the pattern, the matching rule will be created with VLAN as a wild card.
diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index 2776dc700..cb625dc61 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -141,8 +141,16 @@ priv_txq_mp2mr_reg(struct priv *priv, struct mlx5_txq_data *txq,
 	DEBUG("%p: discovered new memory pool \"%s\" (%p)",
 	      (void *)txq_ctrl, mp->name, (void *)mp);
 	mr = priv_mr_get(priv, mp);
-	if (mr == NULL)
-		mr = priv_mr_new(priv, mp);
+	if (mr == NULL) {
+		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+			DEBUG("Using unregistered mempool 0x%p(%s) in secondary process,"
+			     " please create mempool before rte_eth_dev_start()",
+			     (void *)mp, mp->name);
+			return NULL;
+		} else {
+			mr = priv_mr_new(priv, mp);
+		}
+	}
 	if (unlikely(mr == NULL)) {
 		DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
 		      (void *)txq_ctrl);
-- 
2.12.0

[dpdk-dev] [PATCH v2 3/6] net/mlx5: remove assert un-accessible from secondary process

[Shahaf Shuler]

Verbs structs such as ibv_mr are not accessible from the secondary
process.

Choose to remove the assert in favor of performing more checks on the
critical data path.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index a63364d79..26be206a7 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -563,8 +563,6 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 		if (txq->mp2mr[i]->start <= addr &&
 		    txq->mp2mr[i]->end >= addr) {
 			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
-			assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey) ==
-			       txq->mp2mr[i]->lkey);
 			txq->mr_cache_idx = i;
 			return txq->mp2mr[i]->lkey;
 		}
-- 
2.12.0

[dpdk-dev] [PATCH v2 4/6] net/mlx5: warn for un-successful memory registration

[Shahaf Shuler]

Memory registration can fail, add the proper warning for such scenario
for it at least to be visible in debug mode.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 26be206a7..e6d163a59 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -576,6 +576,11 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	if (mr) {
 		rte_atomic32_inc(&mr->refcnt);
 		return mr->lkey;
+	} else {
+		struct rte_mempool *mp = mlx5_tx_mb2mp(mb);
+
+		WARN("Failed to register mempool 0x%p(%s)",
+		      (void *)mp, mp->name);
 	}
 	return (uint32_t)-1;
 }
-- 
2.12.0

[dpdk-dev] [PATCH v2 5/6] net/mlx5: fix Memory Region cache last index

[Shahaf Shuler]

In case Memory Region cache is full, the new mempool will be
inserted in the last index of the array.

Update the last entry being hit to reflect it.

Fixes: b0b093845793 ("net/mlx5: use buffer address for LKEY search")
Cc: stable@dpdk.org
Cc: yskoh@mellanox.com

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index e6d163a59..7e7db0e8c 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -567,7 +567,6 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 			return txq->mp2mr[i]->lkey;
 		}
 	}
-	txq->mr_cache_idx = 0;
 	mr = mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
 	/*
 	 * Request the reference to use in this queue, the original one is
@@ -575,6 +574,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	 */
 	if (mr) {
 		rte_atomic32_inc(&mr->refcnt);
+		txq->mr_cache_idx = i >= RTE_DIM(txq->mp2mr) ? i - 1 : i;
 		return mr->lkey;
 	} else {
 		struct rte_mempool *mp = mlx5_tx_mb2mp(mb);
-- 
2.12.0

[dpdk-dev] [PATCH v2 6/6] net/mlx5: fix Memory Region boundary checks

[Shahaf Shuler]

Since commit f81ec748434b ("net/mlx5: fix memory region lookup") the
Memory Region (MR) are no longer ovetrlaps.

Comparing the end address of the MR should be exclusive, otherwise two
contiguous MRs may cause wrong matching.

Fixes: f81ec748434b ("net/mlx5: fix memory region lookup")
Cc: yskoh@mellanox.com

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 7e7db0e8c..2919a747c 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -552,7 +552,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 	struct mlx5_mr *mr;
 
 	assert(i < RTE_DIM(txq->mp2mr));
-	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
+	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end > addr))
 		return txq->mp2mr[i]->lkey;
 	for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
 		if (unlikely(txq->mp2mr[i] == NULL ||
@@ -561,7 +561,7 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
 			break;
 		}
 		if (txq->mp2mr[i]->start <= addr &&
-		    txq->mp2mr[i]->end >= addr) {
+		    txq->mp2mr[i]->end > addr) {
 			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
 			txq->mr_cache_idx = i;
 			return txq->mp2mr[i]->lkey;
-- 
2.12.0

Re: [dpdk-dev] [PATCH v2 6/6] net/mlx5: fix Memory Region boundary checks

[Yongseok Koh]

> On Jan 25, 2018, at 8:18 AM, Shahaf Shuler <shahafs@mellanox.com> wrote:
> 
> Since commit f81ec748434b ("net/mlx5: fix memory region lookup") the
> Memory Region (MR) are no longer ovetrlaps.
> 
> Comparing the end address of the MR should be exclusive, otherwise two
> contiguous MRs may cause wrong matching.
> 
> Fixes: f81ec748434b ("net/mlx5: fix memory region lookup")
> Cc: yskoh@mellanox.com
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
For the series,

Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

Re: [dpdk-dev] [PATCH v2 0/6] fix Memory region lookups

[Shahaf Shuler]

Thursday, January 25, 2018 6:18 PM, Shahaf Shuler:
> Subject: [dpdk-dev] [PATCH v2 0/6] fix Memory region lookups
> 
> The series contains multiple small patches which address various of issues
> with the Memory region lookup on the Transmit side.
> 
> On v2:
>  - Addressed Nelio's comments.
>  - Extended the "fix Memory Region boundary checks" commit to cover
>    identical cases.
>  - Fixed the commits log.
> 
> Xueming Li
> Shahaf Shuler (6):
>   net/mlx5: fix Memory Region cache lookup
>   net/mlx5: fix secondary process mempool registration
>   net/mlx5: remove assert un-accessible from secondary process
>   net/mlx5: warn for un-successful memory registration
>   net/mlx5: fix Memory Region cache last index
>   net/mlx5: fix Memory Region boundary checks
> 
>  doc/guides/nics/mlx5.rst     |  6 +++++-
>  drivers/net/mlx5/mlx5_mr.c   | 12 ++++++++++--
>  drivers/net/mlx5/mlx5_rxtx.h | 16 ++++++++++------
>  3 files changed, 25 insertions(+), 9 deletions(-)

Series applied to next-net-mlx, thanks. 

> 
> --
> 2.12.0