diff mbox

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

Message ID 7783e46e840052a9abdd4e8962eede598ab848af.1490050764.git.yskoh@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Yongseok Koh March 20, 2017, 11:04 p.m. UTC 
  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(-)

Comments

Thomas Monjalon March 30, 2017, 12:37 a.m. UTC | #1 
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.
Zhang, Helin March 31, 2017, 7:33 a.m. UTC | #2 
> -----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.
Wenzhuo Lu April 1, 2017, 7:28 a.m. UTC | #3 
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.
Ferruh Yigit April 3, 2017, 10:02 a.m. UTC | #4 
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.
diff mbox

Patch

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;