* [PATCH 0/3] net/mlx5: fix updating RETA
@ 2017-03-16 22:40 Yongseok Koh
2017-03-16 22:40 ` [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Yongseok Koh @ 2017-03-16 22:40 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-16 22:40 [PATCH 0/3] net/mlx5: fix updating RETA Yongseok Koh @ 2017-03-16 22:40 ` Yongseok Koh 2017-03-17 17:19 ` Yongseok Koh 2017-03-16 22:40 ` [PATCH 2/3] net/mlx5: use correct RETA table size Yongseok Koh ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Yongseok Koh @ 2017-03-16 22:40 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-16 22:40 ` [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh @ 2017-03-17 17:19 ` Yongseok Koh 0 siblings, 0 replies; 17+ messages in thread From: Yongseok Koh @ 2017-03-17 17:19 UTC (permalink / raw) To: ferruh.yigit, thomas.monjalon; +Cc: dev, adrien.mazarguil, nelio.laranjeiro 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] net/mlx5: use correct RETA table size 2017-03-16 22:40 [PATCH 0/3] net/mlx5: fix updating RETA Yongseok Koh 2017-03-16 22:40 ` [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh @ 2017-03-16 22:40 ` Yongseok Koh 2017-03-16 22:40 ` [PATCH 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh 3 siblings, 0 replies; 17+ messages in thread From: Yongseok Koh @ 2017-03-16 22:40 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] net/mlx5: rebuild flows on updating RETA 2017-03-16 22:40 [PATCH 0/3] net/mlx5: fix updating RETA Yongseok Koh 2017-03-16 22:40 ` [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh 2017-03-16 22:40 ` [PATCH 2/3] net/mlx5: use correct RETA table size Yongseok Koh @ 2017-03-16 22:40 ` Yongseok Koh 2017-03-17 9:11 ` Nélio Laranjeiro 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh 3 siblings, 1 reply; 17+ messages in thread From: Yongseok Koh @ 2017-03-16 22:40 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] net/mlx5: rebuild flows on updating RETA 2017-03-16 22:40 ` [PATCH 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh @ 2017-03-17 9:11 ` Nélio Laranjeiro 2017-03-17 17:14 ` Yongseok Koh 0 siblings, 1 reply; 17+ messages in thread From: Nélio Laranjeiro @ 2017-03-17 9:11 UTC (permalink / raw) To: Yongseok Koh; +Cc: ferruh.yigit, dev, adrien.mazarguil 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] net/mlx5: rebuild flows on updating RETA 2017-03-17 9:11 ` Nélio Laranjeiro @ 2017-03-17 17:14 ` Yongseok Koh 2017-03-20 7:56 ` Nélio Laranjeiro 0 siblings, 1 reply; 17+ messages in thread From: Yongseok Koh @ 2017-03-17 17:14 UTC (permalink / raw) To: Nélio Laranjeiro; +Cc: ferruh.yigit, dev, Adrien Mazarguil 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] net/mlx5: rebuild flows on updating RETA 2017-03-17 17:14 ` Yongseok Koh @ 2017-03-20 7:56 ` Nélio Laranjeiro 0 siblings, 0 replies; 17+ messages in thread From: Nélio Laranjeiro @ 2017-03-20 7:56 UTC (permalink / raw) To: Yongseok Koh; +Cc: ferruh.yigit, dev, Adrien Mazarguil 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/3] net/mlx5: fix updating RETA 2017-03-16 22:40 [PATCH 0/3] net/mlx5: fix updating RETA Yongseok Koh ` (2 preceding siblings ...) 2017-03-16 22:40 ` [PATCH 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh @ 2017-03-20 23:04 ` Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh ` (3 more replies) 3 siblings, 4 replies; 17+ messages in thread From: Yongseok Koh @ 2017-03-20 23:04 UTC (permalink / raw) To: ferruh.yigit, thomas.monjalon Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh @ 2017-03-20 23:04 ` Yongseok Koh 2017-03-30 0:37 ` Thomas Monjalon 2017-03-20 23:04 ` [PATCH v2 2/3] net/mlx5: use correct RETA table size Yongseok Koh ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Yongseok Koh @ 2017-03-20 23:04 UTC (permalink / raw) To: ferruh.yigit, thomas.monjalon Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-20 23:04 ` [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh @ 2017-03-30 0:37 ` Thomas Monjalon 2017-03-31 7:33 ` Zhang, Helin 0 siblings, 1 reply; 17+ messages in thread From: Thomas Monjalon @ 2017-03-30 0:37 UTC (permalink / raw) To: Yongseok Koh, ferruh.yigit, helin.zhang, konstantin.ananyev Cc: dev, adrien.mazarguil, nelio.laranjeiro 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-30 0:37 ` Thomas Monjalon @ 2017-03-31 7:33 ` Zhang, Helin 2017-04-01 7:28 ` Lu, Wenzhuo 0 siblings, 1 reply; 17+ messages in thread From: Zhang, Helin @ 2017-03-31 7:33 UTC (permalink / raw) To: Thomas Monjalon, Yongseok Koh, Yigit, Ferruh, Ananyev, Konstantin Cc: dev, adrien.mazarguil, nelio.laranjeiro > -----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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-03-31 7:33 ` Zhang, Helin @ 2017-04-01 7:28 ` Lu, Wenzhuo 2017-04-03 10:02 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Lu, Wenzhuo @ 2017-04-01 7:28 UTC (permalink / raw) To: Zhang, Helin, Thomas Monjalon, Yongseok Koh, Yigit, Ferruh, Ananyev, Konstantin Cc: dev, adrien.mazarguil, nelio.laranjeiro 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size 2017-04-01 7:28 ` Lu, Wenzhuo @ 2017-04-03 10:02 ` Ferruh Yigit 0 siblings, 0 replies; 17+ messages in thread From: Ferruh Yigit @ 2017-04-03 10:02 UTC (permalink / raw) To: Lu, Wenzhuo, Zhang, Helin, Thomas Monjalon, Yongseok Koh, Ananyev, Konstantin Cc: dev, adrien.mazarguil, nelio.laranjeiro 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/3] net/mlx5: use correct RETA table size 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh @ 2017-03-20 23:04 ` Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh 2017-03-21 7:45 ` [PATCH v2 0/3] net/mlx5: fix " Nélio Laranjeiro 3 siblings, 0 replies; 17+ messages in thread From: Yongseok Koh @ 2017-03-20 23:04 UTC (permalink / raw) To: ferruh.yigit, thomas.monjalon Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/3] net/mlx5: rebuild flows on updating RETA 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 2/3] net/mlx5: use correct RETA table size Yongseok Koh @ 2017-03-20 23:04 ` Yongseok Koh 2017-03-21 7:45 ` [PATCH v2 0/3] net/mlx5: fix " Nélio Laranjeiro 3 siblings, 0 replies; 17+ messages in thread From: Yongseok Koh @ 2017-03-20 23:04 UTC (permalink / raw) To: ferruh.yigit, thomas.monjalon Cc: dev, adrien.mazarguil, nelio.laranjeiro, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/3] net/mlx5: fix updating RETA 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh ` (2 preceding siblings ...) 2017-03-20 23:04 ` [PATCH v2 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh @ 2017-03-21 7:45 ` Nélio Laranjeiro 3 siblings, 0 replies; 17+ messages in thread From: Nélio Laranjeiro @ 2017-03-21 7:45 UTC (permalink / raw) To: Yongseok Koh; +Cc: ferruh.yigit, thomas.monjalon, dev, adrien.mazarguil 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-04-03 10:02 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-16 22:40 [PATCH 0/3] net/mlx5: fix updating RETA Yongseok Koh 2017-03-16 22:40 ` [PATCH 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh 2017-03-17 17:19 ` Yongseok Koh 2017-03-16 22:40 ` [PATCH 2/3] net/mlx5: use correct RETA table size Yongseok Koh 2017-03-16 22:40 ` [PATCH 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh 2017-03-17 9:11 ` Nélio Laranjeiro 2017-03-17 17:14 ` Yongseok Koh 2017-03-20 7:56 ` Nélio Laranjeiro 2017-03-20 23:04 ` [PATCH v2 0/3] net/mlx5: fix " Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 1/3] lib/librte_ether: remove requirement of aligned RETA size Yongseok Koh 2017-03-30 0:37 ` Thomas Monjalon 2017-03-31 7:33 ` Zhang, Helin 2017-04-01 7:28 ` Lu, Wenzhuo 2017-04-03 10:02 ` Ferruh Yigit 2017-03-20 23:04 ` [PATCH v2 2/3] net/mlx5: use correct RETA table size Yongseok Koh 2017-03-20 23:04 ` [PATCH v2 3/3] net/mlx5: rebuild flows on updating RETA Yongseok Koh 2017-03-21 7:45 ` [PATCH v2 0/3] net/mlx5: fix " Nélio Laranjeiro
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.