Re: [dpdk-stable] [PATCH v2] app/testpmd: support non contiguous socket ids

["Wu, Jingjing" <jingjing.wu@intel.com>]

> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Wednesday, May 3, 2017 9:44 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; stable@dpdk.org
> Subject: [PATCH v2] app/testpmd: support non contiguous socket ids
> 
> The test assumes the socket ids are contiguous. This is not necessarily the case
> on all servers and may cause mempool creation to fail.
> 
> Fixing it by detecting the list of valid socket ids and use it for the mempool
> creation.
> 
> Fixes: 7acf894d07d1 ("app/testpmd: detect numa socket count")
> 
> CC: stable@dpdk.org
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
> on v2:
>  * fix minor typo on commit message : be->by.
> ---
>  app/test-pmd/parameters.c | 38 ++++++++++++++++++++++++++++----------
>  app/test-pmd/testpmd.c    | 38 +++++++++++++++++++++++++++++---------
>  app/test-pmd/testpmd.h    |  4 +++-
>  3 files changed, 60 insertions(+), 20 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> 787e1434c..4822a8a8a 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -410,9 +410,14 @@ parse_portnuma_config(const char *q_arg)
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> -		if(socket_id >= max_socket) {
> -			printf("Invalid socket id, range is [0, %d]\n",
> -				 max_socket - 1);
> +		if (new_socket_id(socket_id)) {
> +			unsigned int i = 0;
> +
> +			printf("Invalid socket id, options are: ");
> +			for (i = 0; i < num_sockets; i++) {
> +				printf("%u%s", socket_ids[i],
> +				(i == num_sockets - 1) ? "\n" : ",");
> +			}
>  			return -1;
>  		}
>  		port_numa[port_id] = socket_id;
> @@ -470,9 +475,14 @@ parse_ringnuma_config(const char *q_arg)
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> -		if (socket_id >= max_socket) {
> -			printf("Invalid socket id, range is [0, %d]\n",
> -				max_socket - 1);
> +		if (new_socket_id(socket_id)) {
> +			unsigned int i = 0;
> +
> +			printf("Invalid socket id, options are: ");
> +			for (i = 0; i < num_sockets; i++) {
> +				printf("%u%s", socket_ids[i],
> +				(i == num_sockets - 1) ? "\n" : ",");
> +			}
>  			return -1;
>  		}
>  		ring_flag = (uint8_t)int_fld[FLD_FLAG]; @@ -700,12 +710,20
> @@ launch_args_parse(int argc, char** argv)
>  					   "invalid ring-numa configuration\n");
>  			if (!strcmp(lgopts[opt_idx].name, "socket-num")) {
>  				n = atoi(optarg);
> -				if((uint8_t)n < max_socket)
> +				if (!new_socket_id((uint8_t)n)) {
>  					socket_num = (uint8_t)n;
> -				else
> +				} else {
> +					unsigned int i = 0;
> +
> +					printf("socket id options are: ");
> +					for (i = 0; i < num_sockets; i++) {
> +						printf("%u%s", socket_ids[i],
> +						(i == num_sockets - 1) ?
> +						"\n" : ",");
> +					}
>  					rte_exit(EXIT_FAILURE,
> -						"The socket number should be
> < %d\n",
> -						max_socket);
> +						"Invalid socket id");
> +				}
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "mbuf-size")) {
>  				n = atoi(optarg);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> dfe64427d..a556a8aff 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -354,7 +354,8 @@ struct queue_stats_mappings
> *rx_queue_stats_mappings = rx_queue_stats_mappings_a  uint16_t
> nb_tx_queue_stats_mappings = 0;  uint16_t nb_rx_queue_stats_mappings = 0;
> 
> -unsigned max_socket = 0;
> +unsigned int num_sockets = 0;
> +unsigned int socket_ids[RTE_MAX_NUMA_NODES];
> 
>  #ifdef RTE_LIBRTE_BITRATE
>  /* Bitrate statistics */
> @@ -377,6 +378,22 @@ static void eth_event_callback(uint8_t port_id,  static
> int all_ports_started(void);
> 
>  /*
> + * Helper function to check if socket is allready discovered.
> + * If yes, return positive value. If not, return zero.
> + */
> +int
> +new_socket_id(unsigned int socket_id)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < num_sockets; i++) {
> +		if (socket_ids[i] == socket_id)
> +			return 0;
> +	}
> +	return 1;
> +}
> +
> +/*
>   * Setup default configuration.
>   */
>  static void
> @@ -388,12 +405,14 @@ set_default_fwd_lcores_config(void)
> 
>  	nb_lc = 0;
>  	for (i = 0; i < RTE_MAX_LCORE; i++) {
> -		sock_num = rte_lcore_to_socket_id(i) + 1;
> -		if (sock_num > max_socket) {
> -			if (sock_num > RTE_MAX_NUMA_NODES)
> -				rte_exit(EXIT_FAILURE, "Total sockets greater
> than %u\n", RTE_MAX_NUMA_NODES);
> -			max_socket = sock_num;
> +		sock_num = rte_lcore_to_socket_id(i);
+1 is missed?