[dpdk-dev] [PATCH 0/3] net/mlx5: fix updating RETA

[dpdk-dev] [PATCH 0/3] net/mlx5: fix updating RETA

[Yongseok Koh]

Currently rte_eth_dev_rss_reta_update() doesn't work properly for mlx5 PMD. This
patchset fixes the issue. This also enables testing the API with testpmd.

Yongseok Koh (3):
  lib/librte_ether: remove requirement of aligned RETA size
  net/mlx5: use correct RETA table size
  net/mlx5: rebuild flows on updating RETA

 app/test-pmd/cmdline.c         |  4 +++-
 drivers/net/mlx5/mlx5_ethdev.c |  8 ++------
 drivers/net/mlx5/mlx5_rss.c    | 16 ++++++++--------
 lib/librte_ether/rte_ethdev.c  |  8 +-------
 4 files changed, 14 insertions(+), 22 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Yongseok Koh]

In rte_eth_check_reta_mask(), it is required to align the size of the RETA
table to RTE_RETA_GROUP_SIZE but as the size can be less than the limit,
this should be removed. The change is also applied to a command of testpmd.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 app/test-pmd/cmdline.c        | 4 +++-
 lib/librte_ether/rte_ethdev.c | 8 +-------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d20..463a77e5a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2063,7 +2063,9 @@ showport_parse_reta_config(struct rte_eth_rss_reta_entry64 *conf,
 	char s[256];
 	char *end;
 	char *str_fld[8];
-	uint16_t i, num = nb_entries / RTE_RETA_GROUP_SIZE;
+	uint16_t i;
+	uint16_t num = (nb_entries + RTE_RETA_GROUP_SIZE - 1) /
+			RTE_RETA_GROUP_SIZE;
 	int ret;
 
 	p = strchr(p0, '(');
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a9a..806fff6ec 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1935,13 +1935,7 @@ rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
 	if (!reta_conf)
 		return -EINVAL;
 
-	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
-		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u aligned\n",
-							RTE_RETA_GROUP_SIZE);
-		return -EINVAL;
-	}
-
-	num = reta_size / RTE_RETA_GROUP_SIZE;
+	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) / RTE_RETA_GROUP_SIZE;
 	for (i = 0; i < num; i++) {
 		if (reta_conf[i].mask)
 			return 0;
-- 
2.11.0

Re: [dpdk-dev] [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Yongseok Koh]

On Thu, Mar 16, 2017 at 03:40:54PM -0700, Yongseok Koh wrote:
> In rte_eth_check_reta_mask(), it is required to align the size of the RETA
> table to RTE_RETA_GROUP_SIZE but as the size can be less than the limit,
> this should be removed. The change is also applied to a command of testpmd.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  app/test-pmd/cmdline.c        | 4 +++-
>  lib/librte_ether/rte_ethdev.c | 8 +-------
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 47f935d20..463a77e5a 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2063,7 +2063,9 @@ showport_parse_reta_config(struct rte_eth_rss_reta_entry64 *conf,
>  	char s[256];
>  	char *end;
>  	char *str_fld[8];
> -	uint16_t i, num = nb_entries / RTE_RETA_GROUP_SIZE;
> +	uint16_t i;
> +	uint16_t num = (nb_entries + RTE_RETA_GROUP_SIZE - 1) /
> +			RTE_RETA_GROUP_SIZE;
>  	int ret;
>  
>  	p = strchr(p0, '(');
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index eb0a94a9a..806fff6ec 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1935,13 +1935,7 @@ rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
>  	if (!reta_conf)
>  		return -EINVAL;
>  
> -	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
> -		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u aligned\n",
> -							RTE_RETA_GROUP_SIZE);
> -		return -EINVAL;
> -	}
> -
> -	num = reta_size / RTE_RETA_GROUP_SIZE;
> +	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) / RTE_RETA_GROUP_SIZE;
>  	for (i = 0; i < num; i++) {
>  		if (reta_conf[i].mask)
>  			return 0;
> -- 
> 2.11.0
> 

I'm sorry, I forgot to include the maintainer of librte_ether, Thomas.

Thanks,
Yongseok

[dpdk-dev] [PATCH 2/3] net/mlx5: use correct RETA table size

[Yongseok Koh]

When querying and updating RSS RETA table, it always uses the max size of
the device instead of configured value. This patch fixes it and removed the
related comments in the code.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_ethdev.c |  8 ++------
 drivers/net/mlx5/mlx5_rss.c    | 13 +++++--------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index dd5fe5c1f..0578b11e4 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -701,12 +701,8 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 					  DEV_TX_OFFLOAD_GRE_TNL_TSO);
 	if (priv_get_ifname(priv, &ifname) == 0)
 		info->if_index = if_nametoindex(ifname);
-	/* FIXME: RETA update/query API expects the callee to know the size of
-	 * the indirection table, for this PMD the size varies depending on
-	 * the number of RX queues, it becomes impossible to find the correct
-	 * size if it is not fixed.
-	 * The API should be updated to solve this problem. */
-	info->reta_size = priv->ind_table_max_size;
+	info->reta_size = priv->reta_idx_n ?
+		priv->reta_idx_n : priv->ind_table_max_size;
 	info->hash_key_size = ((*priv->rss_conf) ?
 			       (*priv->rss_conf)[0]->rss_key_len :
 			       0);
diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index 0bed74eeb..0702f1a63 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -257,13 +257,9 @@ priv_dev_rss_reta_query(struct priv *priv,
 {
 	unsigned int idx;
 	unsigned int i;
-	int ret;
-
-	/* See RETA comment in mlx5_dev_infos_get(). */
-	ret = priv_rss_reta_index_resize(priv, priv->ind_table_max_size);
-	if (ret)
-		return ret;
 
+	if (!reta_size || reta_size > priv->reta_idx_n)
+		return EINVAL;
 	/* Fill each entry of the table even if its bit is not set. */
 	for (idx = 0, i = 0; (i != reta_size); ++i) {
 		idx = i / RTE_RETA_GROUP_SIZE;
@@ -296,8 +292,9 @@ priv_dev_rss_reta_update(struct priv *priv,
 	unsigned int pos;
 	int ret;
 
-	/* See RETA comment in mlx5_dev_infos_get(). */
-	ret = priv_rss_reta_index_resize(priv, priv->ind_table_max_size);
+	if (!reta_size)
+		return EINVAL;
+	ret = priv_rss_reta_index_resize(priv, reta_size);
 	if (ret)
 		return ret;
 
-- 
2.11.0

[dpdk-dev] [PATCH 3/3] net/mlx5: rebuild flows on updating RETA

[Yongseok Koh]

Currently mlx5_dev_rss_reta_update() just updates tables in the host,
therefore it isn't immediately effective until restarting the device by
calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
device side. This patch adds rebuilding the device-specific datastructure
and applying it to the device right away.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rss.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index 0702f1a63..30e59faa5 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
 	int ret;
 	struct priv *priv = dev->data->dev_private;
 
+	mlx5_dev_stop(dev);
 	priv_lock(priv);
 	ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
 	priv_unlock(priv);
+	if (!ret)
+		ret = (unsigned int)mlx5_dev_start(dev);
 	return -ret;
 }
-- 
2.11.0

Re: [dpdk-dev] [PATCH 3/3] net/mlx5: rebuild flows on updating RETA

[Nélio Laranjeiro]

On Thu, Mar 16, 2017 at 03:40:56PM -0700, Yongseok Koh wrote:
> Currently mlx5_dev_rss_reta_update() just updates tables in the host,
> therefore it isn't immediately effective until restarting the device by
> calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
> device side. This patch adds rebuilding the device-specific datastructure
> and applying it to the device right away.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_rss.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
> index 0702f1a63..30e59faa5 100644
> --- a/drivers/net/mlx5/mlx5_rss.c
> +++ b/drivers/net/mlx5/mlx5_rss.c
> @@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
>  	int ret;
>  	struct priv *priv = dev->data->dev_private;
>  
> +	mlx5_dev_stop(dev);
>  	priv_lock(priv);
>  	ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
>  	priv_unlock(priv);
> +	if (!ret)
> +		ret = (unsigned int)mlx5_dev_start(dev);
>  	return -ret;
>  }
> -- 
> 2.11.0
> 

Hi Yongseok,

I don't understand why you need the cast for the returned value of
mlx5_dev_start() as it already returns an int and your final variable is
also an int.

Thanks,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH 3/3] net/mlx5: rebuild flows on updating RETA

[Yongseok Koh]

Hi Nelio,

On Fri, Mar 17, 2017 at 02:11:43AM -0700, Nélio Laranjeiro wrote:
> On Thu, Mar 16, 2017 at 03:40:56PM -0700, Yongseok Koh wrote:
> > Currently mlx5_dev_rss_reta_update() just updates tables in the host,
> > therefore it isn't immediately effective until restarting the device by
> > calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
> > device side. This patch adds rebuilding the device-specific datastructure
> > and applying it to the device right away.
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_rss.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
> > index 0702f1a63..30e59faa5 100644
> > --- a/drivers/net/mlx5/mlx5_rss.c
> > +++ b/drivers/net/mlx5/mlx5_rss.c
> > @@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
> >        int ret;
> >        struct priv *priv = dev->data->dev_private;
> > 
> > +     mlx5_dev_stop(dev);
> >        priv_lock(priv);
> >        ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
> >        priv_unlock(priv);
> > +     if (!ret)
> > +             ret = (unsigned int)mlx5_dev_start(dev);
> >        return -ret;
> >  }
> > --
> > 2.11.0
> >
> 
> Hi Yongseok,
> 
> I don't understand why you need the cast for the returned value of
> mlx5_dev_start() as it already returns an int and your final variable is
> also an int.

For some reason, in mlx5 PMD code, priv_* calls return positive error numbers
and corresponding mlx5_* APIs return negative values by adding (-) sign the the
return value from priv_* calls.

To follow this tacit rule in the existing code, I casted the return value to
unsigned.

Thanks,
Yongseok

Re: [dpdk-dev] [PATCH 3/3] net/mlx5: rebuild flows on updating RETA

[Nélio Laranjeiro]

On Fri, Mar 17, 2017 at 10:14:56AM -0700, Yongseok Koh wrote:
> Hi Nelio,
> 
> On Fri, Mar 17, 2017 at 02:11:43AM -0700, Nélio Laranjeiro wrote:
> > On Thu, Mar 16, 2017 at 03:40:56PM -0700, Yongseok Koh wrote:
> > > Currently mlx5_dev_rss_reta_update() just updates tables in the host,
> > > therefore it isn't immediately effective until restarting the device by
> > > calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
> > > device side. This patch adds rebuilding the device-specific datastructure
> > > and applying it to the device right away.
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > >  drivers/net/mlx5/mlx5_rss.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
> > > index 0702f1a63..30e59faa5 100644
> > > --- a/drivers/net/mlx5/mlx5_rss.c
> > > +++ b/drivers/net/mlx5/mlx5_rss.c
> > > @@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
> > >        int ret;
> > >        struct priv *priv = dev->data->dev_private;
> > > 
> > > +     mlx5_dev_stop(dev);
> > >        priv_lock(priv);
> > >        ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
> > >        priv_unlock(priv);
> > > +     if (!ret)
> > > +             ret = (unsigned int)mlx5_dev_start(dev);
> > >        return -ret;
> > >  }
> > > --
> > > 2.11.0
> > >
> > 
> > Hi Yongseok,
> > 
> > I don't understand why you need the cast for the returned value of
> > mlx5_dev_start() as it already returns an int and your final variable is
> > also an int.
> 
> For some reason, in mlx5 PMD code, priv_* calls return positive error numbers
> and corresponding mlx5_* APIs return negative values by adding (-) sign the the
> return value from priv_* calls.

This is due to the IOCTL calls which returns only positives errno, we
decided to keep only positives errors internally and negate them in when
they are returned to DPDK.

> To follow this tacit rule in the existing code, I casted the return value to
> unsigned.

I see more this cast as something confusing than helping, knowing that
it could be avoided with something like:

 if (ret)
 	return -ret;
 return mlx5_dev_start(dev);

Regards,

-- 
Nélio Laranjeiro
6WIND

[dpdk-dev] [PATCH v2 0/3] net/mlx5: fix updating RETA

[Yongseok Koh]

Currently rte_eth_dev_rss_reta_update() doesn't work properly for mlx5 PMD. This
patchset fixes the issue. This also enables testing the API with testpmd.

v2:
* Minor code change by review.

Yongseok Koh (3):
  lib/librte_ether: remove requirement of aligned RETA size
  net/mlx5: use correct RETA table size
  net/mlx5: rebuild flows on updating RETA

 app/test-pmd/cmdline.c         |  4 +++-
 drivers/net/mlx5/mlx5_ethdev.c |  8 ++------
 drivers/net/mlx5/mlx5_rss.c    | 18 +++++++++---------
 lib/librte_ether/rte_ethdev.c  |  8 +-------
 4 files changed, 15 insertions(+), 23 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Yongseok Koh]

In rte_eth_check_reta_mask(), it is required to align the size of the RETA
table to RTE_RETA_GROUP_SIZE but as the size can be less than the limit,
this should be removed. The change is also applied to a command of testpmd.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 app/test-pmd/cmdline.c        | 4 +++-
 lib/librte_ether/rte_ethdev.c | 8 +-------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d20..463a77e5a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2063,7 +2063,9 @@ showport_parse_reta_config(struct rte_eth_rss_reta_entry64 *conf,
 	char s[256];
 	char *end;
 	char *str_fld[8];
-	uint16_t i, num = nb_entries / RTE_RETA_GROUP_SIZE;
+	uint16_t i;
+	uint16_t num = (nb_entries + RTE_RETA_GROUP_SIZE - 1) /
+			RTE_RETA_GROUP_SIZE;
 	int ret;
 
 	p = strchr(p0, '(');
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a9a..806fff6ec 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1935,13 +1935,7 @@ rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
 	if (!reta_conf)
 		return -EINVAL;
 
-	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
-		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u aligned\n",
-							RTE_RETA_GROUP_SIZE);
-		return -EINVAL;
-	}
-
-	num = reta_size / RTE_RETA_GROUP_SIZE;
+	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) / RTE_RETA_GROUP_SIZE;
 	for (i = 0; i < num; i++) {
 		if (reta_conf[i].mask)
 			return 0;
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Thomas Monjalon]

2017-03-20 16:04, Yongseok Koh:
> In rte_eth_check_reta_mask(), it is required to align the size of the RETA
> table to RTE_RETA_GROUP_SIZE but as the size can be less than the limit,
> this should be removed. The change is also applied to a command of testpmd.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
[...]
> -	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
> -		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u aligned\n",
> -							RTE_RETA_GROUP_SIZE);
> -		return -EINVAL;
> -	}
> -
> -	num = reta_size / RTE_RETA_GROUP_SIZE;
> +	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) / RTE_RETA_GROUP_SIZE;

There is no comment for this constraint neither in the code nor in the
commit: http://dpdk.org/commit/66c594904
So, I guess it can be removed.
If a check is needed, it could be added in the relevant drivers.

Helin, Konstantin, please check for Intel drivers.

Ferruh, please take care of this series.

Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Zhang, Helin]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, March 30, 2017 8:38 AM
> To: Yongseok Koh; Yigit, Ferruh; Zhang, Helin; Ananyev, Konstantin
> Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com
> Subject: Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned
> RETA size
> 
> 2017-03-20 16:04, Yongseok Koh:
> > In rte_eth_check_reta_mask(), it is required to align the size of the
> > RETA table to RTE_RETA_GROUP_SIZE but as the size can be less than the
> > limit, this should be removed. The change is also applied to a command of
> testpmd.
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> [...]
> > -	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
> > -		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u
> aligned\n",
> > -
> 	RTE_RETA_GROUP_SIZE);
> > -		return -EINVAL;
> > -	}
> > -
> > -	num = reta_size / RTE_RETA_GROUP_SIZE;
> > +	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) /
> RTE_RETA_GROUP_SIZE;
> 
> There is no comment for this constraint neither in the code nor in the
> commit: http://dpdk.org/commit/66c594904 So, I guess it can be removed.
> If a check is needed, it could be added in the relevant drivers.
> 
> Helin, Konstantin, please check for Intel drivers.
Hi Thomas

Thank you very much for the reminder!
We will check that and see if there is any impacts to Intel drivers.

Regards,
Helin
> 
> Ferruh, please take care of this series.

Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Lu, Wenzhuo]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Helin
> Sent: Friday, March 31, 2017 3:34 PM
> To: Thomas Monjalon; Yongseok Koh; Yigit, Ferruh; Ananyev, Konstantin
> Cc: dev@dpdk.org; adrien.mazarguil@6wind.com;
> nelio.laranjeiro@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement
> of aligned RETA size
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, March 30, 2017 8:38 AM
> > To: Yongseok Koh; Yigit, Ferruh; Zhang, Helin; Ananyev, Konstantin
> > Cc: dev@dpdk.org; adrien.mazarguil@6wind.com;
> > nelio.laranjeiro@6wind.com
> > Subject: Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of
> > aligned RETA size
> >
> > 2017-03-20 16:04, Yongseok Koh:
> > > In rte_eth_check_reta_mask(), it is required to align the size of
> > > the RETA table to RTE_RETA_GROUP_SIZE but as the size can be less
> > > than the limit, this should be removed. The change is also applied
> > > to a command of
> > testpmd.
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > [...]
> > > -	if (reta_size != RTE_ALIGN(reta_size, RTE_RETA_GROUP_SIZE)) {
> > > -		RTE_PMD_DEBUG_TRACE("Invalid reta size, should be %u
> > aligned\n",
> > > -
> > 	RTE_RETA_GROUP_SIZE);
> > > -		return -EINVAL;
> > > -	}
> > > -
> > > -	num = reta_size / RTE_RETA_GROUP_SIZE;
> > > +	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) /
> > RTE_RETA_GROUP_SIZE;
> >
> > There is no comment for this constraint neither in the code nor in the
> > commit: http://dpdk.org/commit/66c594904 So, I guess it can be removed.
> > If a check is needed, it could be added in the relevant drivers.
> >
> > Helin, Konstantin, please check for Intel drivers.
> Hi Thomas
> 
> Thank you very much for the reminder!
> We will check that and see if there is any impacts to Intel drivers.
I don't think it has any impact to the drivers.
To my opinion, it's a good fix as it makes the name ' reta_size' more reasonable.

> 
> Regards,
> Helin
> >
> > Ferruh, please take care of this series.

Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size

[Ferruh Yigit]

On 4/1/2017 8:28 AM, Lu, Wenzhuo wrote:
> Hi,
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Helin
<...>
>>> -----Original Message-----
>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
<...>
>>> 2017-03-20 16:04, Yongseok Koh:
>>>> In rte_eth_check_reta_mask(), it is required to align the size of
>>>> the RETA table to RTE_RETA_GROUP_SIZE but as the size can be less
>>>> than the limit, this should be removed. The change is also applied
>>>> to a command of
>>> testpmd.
>>>>
>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

<...>

>>> There is no comment for this constraint neither in the code nor in the
>>> commit: http://dpdk.org/commit/66c594904 So, I guess it can be removed.
>>> If a check is needed, it could be added in the relevant drivers.
>>>
>>> Helin, Konstantin, please check for Intel drivers.
>> Hi Thomas
>>
>> Thank you very much for the reminder!
>> We will check that and see if there is any impacts to Intel drivers.
> I don't think it has any impact to the drivers.
> To my opinion, it's a good fix as it makes the name ' reta_size' more reasonable.

Series applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v2 2/3] net/mlx5: use correct RETA table size

[Yongseok Koh]

When querying and updating RSS RETA table, it always uses the max size of
the device instead of configured value. This patch fixes it and removed the
related comments in the code.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_ethdev.c |  8 ++------
 drivers/net/mlx5/mlx5_rss.c    | 13 +++++--------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index dd5fe5c1f..0578b11e4 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -701,12 +701,8 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 					  DEV_TX_OFFLOAD_GRE_TNL_TSO);
 	if (priv_get_ifname(priv, &ifname) == 0)
 		info->if_index = if_nametoindex(ifname);
-	/* FIXME: RETA update/query API expects the callee to know the size of
-	 * the indirection table, for this PMD the size varies depending on
-	 * the number of RX queues, it becomes impossible to find the correct
-	 * size if it is not fixed.
-	 * The API should be updated to solve this problem. */
-	info->reta_size = priv->ind_table_max_size;
+	info->reta_size = priv->reta_idx_n ?
+		priv->reta_idx_n : priv->ind_table_max_size;
 	info->hash_key_size = ((*priv->rss_conf) ?
 			       (*priv->rss_conf)[0]->rss_key_len :
 			       0);
diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index 0bed74eeb..0702f1a63 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -257,13 +257,9 @@ priv_dev_rss_reta_query(struct priv *priv,
 {
 	unsigned int idx;
 	unsigned int i;
-	int ret;
-
-	/* See RETA comment in mlx5_dev_infos_get(). */
-	ret = priv_rss_reta_index_resize(priv, priv->ind_table_max_size);
-	if (ret)
-		return ret;
 
+	if (!reta_size || reta_size > priv->reta_idx_n)
+		return EINVAL;
 	/* Fill each entry of the table even if its bit is not set. */
 	for (idx = 0, i = 0; (i != reta_size); ++i) {
 		idx = i / RTE_RETA_GROUP_SIZE;
@@ -296,8 +292,9 @@ priv_dev_rss_reta_update(struct priv *priv,
 	unsigned int pos;
 	int ret;
 
-	/* See RETA comment in mlx5_dev_infos_get(). */
-	ret = priv_rss_reta_index_resize(priv, priv->ind_table_max_size);
+	if (!reta_size)
+		return EINVAL;
+	ret = priv_rss_reta_index_resize(priv, reta_size);
 	if (ret)
 		return ret;
 
-- 
2.11.0

[dpdk-dev] [PATCH v2 3/3] net/mlx5: rebuild flows on updating RETA

[Yongseok Koh]

Currently mlx5_dev_rss_reta_update() just updates tables in the host,
therefore it isn't immediately effective until restarting the device by
calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
device side. This patch adds rebuilding the device-specific datastructure
and applying it to the device right away.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rss.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index 0702f1a63..a2dd7d17c 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
 	int ret;
 	struct priv *priv = dev->data->dev_private;
 
+	mlx5_dev_stop(dev);
 	priv_lock(priv);
 	ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
 	priv_unlock(priv);
-	return -ret;
+	if (ret)
+		return -ret;
+	return mlx5_dev_start(dev);
 }
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 0/3] net/mlx5: fix updating RETA

[Nélio Laranjeiro]

On Mon, Mar 20, 2017 at 04:04:32PM -0700, Yongseok Koh wrote:
> Currently rte_eth_dev_rss_reta_update() doesn't work properly for mlx5 PMD. This
> patchset fixes the issue. This also enables testing the API with testpmd.
> 
> v2:
> * Minor code change by review.
> 
> Yongseok Koh (3):
>   lib/librte_ether: remove requirement of aligned RETA size
>   net/mlx5: use correct RETA table size
>   net/mlx5: rebuild flows on updating RETA
> 
>  app/test-pmd/cmdline.c         |  4 +++-
>  drivers/net/mlx5/mlx5_ethdev.c |  8 ++------
>  drivers/net/mlx5/mlx5_rss.c    | 18 +++++++++---------
>  lib/librte_ether/rte_ethdev.c  |  8 +-------
>  4 files changed, 15 insertions(+), 23 deletions(-)
> 
> -- 
> 2.11.0
> 

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

For the series, thanks,

-- 
Nélio Laranjeiro
6WIND