On 24/06/2019 22:47, Flavio Leitner wrote: > On Fri, Jun 21, 2019 at 02:41:57PM +0100, Kevin Traynor wrote: >> vhost tx retries may occur, and it can be a sign that >> the guest is not optimally configured. >> >> Add some stats so a user will know if vhost tx retries are >> occurring and hence give a hint that guest config should be >> examined. >> >> Signed-off-by: Kevin Traynor <ktray...@redhat.com> >> --- >> Documentation/topics/dpdk/vhost-user.rst | 5 +++++ >> include/openvswitch/netdev.h | 1 + >> lib/netdev-dpdk.c | 7 +++++-- >> vswitchd/bridge.c | 3 ++- >> 4 files changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst >> b/Documentation/topics/dpdk/vhost-user.rst >> index d1682fd05..e79ed082e 100644 >> --- a/Documentation/topics/dpdk/vhost-user.rst >> +++ b/Documentation/topics/dpdk/vhost-user.rst >> @@ -107,4 +107,9 @@ The guest should also have sufficient cores dedicated >> for consuming and >> processing packets at the required rate. >> >> +The amount of Tx retries on a vhost-user or vhost-user-client interface can >> be >> +shown with:: >> + >> + ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries >> + >> .. _dpdk-vhost-user: >> >> diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h >> index 0c10f7b48..4d18b9f66 100644 >> --- a/include/openvswitch/netdev.h >> +++ b/include/openvswitch/netdev.h >> @@ -46,4 +46,5 @@ struct netdev_stats { >> uint64_t multicast; /* Multicast packets received. */ >> uint64_t collisions; >> + uint64_t tx_retries; /* Retries when unable to transmit.*/ >> >> /* Detailed receive errors. */ >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 0f3b9c6f4..03a8de380 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -2298,5 +2298,6 @@ netdev_dpdk_vhost_update_tx_counters(struct >> netdev_stats *stats, >> struct dp_packet **packets, >> int attempted, >> - int dropped) >> + int dropped, >> + int retries) >> { >> int i; >> @@ -2305,4 +2306,5 @@ netdev_dpdk_vhost_update_tx_counters(struct >> netdev_stats *stats, >> stats->tx_packets += sent; >> stats->tx_dropped += dropped; >> + stats->tx_retries += MIN(retries, VHOST_ENQ_RETRY_NUM); > > Why MIN(, VHOST_ENQ_RETRY_NUM)? > The retrans loop is limited to that, so it seems redundant. >
Yes, you are right. It's not needed now, but will be needed in 3/3 when the max can be zero. I will move there and add a comment. > fbl > >> for (i = 0; i < sent; i++) { >> @@ -2359,5 +2361,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int >> qid, >> rte_spinlock_lock(&dev->stats_lock); >> netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts, >> - cnt + dropped); >> + cnt + dropped, retries); >> rte_spinlock_unlock(&dev->stats_lock); >> >> @@ -2597,4 +2599,5 @@ netdev_dpdk_vhost_get_stats(const struct netdev >> *netdev, >> stats->rx_errors = dev->stats.rx_errors; >> stats->rx_length_errors = dev->stats.rx_length_errors; >> + stats->tx_retries = dev->stats.tx_retries; >> >> stats->rx_1_to_64_packets = dev->stats.rx_1_to_64_packets; >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >> index 2976771ae..3dab8d4c7 100644 >> --- a/vswitchd/bridge.c >> +++ b/vswitchd/bridge.c >> @@ -2409,5 +2409,6 @@ iface_refresh_stats(struct iface *iface) >> IFACE_STAT(rx_oversize_errors, "rx_oversize_errors") \ >> IFACE_STAT(rx_fragmented_errors, "rx_fragmented_errors") \ >> - IFACE_STAT(rx_jabber_errors, "rx_jabber_errors") >> + IFACE_STAT(rx_jabber_errors, "rx_jabber_errors") \ >> + IFACE_STAT(tx_retries, "tx_retries") >> >> #define IFACE_STAT(MEMBER, NAME) + 1 >> -- >> 2.20.1 >> >> _______________________________________________ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev