diff mbox series

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

Message ID 1617876888-63458-2-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series fix bugs for ethtool APP |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

humin (Q) April 8, 2021, 10:14 a.m. UTC 
  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(-)

Comments

Thomas Monjalon April 20, 2021, 12:57 a.m. UTC | #1 
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.
humin (Q) April 20, 2021, 9:05 a.m. UTC | #2 
在 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.

> 
> .
>
Thomas Monjalon April 20, 2021, 9:37 a.m. UTC | #3 
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.
diff mbox series

Patch

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;