Re: [ovs-dev] [PATCH 2/3] netdev-dpdk: Add stats for vhost tx retries.

2019-06-25 Thread Kevin Traynor 
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 
>> ---
>>  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(>stats_lock);
>>  netdev_dpdk_vhost_update_tx_counters(>stats, pkts, total_pkts,
>> - cnt + dropped);
>> + cnt + dropped, retries);
>>  rte_spinlock_unlock(>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

Re: [ovs-dev] [PATCH 2/3] netdev-dpdk: Add stats for vhost tx retries.

2019-06-25 Thread Eelco Chaudron 



On 21 Jun 2019, at 15:41, 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 
---
 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);


Don’t think this MIN() is needed here, see also comments in 3/3.



 for (i = 0; i < sent; i++) {
@@ -2359,5 +2361,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, 
int qid,

 rte_spinlock_lock(>stats_lock);
 netdev_dpdk_vhost_update_tx_counters(>stats, pkts, 
total_pkts,

- cnt + dropped);
+ cnt + dropped, retries);
 rte_spinlock_unlock(>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

Re: [ovs-dev] [PATCH 2/3] netdev-dpdk: Add stats for vhost tx retries.

2019-06-24 Thread Flavio Leitner via dev 
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 
> ---
>  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.

fbl
 
>  for (i = 0; i < sent; i++) {
> @@ -2359,5 +2361,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>  rte_spinlock_lock(>stats_lock);
>  netdev_dpdk_vhost_update_tx_counters(>stats, pkts, total_pkts,
> - cnt + dropped);
> + cnt + dropped, retries);
>  rte_spinlock_unlock(>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

[ovs-dev] [PATCH 2/3] netdev-dpdk: Add stats for vhost tx retries.

2019-06-21 Thread Kevin Traynor 
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 
---
 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);
 
 for (i = 0; i < sent; i++) {
@@ -2359,5 +2361,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 rte_spinlock_lock(>stats_lock);
 netdev_dpdk_vhost_update_tx_counters(>stats, pkts, total_pkts,
- cnt + dropped);
+ cnt + dropped, retries);
 rte_spinlock_unlock(>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