[dpdk-dev] [PATCH 0/2] fix bugs for ethtool APP

[dpdk-dev] [PATCH 0/2] fix bugs for ethtool APP

[Min Hu (Connor)]

This patch fixed fix bugs for ethtool APP.

Chengwen Feng (1):
  examples/ethtool: fix Rx/Tx queue setup with rte socket id

Huisong Li (1):
  examples/ethtool: add closing port operation

 examples/ethtool/ethtool-app/main.c | 23 +++++++++++++++++++++++
 examples/ethtool/lib/rte_ethtool.c  |  4 ++--
 2 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Min Hu (Connor)]

From: Chengwen Feng <fengchengwen@huawei.com>

The ethtool use the socket_id which get from rte_eth_dev_socket_id API
in the init stage, but use the rte_socket_id API to get socket_id when
setting ringparam.

This patch make sure it call rte_eth_dev_socket_id API to get
socket_id when setting ringparam.

Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 examples/ethtool/lib/rte_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 4132516..b2e45f5 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -465,12 +465,12 @@ rte_ethtool_set_ringparam(uint16_t port_id,
 		return stat;
 
 	stat = rte_eth_tx_queue_setup(port_id, 0, ring_param->tx_pending,
-		rte_socket_id(), NULL);
+		rte_eth_dev_socket_id(port_id), NULL);
 	if (stat != 0)
 		return stat;
 
 	stat = rte_eth_rx_queue_setup(port_id, 0, ring_param->rx_pending,
-		rte_socket_id(), NULL, rx_qinfo.mp);
+		rte_eth_dev_socket_id(port_id), NULL, rx_qinfo.mp);
 	if (stat != 0)
 		return stat;
 
-- 
2.7.4

[dpdk-dev] [PATCH 2/2] examples/ethtool: add closing port operation

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

Currently, ethtool directly ends the process after 'quit' cmd. In this
case, software resources are not released and hardware resources of the
device are not uninstalled.

This patch adds closing port operation to release resources.

Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 examples/ethtool/ethtool-app/main.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethtool-app/main.c
index c6023a1..adabe6e 100644
--- a/examples/ethtool/ethtool-app/main.c
+++ b/examples/ethtool/ethtool-app/main.c
@@ -256,6 +256,27 @@ static int worker_main(__rte_unused void *ptr_data)
 	return 0;
 }
 
+static void close_ports(void)
+{
+	uint16_t portid;
+	int ret;
+
+	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
+		printf("Closing port %d...", portid);
+		ret = rte_eth_dev_stop(portid);
+		if (ret != 0)
+			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
+				 strerror(-ret), portid);
+		rte_eth_dev_close(portid);
+		printf(" Done\n");
+	}
+
+	ret = rte_eal_cleanup();
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE, "EAL cleanup failed: %s\n",
+			 strerror(-ret));
+}
+
 int main(int argc, char **argv)
 {
 	int cnt_args_parsed;
@@ -299,5 +320,7 @@ int main(int argc, char **argv)
 			return -1;
 	}
 
+	close_ports();
+
 	return 0;
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Thomas Monjalon]

08/04/2021 12:14, Min Hu (Connor):
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> The ethtool use the socket_id which get from rte_eth_dev_socket_id API
> in the init stage, but use the rte_socket_id API to get socket_id when
> setting ringparam.
> 
> This patch make sure it call rte_eth_dev_socket_id API to get
> socket_id when setting ringparam.
> 
> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> Cc: stable@dpdk.org

The explanation is missing something.
Please tell why it is wrong.

Re: [dpdk-dev] [PATCH 2/2] examples/ethtool: add closing port operation

[Thomas Monjalon]

08/04/2021 12:14, Min Hu (Connor):
> From: Huisong Li <lihuisong@huawei.com>
> 
> Currently, ethtool directly ends the process after 'quit' cmd. In this
> case, software resources are not released and hardware resources of the
> device are not uninstalled.
> 
> This patch adds closing port operation to release resources.
> 
> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> --- a/examples/ethtool/ethtool-app/main.c
> +++ b/examples/ethtool/ethtool-app/main.c
> +static void close_ports(void)
> +{
> +	uint16_t portid;
> +	int ret;
> +
> +	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
> +		printf("Closing port %d...", portid);
> +		ret = rte_eth_dev_stop(portid);
> +		if (ret != 0)
> +			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
> +				 strerror(-ret), portid);
> +		rte_eth_dev_close(portid);
> +		printf(" Done\n");
> +	}
> +
> +	ret = rte_eal_cleanup();
> +	if (ret != 0)
> +		rte_exit(EXIT_FAILURE, "EAL cleanup failed: %s\n",
> +			 strerror(-ret));

It would be better to add EAL cleanup in the main function.

Re: [dpdk-dev] [PATCH 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Min Hu (Connor)]

在 2021/4/20 8:57, Thomas Monjalon 写道:
> 08/04/2021 12:14, Min Hu (Connor):
>> From: Chengwen Feng <fengchengwen@huawei.com>
>>
>> The ethtool use the socket_id which get from rte_eth_dev_socket_id API
>> in the init stage, but use the rte_socket_id API to get socket_id when
>> setting ringparam.
>>
>> This patch make sure it call rte_eth_dev_socket_id API to get
>> socket_id when setting ringparam.
>>
>> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
>> Cc: stable@dpdk.org
> 
> The explanation is missing something.
> Please tell why it is wrong.
> 
> 
Hi, Thomas,
It is advised that bind queue resources to the NUMA node where the
device resides, not the NUMA node corresponding to the thread where the
command line resides. And that will be better.

> 
> .
> 

Re: [dpdk-dev] [PATCH 2/2] examples/ethtool: add closing port operation

[Min Hu (Connor)]

在 2021/4/20 8:59, Thomas Monjalon 写道:
> 08/04/2021 12:14, Min Hu (Connor):
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Currently, ethtool directly ends the process after 'quit' cmd. In this
>> case, software resources are not released and hardware resources of the
>> device are not uninstalled.
>>
>> This patch adds closing port operation to release resources.
>>
>> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> --- a/examples/ethtool/ethtool-app/main.c
>> +++ b/examples/ethtool/ethtool-app/main.c
>> +static void close_ports(void)
>> +{
>> +	uint16_t portid;
>> +	int ret;
>> +
>> +	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
>> +		printf("Closing port %d...", portid);
>> +		ret = rte_eth_dev_stop(portid);
>> +		if (ret != 0)
>> +			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
>> +				 strerror(-ret), portid);
>> +		rte_eth_dev_close(portid);
>> +		printf(" Done\n");
>> +	}
>> +
>> +	ret = rte_eal_cleanup();
>> +	if (ret != 0)
>> +		rte_exit(EXIT_FAILURE, "EAL cleanup failed: %s\n",
>> +			 strerror(-ret));
> 
> It would be better to add EAL cleanup in the main function.
> 
Hi, Thomas,
	It has already been done in this patch: "examples: add eal
cleanup to examples", please check it out, thanks.
> 
> .
> 

Re: [dpdk-dev] [PATCH 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Thomas Monjalon]

20/04/2021 11:05, Min Hu (Connor):
> 在 2021/4/20 8:57, Thomas Monjalon 写道:
> > 08/04/2021 12:14, Min Hu (Connor):
> >> From: Chengwen Feng <fengchengwen@huawei.com>
> >>
> >> The ethtool use the socket_id which get from rte_eth_dev_socket_id API
> >> in the init stage, but use the rte_socket_id API to get socket_id when
> >> setting ringparam.
> >>
> >> This patch make sure it call rte_eth_dev_socket_id API to get
> >> socket_id when setting ringparam.
> >>
> >> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> >> Cc: stable@dpdk.org
> > 
> > The explanation is missing something.
> > Please tell why it is wrong.
> > 
> > 
> Hi, Thomas,
> It is advised that bind queue resources to the NUMA node where the
> device resides, not the NUMA node corresponding to the thread where the
> command line resides. And that will be better.

I know, but in the commit message, you give function name
without saying that rte_socket_id is the running socket
while rte_eth_dev_socket_id is the device socket.

Please make your commit messages more obvious for anyone reading.
Story telling is usually a good approach.

Re: [dpdk-dev] [PATCH 2/2] examples/ethtool: add closing port operation

[Thomas Monjalon]

20/04/2021 11:10, Min Hu (Connor):
> 
> 在 2021/4/20 8:59, Thomas Monjalon 写道:
> > 08/04/2021 12:14, Min Hu (Connor):
> >> From: Huisong Li <lihuisong@huawei.com>
> >>
> >> Currently, ethtool directly ends the process after 'quit' cmd. In this
> >> case, software resources are not released and hardware resources of the
> >> device are not uninstalled.
> >>
> >> This patch adds closing port operation to release resources.
> >>
> >> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> ---
> >> --- a/examples/ethtool/ethtool-app/main.c
> >> +++ b/examples/ethtool/ethtool-app/main.c
> >> +static void close_ports(void)
> >> +{
> >> +	uint16_t portid;
> >> +	int ret;
> >> +
> >> +	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
> >> +		printf("Closing port %d...", portid);
> >> +		ret = rte_eth_dev_stop(portid);
> >> +		if (ret != 0)
> >> +			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
> >> +				 strerror(-ret), portid);
> >> +		rte_eth_dev_close(portid);
> >> +		printf(" Done\n");
> >> +	}
> >> +
> >> +	ret = rte_eal_cleanup();
> >> +	if (ret != 0)
> >> +		rte_exit(EXIT_FAILURE, "EAL cleanup failed: %s\n",
> >> +			 strerror(-ret));
> > 
> > It would be better to add EAL cleanup in the main function.
> > 
> Hi, Thomas,
> 	It has already been done in this patch: "examples: add eal
> cleanup to examples", please check it out, thanks.

So why adding it in this patch about ethdev close?

[dpdk-dev] [PATCH v2 0/2] fix bugs for ethtool APP

[Min Hu (Connor)]

This patch fixed fix bugs for ethtool APP.

Chengwen Feng (1):
  examples/ethtool: fix Rx/Tx queue setup with rte socket id

Huisong Li (1):
  examples/ethtool: add closing port operation

 examples/ethtool/ethtool-app/main.c | 18 ++++++++++++++++++
 examples/ethtool/lib/rte_ethtool.c  |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)
---
v2:
* modified commit info.
  add close port operation.
-- 
2.7.4

[dpdk-dev] [PATCH v2 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Min Hu (Connor)]

From: Chengwen Feng <fengchengwen@huawei.com>

In DPDK, 'rte_socket_id' means the running socket while
'rte_eth_dev_socket_id' is the device socket. For better performance,
memory which queue setup used and device should be in the same socket.

This patch make sure it calls rte_eth_dev_socket_id API to get device
socket_id when setting ringparam.

Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 examples/ethtool/lib/rte_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 4132516..b2e45f5 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -465,12 +465,12 @@ rte_ethtool_set_ringparam(uint16_t port_id,
 		return stat;
 
 	stat = rte_eth_tx_queue_setup(port_id, 0, ring_param->tx_pending,
-		rte_socket_id(), NULL);
+		rte_eth_dev_socket_id(port_id), NULL);
 	if (stat != 0)
 		return stat;
 
 	stat = rte_eth_rx_queue_setup(port_id, 0, ring_param->rx_pending,
-		rte_socket_id(), NULL, rx_qinfo.mp);
+		rte_eth_dev_socket_id(port_id), NULL, rx_qinfo.mp);
 	if (stat != 0)
 		return stat;
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] examples/ethtool: add closing port operation

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

Currently, ethtool directly ends the process after 'quit' cmd. In this
case, software resources are not released and hardware resources of the
device are not uninstalled.

This patch adds closing port operation to release resources.

Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 examples/ethtool/ethtool-app/main.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethtool-app/main.c
index 21ed85c..9ac0a44 100644
--- a/examples/ethtool/ethtool-app/main.c
+++ b/examples/ethtool/ethtool-app/main.c
@@ -256,6 +256,22 @@ static int worker_main(__rte_unused void *ptr_data)
 	return 0;
 }
 
+static void close_ports(void)
+{
+	uint16_t portid;
+	int ret;
+
+	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
+		printf("Closing port %d...", portid);
+		ret = rte_eth_dev_stop(portid);
+		if (ret != 0)
+			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
+				 strerror(-ret), portid);
+		rte_eth_dev_close(portid);
+		printf(" Done\n");
+	}
+}
+
 int main(int argc, char **argv)
 {
 	int cnt_args_parsed;
@@ -299,6 +315,8 @@ int main(int argc, char **argv)
 			return -1;
 	}
 
+	close_ports();
+
 	/* clean up the EAL */
 	rte_eal_cleanup();
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 0/2] fix bugs for ethtool APP

[Min Hu (Connor)]

Hi, all,
	any comments?

在 2021/5/6 11:46, Min Hu (Connor) 写道:
> This patch fixed fix bugs for ethtool APP.
> 
> Chengwen Feng (1):
>    examples/ethtool: fix Rx/Tx queue setup with rte socket id
> 
> Huisong Li (1):
>    examples/ethtool: add closing port operation
> 
>   examples/ethtool/ethtool-app/main.c | 18 ++++++++++++++++++
>   examples/ethtool/lib/rte_ethtool.c  |  4 ++--
>   2 files changed, 20 insertions(+), 2 deletions(-)
> ---
> v2:
> * modified commit info.
>    add close port operation.
> 

Re: [dpdk-dev] [PATCH v2 0/2] fix bugs for ethtool APP

[Min Hu (Connor)]

Hi, all,
      any comments?

在 2021/6/28 11:22, Min Hu (Connor) 写道:
> Hi, all,
>      any comments?
> 
> 在 2021/5/6 11:46, Min Hu (Connor) 写道:
>> This patch fixed fix bugs for ethtool APP.
>>
>> Chengwen Feng (1):
>>    examples/ethtool: fix Rx/Tx queue setup with rte socket id
>>
>> Huisong Li (1):
>>    examples/ethtool: add closing port operation
>>
>>   examples/ethtool/ethtool-app/main.c | 18 ++++++++++++++++++
>>   examples/ethtool/lib/rte_ethtool.c  |  4 ++--
>>   2 files changed, 20 insertions(+), 2 deletions(-)
>> ---
>> v2:
>> * modified commit info.
>>    add close port operation.
>>
> .

Re: [dpdk-dev] [PATCH v2 2/2] examples/ethtool: add closing port operation

[Min Hu (Connor)]

Hi, Ferruh,
	any comments?

在 2021/5/6 11:46, Min Hu (Connor) 写道:
> From: Huisong Li <lihuisong@huawei.com>
> 
> Currently, ethtool directly ends the process after 'quit' cmd. In this
> case, software resources are not released and hardware resources of the
> device are not uninstalled.
> 
> This patch adds closing port operation to release resources.
> 
> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>   examples/ethtool/ethtool-app/main.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethtool-app/main.c
> index 21ed85c..9ac0a44 100644
> --- a/examples/ethtool/ethtool-app/main.c
> +++ b/examples/ethtool/ethtool-app/main.c
> @@ -256,6 +256,22 @@ static int worker_main(__rte_unused void *ptr_data)
>   	return 0;
>   }
>   
> +static void close_ports(void)
> +{
> +	uint16_t portid;
> +	int ret;
> +
> +	for (portid = 0; portid < app_cfg.cnt_ports; portid++) {
> +		printf("Closing port %d...", portid);
> +		ret = rte_eth_dev_stop(portid);
> +		if (ret != 0)
> +			rte_exit(EXIT_FAILURE, "rte_eth_dev_stop: err=%s, port=%u\n",
> +				 strerror(-ret), portid);
> +		rte_eth_dev_close(portid);
> +		printf(" Done\n");
> +	}
> +}
> +
>   int main(int argc, char **argv)
>   {
>   	int cnt_args_parsed;
> @@ -299,6 +315,8 @@ int main(int argc, char **argv)
>   			return -1;
>   	}
>   
> +	close_ports();
> +
>   	/* clean up the EAL */
>   	rte_eal_cleanup();
>   
> 

Re: [dpdk-dev] [PATCH v2 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id

[Min Hu (Connor)]

Hi, Thomas,
	any comments?

在 2021/5/6 11:46, Min Hu (Connor) 写道:
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> In DPDK, 'rte_socket_id' means the running socket while
> 'rte_eth_dev_socket_id' is the device socket. For better performance,
> memory which queue setup used and device should be in the same socket.
> 
> This patch make sure it calls rte_eth_dev_socket_id API to get device
> socket_id when setting ringparam.
> 
> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>   examples/ethtool/lib/rte_ethtool.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
> index 4132516..b2e45f5 100644
> --- a/examples/ethtool/lib/rte_ethtool.c
> +++ b/examples/ethtool/lib/rte_ethtool.c
> @@ -465,12 +465,12 @@ rte_ethtool_set_ringparam(uint16_t port_id,
>   		return stat;
>   
>   	stat = rte_eth_tx_queue_setup(port_id, 0, ring_param->tx_pending,
> -		rte_socket_id(), NULL);
> +		rte_eth_dev_socket_id(port_id), NULL);
>   	if (stat != 0)
>   		return stat;
>   
>   	stat = rte_eth_rx_queue_setup(port_id, 0, ring_param->rx_pending,
> -		rte_socket_id(), NULL, rx_qinfo.mp);
> +		rte_eth_dev_socket_id(port_id), NULL, rx_qinfo.mp);
>   	if (stat != 0)
>   		return stat;
>   
> 

Re: [dpdk-dev] [PATCH v2 2/2] examples/ethtool: add closing port operation

[David Marchand]

On Thu, May 6, 2021 at 5:46 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> From: Huisong Li <lihuisong@huawei.com>
>
> Currently, ethtool directly ends the process after 'quit' cmd. In this
> case, software resources are not released and hardware resources of the
> device are not uninstalled.
>
> This patch adds closing port operation to release resources.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

I see nothing wrong with series and there was no objection.
Series applied, thanks.


-- 
David Marchand