DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] bugfix for kni
@ 2021-04-22  3:56 Min Hu (Connor)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 1/3] kni: fix wrong comments Min Hu (Connor)
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-22  3:56 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This patchset contains three bugfixes for kni.

Min Hu (Connor) (3):
  kni: fix wrong comments
  net/kni: fix rewritten return value
  kni: fix unchecked return value

 app/test/test_kni.c           | 8 ++++++--
 drivers/net/kni/rte_eth_kni.c | 5 +++++
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.7.4


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

* [dpdk-dev] [PATCH 1/3] kni: fix wrong comments
  2021-04-22  3:56 [dpdk-dev] [PATCH 0/3] bugfix for kni Min Hu (Connor)
@ 2021-04-22  3:56 ` Min Hu (Connor)
  2021-04-26 13:34   ` Ferruh Yigit
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value Min Hu (Connor)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-22  3:56 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This patch changed 'subsytem' to 'subsystem'.

Fixes: 0c6bc8ef70ba ("kni: memzone pool for alloc and release")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test/test_kni.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_kni.c b/app/test/test_kni.c
index f53a53e..3470005 100644
--- a/app/test/test_kni.c
+++ b/app/test/test_kni.c
@@ -562,7 +562,7 @@ test_kni(void)
 	}
 	closedir(dir);
 
-	/* Initialize KNI subsytem */
+	/* Initialize KNI subsystem */
 	rte_kni_init(KNI_TEST_MAX_PORTS);
 
 	if (test_kni_allocate_lcores() < 0) {
-- 
2.7.4


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

* [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value
  2021-04-22  3:56 [dpdk-dev] [PATCH 0/3] bugfix for kni Min Hu (Connor)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 1/3] kni: fix wrong comments Min Hu (Connor)
@ 2021-04-22  3:56 ` Min Hu (Connor)
  2021-04-26 13:18   ` Ferruh Yigit
  2021-04-27  2:08   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 3/3] kni: fix unchecked " Min Hu (Connor)
  2021-04-26 13:35 ` [dpdk-dev] [PATCH 0/3] bugfix for kni Ferruh Yigit
  3 siblings, 2 replies; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-22  3:56 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Return value of function 'eth_kni_dev_stop' passed to 'ret' is
rewritten later, and this is unreasonable.

This patch fixes it.

Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/kni/rte_eth_kni.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 9ce74e5..067584c 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -211,6 +211,11 @@ eth_kni_close(struct rte_eth_dev *eth_dev)
 		return 0;
 
 	ret = eth_kni_dev_stop(eth_dev);
+	if (ret) {
+		PMD_LOG(WARNING, "Not able to stop kni for %s",
+			eth_dev->data->name);
+		return ret;
+	}
 
 	/* mac_addrs must not be freed alone because part of dev_private */
 	eth_dev->data->mac_addrs = NULL;
-- 
2.7.4


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

* [dpdk-dev] [PATCH 3/3] kni: fix unchecked return value
  2021-04-22  3:56 [dpdk-dev] [PATCH 0/3] bugfix for kni Min Hu (Connor)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 1/3] kni: fix wrong comments Min Hu (Connor)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value Min Hu (Connor)
@ 2021-04-22  3:56 ` Min Hu (Connor)
  2021-04-26 13:35   ` Ferruh Yigit
  2021-04-26 13:35 ` [dpdk-dev] [PATCH 0/3] bugfix for kni Ferruh Yigit
  3 siblings, 1 reply; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-22  3:56 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Return value 'rte_kni_init' of a function is not checked. If
it fails, error handling (logging and return) should be done.

This patch fixed it.

Fixes: 0c6bc8ef70ba ("kni: memzone pool for alloc and release")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test/test_kni.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test/test_kni.c b/app/test/test_kni.c
index 3470005..9673355 100644
--- a/app/test/test_kni.c
+++ b/app/test/test_kni.c
@@ -563,7 +563,11 @@ test_kni(void)
 	closedir(dir);
 
 	/* Initialize KNI subsystem */
-	rte_kni_init(KNI_TEST_MAX_PORTS);
+	ret = rte_kni_init(KNI_TEST_MAX_PORTS);
+	if (ret < 0) {
+		printf("fail to initialize KNI subsystem\n");
+		return -1;
+	}
 
 	if (test_kni_allocate_lcores() < 0) {
 		printf("No enough lcores for kni processing\n");
-- 
2.7.4


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

* Re: [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value Min Hu (Connor)
@ 2021-04-26 13:18   ` Ferruh Yigit
  2021-04-27  2:09     ` Min Hu (Connor)
  2021-04-27  2:08   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-26 13:18 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
> Return value of function 'eth_kni_dev_stop' passed to 'ret' is
> rewritten later, and this is unreasonable.
> 
> This patch fixes it.
> 
> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/kni/rte_eth_kni.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
> index 9ce74e5..067584c 100644
> --- a/drivers/net/kni/rte_eth_kni.c
> +++ b/drivers/net/kni/rte_eth_kni.c
> @@ -211,6 +211,11 @@ eth_kni_close(struct rte_eth_dev *eth_dev)
>  		return 0;
>  
>  	ret = eth_kni_dev_stop(eth_dev);
> +	if (ret) {
> +		PMD_LOG(WARNING, "Not able to stop kni for %s",
> +			eth_dev->data->name);
> +		return ret;
> +	}
>  

'eth_kni_close()' is called by 'eth_kni_remove()', and returning here without
setting 'eth_dev->data->mac_addrs' to NULL, 'eth_kni_remove()' may crash.

And the close ops is to not use the device anymore, there is not much to do if
stop() fails here. So what do you think to log but not return on the
'eth_kni_dev_stop()' failure?


>  	/* mac_addrs must not be freed alone because part of dev_private */
>  	eth_dev->data->mac_addrs = NULL;
> 


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

* Re: [dpdk-dev] [PATCH 1/3] kni: fix wrong comments
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 1/3] kni: fix wrong comments Min Hu (Connor)
@ 2021-04-26 13:34   ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-26 13:34 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
> This patch changed 'subsytem' to 'subsystem'.
> 
> Fixes: 0c6bc8ef70ba ("kni: memzone pool for alloc and release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

This is not on the "kni:", which implies the kni library, but the kni unit test,
will fix title while merging.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

* Re: [dpdk-dev] [PATCH 3/3] kni: fix unchecked return value
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 3/3] kni: fix unchecked " Min Hu (Connor)
@ 2021-04-26 13:35   ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-26 13:35 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
> Return value 'rte_kni_init' of a function is not checked. If
> it fails, error handling (logging and return) should be done.
> 
> This patch fixed it.
> 
> Fixes: 0c6bc8ef70ba ("kni: memzone pool for alloc and release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

With the patch title update,

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

* Re: [dpdk-dev] [PATCH 0/3] bugfix for kni
  2021-04-22  3:56 [dpdk-dev] [PATCH 0/3] bugfix for kni Min Hu (Connor)
                   ` (2 preceding siblings ...)
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 3/3] kni: fix unchecked " Min Hu (Connor)
@ 2021-04-26 13:35 ` Ferruh Yigit
  3 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-26 13:35 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
> This patchset contains three bugfixes for kni.
> 
> Min Hu (Connor) (3):
>   kni: fix wrong comments
>   net/kni: fix rewritten return value
>   kni: fix unchecked return value
> 

Except from 2/3,
Series applied to dpdk-next-net/main, thanks.

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

* [dpdk-dev] [PATCH v2] net/kni: fix rewritten return value
  2021-04-22  3:56 ` [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value Min Hu (Connor)
  2021-04-26 13:18   ` Ferruh Yigit
@ 2021-04-27  2:08   ` Min Hu (Connor)
  2021-04-29 13:37     ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-27  2:08 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Return value of function 'eth_kni_dev_stop' passed to 'ret' is
rewritten later, and this is unreasonable.

This patch fixes it.

Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v2:
* just log but not return.
---
 drivers/net/kni/rte_eth_kni.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 9ce74e5..00f9e1b 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -211,6 +211,9 @@ eth_kni_close(struct rte_eth_dev *eth_dev)
 		return 0;
 
 	ret = eth_kni_dev_stop(eth_dev);
+	if (ret)
+		PMD_LOG(WARNING, "Not able to stop kni for %s",
+			eth_dev->data->name);
 
 	/* mac_addrs must not be freed alone because part of dev_private */
 	eth_dev->data->mac_addrs = NULL;
-- 
2.7.4


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

* Re: [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value
  2021-04-26 13:18   ` Ferruh Yigit
@ 2021-04-27  2:09     ` Min Hu (Connor)
  0 siblings, 0 replies; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-27  2:09 UTC (permalink / raw)
  To: Ferruh Yigit, dev



在 2021/4/26 21:18, Ferruh Yigit 写道:
> On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
>> Return value of function 'eth_kni_dev_stop' passed to 'ret' is
>> rewritten later, and this is unreasonable.
>>
>> This patch fixes it.
>>
>> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   drivers/net/kni/rte_eth_kni.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
>> index 9ce74e5..067584c 100644
>> --- a/drivers/net/kni/rte_eth_kni.c
>> +++ b/drivers/net/kni/rte_eth_kni.c
>> @@ -211,6 +211,11 @@ eth_kni_close(struct rte_eth_dev *eth_dev)
>>   		return 0;
>>   
>>   	ret = eth_kni_dev_stop(eth_dev);
>> +	if (ret) {
>> +		PMD_LOG(WARNING, "Not able to stop kni for %s",
>> +			eth_dev->data->name);
>> +		return ret;
>> +	}
>>   
> 
> 'eth_kni_close()' is called by 'eth_kni_remove()', and returning here without
> setting 'eth_dev->data->mac_addrs' to NULL, 'eth_kni_remove()' may crash.
> 
> And the close ops is to not use the device anymore, there is not much to do if
> stop() fails here. So what do you think to log but not return on the
> 'eth_kni_dev_stop()' failure?
> 
Agreed, fixed in v2, thanks.
> 
>>   	/* mac_addrs must not be freed alone because part of dev_private */
>>   	eth_dev->data->mac_addrs = NULL;
>>
> 
> .
> 

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

* Re: [dpdk-dev] [PATCH v2] net/kni: fix rewritten return value
  2021-04-27  2:08   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
@ 2021-04-29 13:37     ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-29 13:37 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/27/2021 3:08 AM, Min Hu (Connor) wrote:
> Return value of function 'eth_kni_dev_stop' passed to 'ret' is
> rewritten later, and this is unreasonable.
> 
> This patch fixes it.
> 
> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

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

end of thread, other threads:[~2021-04-29 13:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22  3:56 [dpdk-dev] [PATCH 0/3] bugfix for kni Min Hu (Connor)
2021-04-22  3:56 ` [dpdk-dev] [PATCH 1/3] kni: fix wrong comments Min Hu (Connor)
2021-04-26 13:34   ` Ferruh Yigit
2021-04-22  3:56 ` [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value Min Hu (Connor)
2021-04-26 13:18   ` Ferruh Yigit
2021-04-27  2:09     ` Min Hu (Connor)
2021-04-27  2:08   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
2021-04-29 13:37     ` Ferruh Yigit
2021-04-22  3:56 ` [dpdk-dev] [PATCH 3/3] kni: fix unchecked " Min Hu (Connor)
2021-04-26 13:35   ` Ferruh Yigit
2021-04-26 13:35 ` [dpdk-dev] [PATCH 0/3] bugfix for kni Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).