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
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
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
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.
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.
在 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. > > . >
在 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. > > . >
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.
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?
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
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
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
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.
>
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.
>>
> .
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();
>
>
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;
>
>
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