[v2] app/testpmd: fix physic port socket initialization

Message ID 1539740190-22632-1-git-send-email-phil.yang@arm.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] app/testpmd: fix physic port socket initialization |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Phil Yang Oct. 17, 2018, 1:36 a.m. UTC
  Once the lcore list setting excluded the socket which physic device
attached, it will cause failure. Meanwhile, it will disable Testpmd
cross NUMA scenario.

Fixes: dbfb8ec ("app/testpmd: optimize mbuf pool allocation")

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>
---
 app/test-pmd/testpmd.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
  

Comments

Ananyev, Konstantin Oct. 17, 2018, 11:02 a.m. UTC | #1
> -----Original Message-----
> From: phil.yang@arm.com [mailto:phil.yang@arm.com]
> Sent: Wednesday, October 17, 2018 2:37 AM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: [PATCH v2] app/testpmd: fix physic port socket initialization
> 
> Once the lcore list setting excluded the socket which physic device
> attached, it will cause failure. Meanwhile, it will disable Testpmd
> cross NUMA scenario.
> 
> Fixes: dbfb8ec ("app/testpmd: optimize mbuf pool allocation")
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>
> ---
>  app/test-pmd/testpmd.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 5dbbf78..fd718b0 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -565,9 +565,21 @@ set_default_fwd_ports_config(void)
>  	portid_t pt_id;
>  	int i = 0;
> 
> -	RTE_ETH_FOREACH_DEV(pt_id)
> +	RTE_ETH_FOREACH_DEV(pt_id) {
>  		fwd_ports_ids[i++] = pt_id;
> 
> +		/* Update sockets info according to the attached device */
> +		int socket_id = rte_eth_dev_socket_id(pt_id);
> +		if (socket_id >= 0 && new_socket_id(socket_id)) {
> +			if (num_sockets >= RTE_MAX_NUMA_NODES) {
> +				rte_exit(EXIT_FAILURE,
> +					 "Total sockets greater than %u\n",
> +					 RTE_MAX_NUMA_NODES);
> +			}
> +			socket_ids[num_sockets++] = socket_id;
> +		}
> +	}
> +
>  	nb_cfg_ports = nb_ports;
>  	nb_fwd_ports = nb_ports;
>  }
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.7.4
  
Ferruh Yigit Oct. 17, 2018, 1:04 p.m. UTC | #2
On 10/17/2018 12:02 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: phil.yang@arm.com [mailto:phil.yang@arm.com]
>> Sent: Wednesday, October 17, 2018 2:37 AM
>> To: dev@dpdk.org
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: [PATCH v2] app/testpmd: fix physic port socket initialization
>>
>> Once the lcore list setting excluded the socket which physic device
>> attached, it will cause failure. Meanwhile, it will disable Testpmd
>> cross NUMA scenario.
>>
>> Fixes: dbfb8ec ("app/testpmd: optimize mbuf pool allocation")
>>
>> Signed-off-by: Phil Yang <phil.yang@arm.com>
>> Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

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



What about 3rd item discussed before,
`port-numa-config` and `rxring-numa-config`

Currently if all cores and devices are in socket 0, there is no way for user to
ask allocating memory from socket 1. Again this happened after optimization patch.

For above case, when `port-numa-config` used to ask memory from socket 1:
`port-numa-config=(0,1)` testpmd will give error because no memory can be
allocated from socket 1.

What do you think, if user explicitly requested memory to be allocated from a
socket via `port-numa-config` and `rxring-numa-config`, and if that socket is
valid, add that socket into socket_ids[] so that mempool allocated for that socket?
  
Phil Yang Oct. 18, 2018, 2:37 a.m. UTC | #3
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, October 17, 2018 9:05 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Phil Yang (Arm
> Technology China) <Phil.Yang@arm.com>; dev@dpdk.org
> Subject: Re: [PATCH v2] app/testpmd: fix physic port socket initialization
> 
> On 10/17/2018 12:02 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: phil.yang@arm.com [mailto:phil.yang@arm.com]
> >> Sent: Wednesday, October 17, 2018 2:37 AM
> >> To: dev@dpdk.org
> >> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh
> >> <ferruh.yigit@intel.com>
> >> Subject: [PATCH v2] app/testpmd: fix physic port socket
> >> initialization
> >>
> >> Once the lcore list setting excluded the socket which physic device
> >> attached, it will cause failure. Meanwhile, it will disable Testpmd
> >> cross NUMA scenario.
> >>
> >> Fixes: dbfb8ec ("app/testpmd: optimize mbuf pool allocation")
> >>
> >> Signed-off-by: Phil Yang <phil.yang@arm.com>
> >> Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Applied to dpdk-next-net/master, thanks.
> 
> 
> 
> What about 3rd item discussed before,
> `port-numa-config` and `rxring-numa-config`
> 
> Currently if all cores and devices are in socket 0, there is no way for user to ask
> allocating memory from socket 1. Again this happened after optimization patch.
> 
> For above case, when `port-numa-config` used to ask memory from socket 1:
> `port-numa-config=(0,1)` testpmd will give error because no memory can be
> allocated from socket 1.
> 
> What do you think, if user explicitly requested memory to be allocated from a
> socket via `port-numa-config` and `rxring-numa-config`, and if that socket is
> valid, add that socket into socket_ids[] so that mempool allocated for that
> socket?

I was misunderstood about the 3rd item we discussed before. I thought 'port-numa-config' is used for picking up one of the available socket in socket_ids[].

I agree with your suggestion about adding the 'port-numa-config' specified socket into socket_ids[]. I can fix it. Thanks
  

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5dbbf78..fd718b0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -565,9 +565,21 @@  set_default_fwd_ports_config(void)
 	portid_t pt_id;
 	int i = 0;
 
-	RTE_ETH_FOREACH_DEV(pt_id)
+	RTE_ETH_FOREACH_DEV(pt_id) {
 		fwd_ports_ids[i++] = pt_id;
 
+		/* Update sockets info according to the attached device */
+		int socket_id = rte_eth_dev_socket_id(pt_id);
+		if (socket_id >= 0 && new_socket_id(socket_id)) {
+			if (num_sockets >= RTE_MAX_NUMA_NODES) {
+				rte_exit(EXIT_FAILURE,
+					 "Total sockets greater than %u\n",
+					 RTE_MAX_NUMA_NODES);
+			}
+			socket_ids[num_sockets++] = socket_id;
+		}
+	}
+
 	nb_cfg_ports = nb_ports;
 	nb_fwd_ports = nb_ports;
 }