Re: [dpdk-dev] [PATCH 1/3] examples/flow_classify: fix check of port and core

[Thomas Monjalon <thomas@monjalon.net>]

20/04/2021 04:26, Min Hu (Connor):
> 2021/4/20 9:08, Thomas Monjalon:
> > 27/03/2021 08:40, Min Hu (Connor):
> >> fix check of port and core in flow_classify example.
> >>
> >> Fixes: bab16ddaf2c1 ("examples/flow_classify: add sample application")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> ---
> >>   	RTE_ETH_FOREACH_DEV(port)
> >> -		if (rte_eth_dev_socket_id(port) > 0 &&
> >> +		if (rte_eth_dev_socket_id(port) >= 0 &&
> >>   			rte_eth_dev_socket_id(port) != (int)rte_socket_id()) {
> >>   			printf("\n\n");
> >>   			printf("WARNING: port %u is on remote NUMA node\n",
> > 
> > Please explain which case is broken and why.
> > If I understand well, we don't detect remote NUMA if not running on first socket.
> > 
> Hi, the code is this:
> *************************************************************************
> 	/*
> 	 * Check that the port is on the same NUMA node as the polling thread
> 	 * for best performance.
> 	 */
> 	RTE_ETH_FOREACH_DEV(port)
> 		if (rte_eth_dev_socket_id(port) > 0 &&
> 			rte_eth_dev_socket_id(port) != (int)rte_socket_id()) {
> 			printf("\n\n");
> 			printf("WARNING: port %u is on remote NUMA node\n",
> 			       port);
> 			printf("to polling thread.\n");
> 			printf("Performance will not be optimal.\n");
> 		}
> 	printf("\nCore %u forwarding packets. ", rte_lcore_id());
> 	printf("[Ctrl+C to quit]\n");
> *************************************************************************
> 
> According to the comments and logging, the author just hope user to use
> the core and device which are in the same numa node for optimal
> performance. If not, A warning gives out.
> 
> For example in flow_classify:
> ./build/flow_classify -w 0000:7d:00.1  -l 93
> Here:
> 0000:7d:00.1 is on numa node 0.
> core 93  is on numa node 3.
> 
> the two are not in same numa node, but no warning gives out in old codes.
> 
> Well, using this patch, we can get the waring.

You need to explain which case was broken in the commit log.
Thanks