[dpdk-dev] [PATCH] app/testpmd: fix little perf drop with XL710

[dpdk-dev] [PATCH] app/testpmd: fix little perf drop with XL710

[Xiaoyun Li]

There is about 1.8M perf drop with XL710. And it is because of a
bitrate calculation in the datapath. So improve it by maintaining
an array of port indexes in testpmd, which is updated with ethdev
events.

Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 app/test-pmd/testpmd.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..0600806 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -127,6 +127,8 @@ portid_t nb_ports;             /**< Number of probed ethernet ports. */
 struct fwd_lcore **fwd_lcores; /**< For all probed logical cores. */
 lcoreid_t nb_lcores;           /**< Number of probed logical cores. */
 
+portid_t ports_ids[RTE_MAX_ETHPORTS]; /**< Store all port ids. */
+
 /*
  * Test Forwarding Configuration.
  *    nb_fwd_lcores <= nb_cfg_lcores <= nb_lcores
@@ -1148,8 +1150,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 	uint64_t tics_per_1sec;
 	uint64_t tics_datum;
 	uint64_t tics_current;
-	uint16_t idx_port;
+	uint16_t i, cnt_ports;
 
+	cnt_ports = nb_ports;
 	tics_datum = rte_rdtsc();
 	tics_per_1sec = rte_get_timer_hz();
 #endif
@@ -1164,9 +1167,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			tics_current = rte_rdtsc();
 			if (tics_current - tics_datum >= tics_per_1sec) {
 				/* Periodic bitrate calculation */
-				RTE_ETH_FOREACH_DEV(idx_port)
+				for (i = 0; i < cnt_ports; i++)
 					rte_stats_bitrate_calc(bitrate_data,
-						idx_port);
+						ports_ids[i]);
 				tics_datum = tics_current;
 			}
 		}
@@ -2196,16 +2199,31 @@ static void
 eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
 			     __rte_unused void *arg)
 {
+	portid_t port_id;
+	uint16_t i;
+
 	if (type >= RTE_DEV_EVENT_MAX) {
 		fprintf(stderr, "%s called upon invalid event %d\n",
 			__func__, type);
 		fflush(stderr);
 	}
 
+	rte_eth_dev_get_port_by_name(device_name, &port_id);
+
 	switch (type) {
 	case RTE_DEV_EVENT_REMOVE:
 		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
 			device_name);
+
+		for (i = 0; i < nb_ports; i++) {
+			if (ports_ids[i] == port_id) {
+				ports_ids[i] = ports_ids[nb_ports-1];
+				ports_ids[nb_ports-1] = 0;
+				nb_ports--;
+				break;
+			}
+		}
+
 		/* TODO: After finish failure handle, begin to stop
 		 * packet forward, stop port, close port, detach port.
 		 */
@@ -2213,6 +2231,10 @@ eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
 	case RTE_DEV_EVENT_ADD:
 		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
 			device_name);
+
+		ports_ids[nb_ports] = port_id;
+		nb_ports++;
+
 		/* TODO: After finish kernel driver binding,
 		 * begin to attach port.
 		 */
@@ -2652,6 +2674,7 @@ main(int argc, char** argv)
 {
 	int diag;
 	portid_t port_id;
+	uint16_t count;
 	int ret;
 
 	signal(SIGINT, signal_handler);
@@ -2671,7 +2694,12 @@ main(int argc, char** argv)
 	rte_pdump_init(NULL);
 #endif
 
-	nb_ports = (portid_t) rte_eth_dev_count_avail();
+	count = 0;
+	RTE_ETH_FOREACH_DEV(port_id) {
+		ports_ids[count] = port_id;
+		count++;
+	}
+	nb_ports = (portid_t) count;
 	if (nb_ports == 0)
 		TESTPMD_LOG(WARNING, "No probed ethernet devices\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: fix little perf drop with XL710

[Zhang, Qi Z]

Hi Xiaoyun:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xiaoyun Li
> Sent: Tuesday, July 10, 2018 10:28 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] app/testpmd: fix little perf drop with XL710
> 
> There is about 1.8M perf drop with XL710. And it is because of a bitrate
> calculation in the datapath. So improve it by maintaining an array of port
> indexes in testpmd, which is updated with ethdev events.
> 
> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
<...>

> @@ -2196,16 +2199,31 @@ static void
>  eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
>  			     __rte_unused void *arg)

Do we also need to update ports_ids when user attach or detach a port from command line? (see function attach_port and detach_port).
I think eth_dev_event_callback is only for the case to handle udev event when a hardware hotplug happen.


>  {
> +	portid_t port_id;
> +	uint16_t i;
> +
>  	if (type >= RTE_DEV_EVENT_MAX) {
>  		fprintf(stderr, "%s called upon invalid event %d\n",
>  			__func__, type);
>  		fflush(stderr);
>  	}
> 
> +	rte_eth_dev_get_port_by_name(device_name, &port_id);

It's better to check the return value to make sure we get a valid port_id.

Regards
Qi

<...>

[dpdk-dev] [PATCH v2] app/testpmd: fix little perf drop with XL710

[Xiaoyun Li]

There is about 1.8M perf drop with XL710. And it is because of a
bitrate calculation in the datapath. So improve it by maintaining
an array of port indexes in testpmd, which is updated with ethdev
events.

Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2:
* Update ports_ids when user attach or detach a port.
---
 app/test-pmd/testpmd.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dde7d43..e4f39be 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -127,6 +127,8 @@ portid_t nb_ports;             /**< Number of probed ethernet ports. */
 struct fwd_lcore **fwd_lcores; /**< For all probed logical cores. */
 lcoreid_t nb_lcores;           /**< Number of probed logical cores. */
 
+portid_t ports_ids[RTE_MAX_ETHPORTS]; /**< Store all port ids. */
+
 /*
  * Test Forwarding Configuration.
  *    nb_fwd_lcores <= nb_cfg_lcores <= nb_lcores
@@ -1147,8 +1149,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 	uint64_t tics_per_1sec;
 	uint64_t tics_datum;
 	uint64_t tics_current;
-	uint16_t idx_port;
+	uint16_t i, cnt_ports;
 
+	cnt_ports = nb_ports;
 	tics_datum = rte_rdtsc();
 	tics_per_1sec = rte_get_timer_hz();
 #endif
@@ -1163,9 +1166,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			tics_current = rte_rdtsc();
 			if (tics_current - tics_datum >= tics_per_1sec) {
 				/* Periodic bitrate calculation */
-				RTE_ETH_FOREACH_DEV(idx_port)
+				for (i = 0; i < cnt_ports; i++)
 					rte_stats_bitrate_calc(bitrate_data,
-						idx_port);
+						ports_ids[i]);
 				tics_datum = tics_current;
 			}
 		}
@@ -1968,6 +1971,7 @@ attach_port(char *identifier)
 	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
+	ports_ids[nb_ports] = pi;
 	nb_ports = rte_eth_dev_count_avail();
 
 	ports[pi].port_status = RTE_PORT_STOPPED;
@@ -1982,6 +1986,7 @@ void
 detach_port(portid_t port_id)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t i;
 
 	printf("Detaching a port...\n");
 
@@ -1998,6 +2003,13 @@ detach_port(portid_t port_id)
 		return;
 	}
 
+	for (i = 0; i < nb_ports; i++) {
+		if (ports_ids[i] == port_id) {
+			ports_ids[i] = ports_ids[nb_ports-1];
+			ports_ids[nb_ports-1] = 0;
+			break;
+		}
+	}
 	nb_ports = rte_eth_dev_count_avail();
 
 	update_fwd_ports(RTE_MAX_ETHPORTS);
@@ -2649,6 +2661,7 @@ main(int argc, char** argv)
 {
 	int diag;
 	portid_t port_id;
+	uint16_t count;
 	int ret;
 
 	signal(SIGINT, signal_handler);
@@ -2668,7 +2681,12 @@ main(int argc, char** argv)
 	rte_pdump_init(NULL);
 #endif
 
-	nb_ports = (portid_t) rte_eth_dev_count_avail();
+	count = 0;
+	RTE_ETH_FOREACH_DEV(port_id) {
+		ports_ids[count] = port_id;
+		count++;
+	}
+	nb_ports = (portid_t) count;
 	if (nb_ports == 0)
 		TESTPMD_LOG(WARNING, "No probed ethernet devices\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix little perf drop with XL710

[Zhang, Qi Z]

> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Wednesday, July 11, 2018 10:16 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] app/testpmd: fix little perf drop with XL710
> 
> There is about 1.8M perf drop with XL710. And it is because of a bitrate
> calculation in the datapath. So improve it by maintaining an array of port
> indexes in testpmd, which is updated with ethdev events.
> 
> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix little perf drop with XL710

[Lu, Wenzhuo]

Hi Xiaoyun,


> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Wednesday, July 11, 2018 10:16 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] app/testpmd: fix little perf drop with XL710
> 
> There is about 1.8M perf drop with XL710. And it is because of a bitrate
What's 1.8M mean? BPS? PPS?
Looks like this patch fixes the CPU consuming problem and has nothing to do with a specific NIC.
2 suggestions.
Just omit XL710, also in the tittle.
Maybe better mentioning how many percent drop but not an accurate number.

Except that, Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>


> calculation in the datapath. So improve it by maintaining an array of port
> indexes in testpmd, which is updated with ethdev events.
> 
> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix little perf drop with XL710

[Li, Xiaoyun]

OK. I will modify the commit log and name and send v3 later. Thanks.

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Thursday, July 12, 2018 13:56
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v2] app/testpmd: fix little perf drop with XL710
> 
> Hi Xiaoyun,
> 
> 
> > -----Original Message-----
> > From: Li, Xiaoyun
> > Sent: Wednesday, July 11, 2018 10:16 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] app/testpmd: fix little perf drop with XL710
> >
> > There is about 1.8M perf drop with XL710. And it is because of a
> > bitrate
> What's 1.8M mean? BPS? PPS?
> Looks like this patch fixes the CPU consuming problem and has nothing to do
> with a specific NIC.
> 2 suggestions.
> Just omit XL710, also in the tittle.
> Maybe better mentioning how many percent drop but not an accurate
> number.
> 
> Except that, Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> 
> > calculation in the datapath. So improve it by maintaining an array of
> > port indexes in testpmd, which is updated with ethdev events.
> >
> > Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

[dpdk-dev] [PATCH v3] app/testpmd: fix little perf drop

[Xiaoyun Li]

There is about 3% perf drop. And it is because of a bitrate
calculation in the datapath. So improve it by maintaining an array
of port indexes in testpmd, which is updated with ethdev events.

Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v3:
* Modify the commit log and patch name.
v2:
* Update ports_ids when user attach or detach a port.
---
 app/test-pmd/testpmd.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dde7d43..e4f39be 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -127,6 +127,8 @@ portid_t nb_ports;             /**< Number of probed ethernet ports. */
 struct fwd_lcore **fwd_lcores; /**< For all probed logical cores. */
 lcoreid_t nb_lcores;           /**< Number of probed logical cores. */
 
+portid_t ports_ids[RTE_MAX_ETHPORTS]; /**< Store all port ids. */
+
 /*
  * Test Forwarding Configuration.
  *    nb_fwd_lcores <= nb_cfg_lcores <= nb_lcores
@@ -1147,8 +1149,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 	uint64_t tics_per_1sec;
 	uint64_t tics_datum;
 	uint64_t tics_current;
-	uint16_t idx_port;
+	uint16_t i, cnt_ports;
 
+	cnt_ports = nb_ports;
 	tics_datum = rte_rdtsc();
 	tics_per_1sec = rte_get_timer_hz();
 #endif
@@ -1163,9 +1166,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			tics_current = rte_rdtsc();
 			if (tics_current - tics_datum >= tics_per_1sec) {
 				/* Periodic bitrate calculation */
-				RTE_ETH_FOREACH_DEV(idx_port)
+				for (i = 0; i < cnt_ports; i++)
 					rte_stats_bitrate_calc(bitrate_data,
-						idx_port);
+						ports_ids[i]);
 				tics_datum = tics_current;
 			}
 		}
@@ -1968,6 +1971,7 @@ attach_port(char *identifier)
 	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
+	ports_ids[nb_ports] = pi;
 	nb_ports = rte_eth_dev_count_avail();
 
 	ports[pi].port_status = RTE_PORT_STOPPED;
@@ -1982,6 +1986,7 @@ void
 detach_port(portid_t port_id)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t i;
 
 	printf("Detaching a port...\n");
 
@@ -1998,6 +2003,13 @@ detach_port(portid_t port_id)
 		return;
 	}
 
+	for (i = 0; i < nb_ports; i++) {
+		if (ports_ids[i] == port_id) {
+			ports_ids[i] = ports_ids[nb_ports-1];
+			ports_ids[nb_ports-1] = 0;
+			break;
+		}
+	}
 	nb_ports = rte_eth_dev_count_avail();
 
 	update_fwd_ports(RTE_MAX_ETHPORTS);
@@ -2649,6 +2661,7 @@ main(int argc, char** argv)
 {
 	int diag;
 	portid_t port_id;
+	uint16_t count;
 	int ret;
 
 	signal(SIGINT, signal_handler);
@@ -2668,7 +2681,12 @@ main(int argc, char** argv)
 	rte_pdump_init(NULL);
 #endif
 
-	nb_ports = (portid_t) rte_eth_dev_count_avail();
+	count = 0;
+	RTE_ETH_FOREACH_DEV(port_id) {
+		ports_ids[count] = port_id;
+		count++;
+	}
+	nb_ports = (portid_t) count;
 	if (nb_ports == 0)
 		TESTPMD_LOG(WARNING, "No probed ethernet devices\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix little perf drop

[Lu, Wenzhuo]

Hi,

> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Thursday, July 12, 2018 3:56 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> Subject: [PATCH v3] app/testpmd: fix little perf drop
> 
> There is about 3% perf drop. And it is because of a bitrate calculation in the
> datapath. So improve it by maintaining an array of port indexes in testpmd,
> which is updated with ethdev events.
> 
> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [dpdk-stable] [PATCH v3] app/testpmd: fix little perf drop

[Ferruh Yigit]

On 7/16/2018 4:02 AM, Lu, Wenzhuo wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Li, Xiaoyun
>> Sent: Thursday, July 12, 2018 3:56 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>
>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
>> Subject: [PATCH v3] app/testpmd: fix little perf drop
>>
>> There is about 3% perf drop. And it is because of a bitrate calculation in the
>> datapath. So improve it by maintaining an array of port indexes in testpmd,
>> which is updated with ethdev events.
>>
>> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

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

Is it possible that someone from test team confirm that performance recovered
after this commit and report to mail list?

Thanks,
ferruh