[ovs-dev] [PATCH v2] Detailed packet drop statistics per dpdk and vhostuser ports

Sriram Vatala sriram.v at altencalsoftlabs.com
Wed Jun 19 10:48:08 UTC 2019


Hi Kevin,

Thanks for reviewing the patch. Please find my answers to your comments below.

Comment-1
I'm not sure about this name, you would need to know that it was the
only reason rte_eth_tx_burst() and rte_vhost_enqueue_burst() will not
send pkts.
---
Agree that queue full is not the only reason for which the above DPDK
apis will fail to send packets. But queue full is the most common reason.
Instead of overlooking other reasons like invalid queue-id, i can change
the name of the counter to a generic name something like "tx_failed".
what do you think?

Comment-2
There can be other drops earlier in this function
("__netdev_dpdk_vhost_send"), should they be logged also?
---
These are the drops for invalid queue-id or vhost device id and if device is
not up.
Again these are very unlikely to happen, but i can add a rate limiting warn
log.

Comment-3
>      rte_spinlock_lock(&dev->stats_lock);
>      netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts,
> +                                         dropped);
> +    dev->stats.tx_mtu_drops += mtu_drops;

There is a function just above for updating vhost tx stats. Now the
updates are split with some updated in the function and some updated
here, it would be better to update them all in the same place.
---
Agree. will change the implementation here.

Comment-4
>
> +    dropped += mtu_drops + qos_drops + qfull_drops;

= is enough, you don't need +=
---
Actually in the code above to this part in "dpdk_do_tx_copy" function, dropped
variable Will be updated if mbuf alloc fails.
Here is the code snippet:


        pkts[txcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
        if (OVS_UNLIKELY(!pkts[txcnt])) {
            dropped += cnt - i;
            break;
        }

To take care not to miss this in total drops, i am using dropped+=, Even if
the above part of the code doesn't hit, as dropped variable is initialized to
Zero, that expression should not result in incorrect value for drops.


Comment-5

> +    <command label="ovs-vsctl-get-interface-statistics"
> +
> filters="ovs">/usr/share/openvswitch/scripts/ovs-bugtool-get-iface-stats
> +    </command>

Probably better to be consistent and not wrap this line
---
Ok. I wrapped this line as utilities/checkpatch script is giving warning as
the line length is exceeding 79 characters. I will unwrap this.



Comment-6

> I think you also need to update vswitchd.xml with some docs for these stats
>

Actually, it doesn't seem needed there, perhaps something in the dpdk docs

---
Bit unclear on where to document this new counters. As we have not done any
modifications to dpdk APIs, can i document these new counters in man page of
ovs-vsctl.
what do you think?

Thanks & Regards,
Sriram.

-----Original Message-----
From: Kevin Traynor <ktraynor at redhat.com>
Sent: 19 June 2019 01:09
To: Sriram Vatala <sriram.v at altencalsoftlabs.com>; ovs-dev at openvswitch.org
Cc: Ilya Maximets <i.maximets at samsung.com>
Subject: Re: [ovs-dev] [PATCH v2] Detailed packet drop statistics per dpdk and
vhostuser ports

On 18/06/2019 15:05, Kevin Traynor wrote:
> Hi Sriram,
>
> On 14/06/2019 14:38, Sriram Vatala via dev wrote:
>> OVS may be unable to transmit packets for multiple reasons and today
>> there is a single counter to track packets dropped due to any of
>> those reasons. The most common reason is that a VM is unable to read
>> packets fast enough causing the vhostuser port transmit queue on the
>> OVS side to become full. This manifests as a problem with VNFs not
>> receiving all packets. Having a separate drop counter to track
>> packets dropped because the transmit queue is full will clearly
>> indicate that the problem is on the VM side and not in OVS. Similarly
>> maintaining separate counters for all possible drops helps in
>> indicating sensible cause for packet drops.
>>
>> This patch adds counters to track packets dropped due to all possible
>> reasons and these counters are displayed along with other stats in
>> "ovs-vsctl get interface <iface> statistics"
>> command. The detailed stats will be available for both dpdk and
>> vhostuser ports.
>>
>> Following are the details of the new counters :
>>
>> tx_qfull_drops : These are the packets dropped due to transmit queue
>> overrun.
>>
>
> I'm not sure about this name, you would need to know that it was the
> only reason rte_eth_tx_burst() and rte_vhost_enqueue_burst() will not
> send pkts
>
>> tx_mtu_exceeded_drops : These are the packets dropped due to MTU
>> mismatch (i.e Pkt len > Max Dev MTU).
>>
>> tx_qos_drops/rx_qos_drops : These are the packets dropped due to
>> transmission/reception rate exceeding the configured Egress/Ingress
>> policer rate on the interface.
>>
>
> I think you also need to update vswitchd.xml with some docs for these
> stats
>

Actually, it doesn't seem needed there, perhaps something in the dpdk docs

<snip>

>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index
>> 0702cc6..2cb5558 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -2407,7 +2407,11 @@ iface_refresh_stats(struct iface *iface)
>>      IFACE_STAT(rx_undersized_errors,    "rx_undersized_errors")     \
>>      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(rx_qos_drops,            "rx_qos_drops")             \
>> +    IFACE_STAT(tx_qfull_drops,          "tx_qfull_drops")           \
>> +    IFACE_STAT(tx_qos_drops,            "tx_qos_drops")             \
>> +    IFACE_STAT(tx_mtu_drops,            "tx_mtu_exceeded_drops")
>>
>>  #define IFACE_STAT(MEMBER, NAME) + 1
>>      enum { N_IFACE_STATS = IFACE_STATS };
>>
>
> What about updating show_dpif() to print with ovs-appctl dpctl/show -s ?
>

scratch that comment, I since saw the discussion on v1

> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list