All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/sfc: specify correct scale table size on Rx start
@ 2017-08-28 12:53 Andrew Rybchenko
  2017-08-29 17:03 ` [dpdk-stable] " Ferruh Yigit
  2017-08-30 13:47 ` Ferruh Yigit
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Rybchenko @ 2017-08-28 12:53 UTC (permalink / raw)
  To: dev; +Cc: Ivan Malov, stable

From: Ivan Malov <ivan.malov@oktetlabs.ru>

efx_rx_scale_tbl_set() takes the number of entries in the scale table
to be set, not the size of the table in bytes; currently this bug does
not make any damage since the size argument is used to wrap the loop
on the input table when filling in an MCDI request in case if the table
size in the MCDI request is larger then one provided by the user,
and MCDI scale table size is the same as the size of the table provided
by the driver; this patch brings a fix for the bug

Fixes: 4ec1fc3ba881 ("net/sfc: add basic stubs for RSS support on driver attach")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 1bf8644..364f718 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1068,7 +1068,7 @@ struct sfc_dp_rx sfc_efx_rx = {
 			goto finish;
 
 		rc = efx_rx_scale_tbl_set(sa->nic, sa->rss_tbl,
-					  sizeof(sa->rss_tbl));
+					  RTE_DIM(sa->rss_tbl));
 	}
 
 finish:
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH] net/sfc: specify correct scale table size on Rx start
  2017-08-28 12:53 [PATCH] net/sfc: specify correct scale table size on Rx start Andrew Rybchenko
@ 2017-08-29 17:03 ` Ferruh Yigit
  2017-08-30  7:24   ` Ferruh Yigit
  2017-08-30  9:16   ` Andrew Rybchenko
  2017-08-30 13:47 ` Ferruh Yigit
  1 sibling, 2 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-08-29 17:03 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: Ivan Malov, stable

On 8/28/2017 1:53 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> efx_rx_scale_tbl_set() takes the number of entries in the scale table
> to be set, not the size of the table in bytes; currently this bug does
> not make any damage since the size argument is used to wrap the loop
> on the input table when filling in an MCDI request in case if the table
> size in the MCDI request is larger then one provided by the user,
> and MCDI scale table size is the same as the size of the table provided
> by the driver; this patch brings a fix for the bug
> 
> Fixes: 4ec1fc3ba881 ("net/sfc: add basic stubs for RSS support on driver attach")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/sfc/sfc_rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
> index 1bf8644..364f718 100644
> --- a/drivers/net/sfc/sfc_rx.c
> +++ b/drivers/net/sfc/sfc_rx.c
> @@ -1068,7 +1068,7 @@ struct sfc_dp_rx sfc_efx_rx = {
>  			goto finish;
>  

It is not shown in this patch, but there is "sizeof(sa->rss_key)" usage
here, since its type is uint8_t, it is not wrong, but it can be good to
change that one too to RTE_DIM() both to show the intent and to be safe
if in the future type updated... But this is your call to update or not.

>  		rc = efx_rx_scale_tbl_set(sa->nic, sa->rss_tbl,
> -					  sizeof(sa->rss_tbl));
> +					  RTE_DIM(sa->rss_tbl));

There are more sizeof(sa->rss_tbl) usage in sfc_ethdev.c [1], can you
please check them too? malloc and memcpy ones look suspicious.

[1]
$ git grep "sizeof(sa->rss_tbl)"
drivers/net/sfc/sfc_ethdev.c:   rss_tbl_new = rte_zmalloc("rss_tbl_new",
sizeof(sa->rss_tbl), 0);
drivers/net/sfc/sfc_ethdev.c:   rte_memcpy(rss_tbl_new, sa->rss_tbl,
sizeof(sa->rss_tbl));
drivers/net/sfc/sfc_ethdev.c:           rte_memcpy(sa->rss_tbl,
rss_tbl_new, sizeof(sa->rss_tbl));
drivers/net/sfc/sfc_rx.c:
sizeof(sa->rss_tbl));

>  	}
>  
>  finish:
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH] net/sfc: specify correct scale table size on Rx start
  2017-08-29 17:03 ` [dpdk-stable] " Ferruh Yigit
@ 2017-08-30  7:24   ` Ferruh Yigit
  2017-08-30  9:16   ` Andrew Rybchenko
  1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-08-30  7:24 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: Ivan Malov, stable

On 8/29/2017 6:03 PM, Ferruh Yigit wrote:
> On 8/28/2017 1:53 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> efx_rx_scale_tbl_set() takes the number of entries in the scale table
>> to be set, not the size of the table in bytes; currently this bug does
>> not make any damage since the size argument is used to wrap the loop
>> on the input table when filling in an MCDI request in case if the table
>> size in the MCDI request is larger then one provided by the user,
>> and MCDI scale table size is the same as the size of the table provided
>> by the driver; this patch brings a fix for the bug
>>
>> Fixes: 4ec1fc3ba881 ("net/sfc: add basic stubs for RSS support on driver attach")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  drivers/net/sfc/sfc_rx.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
>> index 1bf8644..364f718 100644
>> --- a/drivers/net/sfc/sfc_rx.c
>> +++ b/drivers/net/sfc/sfc_rx.c
>> @@ -1068,7 +1068,7 @@ struct sfc_dp_rx sfc_efx_rx = {
>>  			goto finish;
>>  
> 
> It is not shown in this patch, but there is "sizeof(sa->rss_key)" usage
> here, since its type is uint8_t, it is not wrong, but it can be good to
> change that one too to RTE_DIM() both to show the intent and to be safe
> if in the future type updated... But this is your call to update or not.
> 
>>  		rc = efx_rx_scale_tbl_set(sa->nic, sa->rss_tbl,
>> -					  sizeof(sa->rss_tbl));
>> +					  RTE_DIM(sa->rss_tbl));
> 
> There are more sizeof(sa->rss_tbl) usage in sfc_ethdev.c [1], can you
> please check them too? malloc and memcpy ones look suspicious.

My bad, please ignore this.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH] net/sfc: specify correct scale table size on Rx start
  2017-08-29 17:03 ` [dpdk-stable] " Ferruh Yigit
  2017-08-30  7:24   ` Ferruh Yigit
@ 2017-08-30  9:16   ` Andrew Rybchenko
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Rybchenko @ 2017-08-30  9:16 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: Ivan Malov, stable

On 08/29/2017 08:03 PM, Ferruh Yigit wrote:
> On 8/28/2017 1:53 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> efx_rx_scale_tbl_set() takes the number of entries in the scale table
>> to be set, not the size of the table in bytes; currently this bug does
>> not make any damage since the size argument is used to wrap the loop
>> on the input table when filling in an MCDI request in case if the table
>> size in the MCDI request is larger then one provided by the user,
>> and MCDI scale table size is the same as the size of the table provided
>> by the driver; this patch brings a fix for the bug
>>
>> Fixes: 4ec1fc3ba881 ("net/sfc: add basic stubs for RSS support on driver attach")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   drivers/net/sfc/sfc_rx.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
>> index 1bf8644..364f718 100644
>> --- a/drivers/net/sfc/sfc_rx.c
>> +++ b/drivers/net/sfc/sfc_rx.c
>> @@ -1068,7 +1068,7 @@ struct sfc_dp_rx sfc_efx_rx = {
>>   			goto finish;
>>   
> It is not shown in this patch, but there is "sizeof(sa->rss_key)" usage
> here, since its type is uint8_t, it is not wrong, but it can be good to
> change that one too to RTE_DIM() both to show the intent and to be safe
> if in the future type updated... But this is your call to update or not.

In theory API annotation in efx.h says __in_ecount(n), so it should be 
element
count and RTE_DIM() should be used. However, the RSS hash key semantics is
just a byte string and I think it would be more correct to annotate it as
__in_bcount(n) regardless element size. I'll discuss it internally, but 
right now
I would prefer to keep it as is just to avoid code shuffling.

Thanks,
Andrew.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH] net/sfc: specify correct scale table size on Rx start
  2017-08-28 12:53 [PATCH] net/sfc: specify correct scale table size on Rx start Andrew Rybchenko
  2017-08-29 17:03 ` [dpdk-stable] " Ferruh Yigit
@ 2017-08-30 13:47 ` Ferruh Yigit
  1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-08-30 13:47 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: Ivan Malov, stable

On 8/28/2017 1:53 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> efx_rx_scale_tbl_set() takes the number of entries in the scale table
> to be set, not the size of the table in bytes; currently this bug does
> not make any damage since the size argument is used to wrap the loop
> on the input table when filling in an MCDI request in case if the table
> size in the MCDI request is larger then one provided by the user,
> and MCDI scale table size is the same as the size of the table provided
> by the driver; this patch brings a fix for the bug
> 
> Fixes: 4ec1fc3ba881 ("net/sfc: add basic stubs for RSS support on driver attach")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-08-30 13:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 12:53 [PATCH] net/sfc: specify correct scale table size on Rx start Andrew Rybchenko
2017-08-29 17:03 ` [dpdk-stable] " Ferruh Yigit
2017-08-30  7:24   ` Ferruh Yigit
2017-08-30  9:16   ` Andrew Rybchenko
2017-08-30 13:47 ` Ferruh Yigit

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.