Hi Ilya,
On 1/17/22 19:01, Ilya Maximets wrote:
On 1/5/22 09:19, Maxime Coquelin wrote:
Hash-based Tx steering feature will enable steering Tx
packets on transmit queues based on their hashes. In order
to test the feature, it is needed to be able to get the
per-queue statistics for Vhost-user ports.
This patch introduces "bytes", "packets" and "error"
per-queue custom statistics for Vhost-user ports.
Suggested-by David Marchand <[email protected]>
Signed-off-by: Maxime Coquelin <[email protected]>
Reviewed-by: David Marchand <[email protected]>
---
lib/netdev-dpdk.c | 147 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 138 insertions(+), 9 deletions(-)
Hi, Maxime.
Thanks for the patch; and I really think that it's an important
feature for debugging performance issues in a real-world setups.
However, it causes a performance drop by about 2-2.5% for me
with the VM-VM bidirectional traffic with 2 PMD threads.
The reason is the existing stats_lock. Unfortunately, in current
code, we're taking the same stats_lock on both rx and tx paths,
and since rx and tx are likely performed by different threads at
the same time, they are frequently locking each other.
Under this circumstances even a slight increase of a critical
section causes a visible performance drop.
One of the possible solutions might be to split the stats_lock
in two (one for rx stats and one for tx stats). We also should
split or re-align to different cache lines rx and tx fields
of the generic struct netdev_stats, or count all the stats on the
per-queue basis.
Quick prototype of such a solution gives an extra 2-3% performance
boost over the current master and reduces the impact of extra
stats in this patch to a minimum.
I'll polish and submit my prototype code sometime later.
For now, I think, we won't be able to accept this change for 2.17,
since some more development is needed to avoid regression.
I'm currently working on supporting the new Vhost per queue stats API in
OVS. Have you posted the prototype you did? I cannot find it, and think
it would be better to be applied before my series.
Thanks,
Maxime
There is also a memory leak in this code, but that can be easily
fixed:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 0e1efefe3..f8680a058 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1549,6 +1549,9 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
dev->vhost_id = NULL;
rte_free(dev->vhost_rxq_enabled);
+ free(dev->vhost_rxq_stats);
+ free(dev->vhost_txq_stats);
+
common_destruct(dev);
ovs_mutex_unlock(&dpdk_mutex);
---
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
