DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] bugfix for tap device
@ 2021-04-22 11:27 Min Hu (Connor)
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored Min Hu (Connor)
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size Min Hu (Connor)
  0 siblings, 2 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-22 11:27 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, keith.wiles

This patchset contains two bugfixes for tap device.

Chengchang Tang (2):
  net/tap: fix log loss when state fails to be restored
  net/tap: fix tap interrupt vector array size

 drivers/net/tap/rte_eth_tap.c | 6 +++++-
 drivers/net/tap/tap_intr.c    | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.7.4


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

* [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored
  2021-04-22 11:27 [dpdk-dev] [PATCH 0/2] bugfix for tap device Min Hu (Connor)
@ 2021-04-22 11:27 ` Min Hu (Connor)
  2021-04-26 15:30   ` Ferruh Yigit
  2021-04-27  0:54   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size Min Hu (Connor)
  1 sibling, 2 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-22 11:27 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, keith.wiles

From: Chengchang Tang <tangchengchang@huawei.com>

After restoring the remote states, the return value of ioctl() is not
checked. Therefore, users cannot know whether the remote state is
restored successfully.

This patch add log for restoring failure.

Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/tap/rte_eth_tap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 68baa18..6007c78 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1101,6 +1101,7 @@ tap_dev_close(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 	struct pmd_process_private *process_private = dev->process_private;
 	struct rx_queue *rxq;
+	int ret;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		rte_free(dev->process_private);
@@ -1133,8 +1134,11 @@ tap_dev_close(struct rte_eth_dev *dev)
 
 	if (internals->remote_if_index) {
 		/* Restore initial remote state */
-		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
+		ret = ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
 				&internals->remote_initial_flags);
+		if (ret)
+			TAP_LOG(ERR, "restore remote state failed: %d", ret);
+
 	}
 
 	rte_mempool_free(internals->gso_ctx_mp);
-- 
2.7.4


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

* [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size
  2021-04-22 11:27 [dpdk-dev] [PATCH 0/2] bugfix for tap device Min Hu (Connor)
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored Min Hu (Connor)
@ 2021-04-22 11:27 ` Min Hu (Connor)
  2021-04-22 15:20   ` Stephen Hemminger
  2021-04-26 15:31   ` Ferruh Yigit
  1 sibling, 2 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-22 11:27 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, keith.wiles

From: Chengchang Tang <tangchengchang@huawei.com>

The size of the current interrupt vector array is fixed to an integer.

This patch will create an interrupt vector array based on the number
of rxqs.

Fixes: 4870a8cdd968 ("net/tap: support Rx interrupt")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/tap/tap_intr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c
index 5cf4f17..1cacc15 100644
--- a/drivers/net/tap/tap_intr.c
+++ b/drivers/net/tap/tap_intr.c
@@ -59,7 +59,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev)
 
 	if (!dev->data->dev_conf.intr_conf.rxq)
 		return 0;
-	intr_handle->intr_vec = malloc(sizeof(intr_handle->intr_vec[rxqs_n]));
+	intr_handle->intr_vec = malloc(sizeof(int) * rxqs_n);
 	if (intr_handle->intr_vec == NULL) {
 		rte_errno = ENOMEM;
 		TAP_LOG(ERR,
-- 
2.7.4


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

* Re: [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size Min Hu (Connor)
@ 2021-04-22 15:20   ` Stephen Hemminger
  2021-04-23  9:34     ` Min Hu (Connor)
  2021-04-26 15:31   ` Ferruh Yigit
  1 sibling, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2021-04-22 15:20 UTC (permalink / raw)
  To: Min Hu (Connor); +Cc: dev, ferruh.yigit, keith.wiles

On Thu, 22 Apr 2021 19:27:14 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c
> index 5cf4f17..1cacc15 100644
> --- a/drivers/net/tap/tap_intr.c
> +++ b/drivers/net/tap/tap_intr.c
> @@ -59,7 +59,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev)
>  
>  	if (!dev->data->dev_conf.intr_conf.rxq)
>  		return 0;
> -	intr_handle->intr_vec = malloc(sizeof(intr_handle->intr_vec[rxqs_n]));
> +	intr_handle->intr_vec = malloc(sizeof(int) * rxqs_n);

Maybe calloc() here would be good idea?

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

* Re: [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size
  2021-04-22 15:20   ` Stephen Hemminger
@ 2021-04-23  9:34     ` Min Hu (Connor)
  0 siblings, 0 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-23  9:34 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, ferruh.yigit, keith.wiles



在 2021/4/22 23:20, Stephen Hemminger 写道:
> On Thu, 22 Apr 2021 19:27:14 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>> diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c
>> index 5cf4f17..1cacc15 100644
>> --- a/drivers/net/tap/tap_intr.c
>> +++ b/drivers/net/tap/tap_intr.c
>> @@ -59,7 +59,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev)
>>   
>>   	if (!dev->data->dev_conf.intr_conf.rxq)
>>   		return 0;
>> -	intr_handle->intr_vec = malloc(sizeof(intr_handle->intr_vec[rxqs_n]));
>> +	intr_handle->intr_vec = malloc(sizeof(int) * rxqs_n);
> 
> Maybe calloc() here would be good idea?
> 
Hi, Stephen,
	No need to use calloc, because members of
  'intr_handle->intr_vec' array will be set new
value.
> 

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

* Re: [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored Min Hu (Connor)
@ 2021-04-26 15:30   ` Ferruh Yigit
  2021-04-27  0:54     ` Min Hu (Connor)
  2021-04-27  0:54   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
  1 sibling, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2021-04-26 15:30 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: keith.wiles

On 4/22/2021 12:27 PM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> After restoring the remote states, the return value of ioctl() is not
> checked. Therefore, users cannot know whether the remote state is
> restored successfully.
> 
> This patch add log for restoring failure.
> 
> Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 68baa18..6007c78 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1101,6 +1101,7 @@ tap_dev_close(struct rte_eth_dev *dev)
>  	struct pmd_internals *internals = dev->data->dev_private;
>  	struct pmd_process_private *process_private = dev->process_private;
>  	struct rx_queue *rxq;
> +	int ret;
>  
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>  		rte_free(dev->process_private);
> @@ -1133,8 +1134,11 @@ tap_dev_close(struct rte_eth_dev *dev)
>  
>  	if (internals->remote_if_index) {
>  		/* Restore initial remote state */
> -		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
> +		ret = ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>  				&internals->remote_initial_flags);
> +		if (ret)
> +			TAP_LOG(ERR, "restore remote state failed: %d", ret);
> +

'ret' is used only in this scope, can you please move the variable declaration
at the beginning of the this if block?
You can do something like "int ret = ioctl(...."


>  	}
>  
>  	rte_mempool_free(internals->gso_ctx_mp);
> 


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

* Re: [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size Min Hu (Connor)
  2021-04-22 15:20   ` Stephen Hemminger
@ 2021-04-26 15:31   ` Ferruh Yigit
  1 sibling, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2021-04-26 15:31 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: keith.wiles

On 4/22/2021 12:27 PM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The size of the current interrupt vector array is fixed to an integer.
> 
> This patch will create an interrupt vector array based on the number
> of rxqs.
> 
> Fixes: 4870a8cdd968 ("net/tap: support Rx interrupt")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> 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] 10+ messages in thread

* [dpdk-dev] [PATCH v2] net/tap: fix log loss when state fails to be restored
  2021-04-22 11:27 ` [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored Min Hu (Connor)
  2021-04-26 15:30   ` Ferruh Yigit
@ 2021-04-27  0:54   ` Min Hu (Connor)
  2021-04-29 13:36     ` Ferruh Yigit
  1 sibling, 1 reply; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-27  0:54 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

From: Chengchang Tang <tangchengchang@huawei.com>

After restoring the remote states, the return value of ioctl() is not
checked. Therefore, users cannot know whether the remote state is
restored successfully.

This patch add log for restoring failure.

Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v2:
* move the variable declaration.
---
 drivers/net/tap/rte_eth_tap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 68baa18..854abf4 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1133,8 +1133,11 @@ tap_dev_close(struct rte_eth_dev *dev)
 
 	if (internals->remote_if_index) {
 		/* Restore initial remote state */
-		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
+		int ret = ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
 				&internals->remote_initial_flags);
+		if (ret)
+			TAP_LOG(ERR, "restore remote state failed: %d", ret);
+
 	}
 
 	rte_mempool_free(internals->gso_ctx_mp);
-- 
2.7.4


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

* Re: [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored
  2021-04-26 15:30   ` Ferruh Yigit
@ 2021-04-27  0:54     ` Min Hu (Connor)
  0 siblings, 0 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2021-04-27  0:54 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: keith.wiles



在 2021/4/26 23:30, Ferruh Yigit 写道:
> On 4/22/2021 12:27 PM, Min Hu (Connor) wrote:
>> From: Chengchang Tang <tangchengchang@huawei.com>
>>
>> After restoring the remote states, the return value of ioctl() is not
>> checked. Therefore, users cannot know whether the remote state is
>> restored successfully.
>>
>> This patch add log for restoring failure.
>>
>> Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   drivers/net/tap/rte_eth_tap.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 68baa18..6007c78 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -1101,6 +1101,7 @@ tap_dev_close(struct rte_eth_dev *dev)
>>   	struct pmd_internals *internals = dev->data->dev_private;
>>   	struct pmd_process_private *process_private = dev->process_private;
>>   	struct rx_queue *rxq;
>> +	int ret;
>>   
>>   	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>>   		rte_free(dev->process_private);
>> @@ -1133,8 +1134,11 @@ tap_dev_close(struct rte_eth_dev *dev)
>>   
>>   	if (internals->remote_if_index) {
>>   		/* Restore initial remote state */
>> -		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>> +		ret = ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>>   				&internals->remote_initial_flags);
>> +		if (ret)
>> +			TAP_LOG(ERR, "restore remote state failed: %d", ret);
>> +
> 
> 'ret' is used only in this scope, can you please move the variable declaration
> at the beginning of the this if block?
> You can do something like "int ret = ioctl(...."
> 
Hi, fixed in v2, thanks.
> 
>>   	}
>>   
>>   	rte_mempool_free(internals->gso_ctx_mp);
>>
> 
> .
> 

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

* Re: [dpdk-dev] [PATCH v2] net/tap: fix log loss when state fails to be restored
  2021-04-27  0:54   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
@ 2021-04-29 13:36     ` Ferruh Yigit
  0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2021-04-29 13:36 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/27/2021 1:54 AM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> After restoring the remote states, the return value of ioctl() is not
> checked. Therefore, users cannot know whether the remote state is
> restored successfully.
> 
> This patch add log for restoring failure.
> 
> Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> 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] 10+ messages in thread

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 11:27 [dpdk-dev] [PATCH 0/2] bugfix for tap device Min Hu (Connor)
2021-04-22 11:27 ` [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored Min Hu (Connor)
2021-04-26 15:30   ` Ferruh Yigit
2021-04-27  0:54     ` Min Hu (Connor)
2021-04-27  0:54   ` [dpdk-dev] [PATCH v2] " Min Hu (Connor)
2021-04-29 13:36     ` Ferruh Yigit
2021-04-22 11:27 ` [dpdk-dev] [PATCH 2/2] net/tap: fix tap interrupt vector array size Min Hu (Connor)
2021-04-22 15:20   ` Stephen Hemminger
2021-04-23  9:34     ` Min Hu (Connor)
2021-04-26 15:31   ` 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).