[dpdk-dev] [PATCH] app/testpmd: fix physic port socket initialization

[dpdk-dev] [PATCH] app/testpmd: fix physic port socket initialization

[Phil Yang]

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..4739ddb 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(pt_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;
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: fix physic port socket initialization

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org]
> Sent: Tuesday, October 16, 2018 11:15 AM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; nd@arm.com
> Subject: [dpdk-dev] [PATCH] 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..4739ddb 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(pt_id)) {

Should be: ' new_socket_id(socket_id)', I think.
Konstantin

> +			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;
>  }
> --
> 2.7.4

[dpdk-dev] [PATCH v2] app/testpmd: fix physic port socket initialization

[Phil Yang]

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;
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: fix physic port socket initialization

[Phil Yang (Arm Technology China)]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ananyev, Konstantin
> Sent: Tuesday, October 16, 2018 6:40 PM
> To: dev <dev-bounces@dpdk.org>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix physic port socket initialization
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org]
> > Sent: Tuesday, October 16, 2018 11:15 AM
> > To: dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; nd@arm.com
> > Subject: [dpdk-dev] [PATCH] 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..4739ddb 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(pt_id)) {
> 
> Should be: ' new_socket_id(socket_id)', I think.
> Konstantin

Exactly.
Thanks Konstantin. It has been fixed in V2.

> 
> > +			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;
> >  }
> > --
> > 2.7.4

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix physic port socket initialization

[Ananyev, Konstantin]

> -----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

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix physic port socket initialization

[Ferruh Yigit]

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?

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix physic port socket initialization

[Phil Yang (Arm Technology China)]

> -----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