Virtio xstats problem

Virtio xstats problem

[Igor Ryzhov]

Hello.

I found some problem with virtio xstats counters.

Example:

rx_good_packets: 3
rx_good_bytes: 180
rx_errors: 0

rx_q0_good_packets: 3
rx_q0_good_bytes: 180
rx_q0_errors: 0

rx_q0_multicast_packets: 3
rx_q0_broadcast_packets: 1
rx_q0_undersize_packets: 3

It means that undersize packets are counted as good packets instead of errors. Or maybe size of packet is calculated wrong.
I don't have time now to check it more deeply - I can do it sometime later, but maybe someone want to help.

PS. Is it a common practice to count broadcast packets twice - in broadcast and multicast counters?

Best regards,
Igor

Re: Virtio xstats problem

[Van Haaren, Harry]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov
> I found some problem with virtio xstats counters.
> 
> Example:
> 
> rx_good_packets: 3
> rx_good_bytes: 180
> rx_errors: 0
> 
> rx_q0_good_packets: 3
> rx_q0_good_bytes: 180
> rx_q0_errors: 0
> 
> rx_q0_multicast_packets: 3
> rx_q0_broadcast_packets: 1
> rx_q0_undersize_packets: 3

> It means that undersize packets are counted as good packets instead of errors.

Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
interfaces, so 60 bytes per packet is OK.

> Or maybe
> size of packet is calculated wrong.
> I don't have time now to check it more deeply - I can do it sometime later, but maybe
> someone want to help.

Are the packets multicast or broadcast?
It looks like one of the counters there is wrong.

> PS. Is it a common practice to count broadcast packets twice - in broadcast and multicast
> counters?

No packet should be counted twice - it must be put into one bucket of mutli, broad or unicast.

-Harry

Re: Virtio xstats problem

[Igor Ryzhov]

Hello, Harry.

Understood about size of packets. It's a bit confusing, because in all other drivers undersized packet is an error. Maybe we should add another one size bin for virtio - 60 to 63 bytes?

I already checked about multicast/broadcast counters - broadcast packets are counted twice:

	vq->multicast += is_multicast_ether_addr(ea);
	vq->broadcast += is_broadcast_ether_addr(ea);

I think it should be something like:

	if (is_multicast_ether_addr(ea)) {
		if (is_broadcast_ether_addr(ea)) {
			vq->broadcast++;
		} else {
			vq->multicast++;
		}
	}

Best regards,
Igor

> 26 февр. 2016 г., в 17:29, Van Haaren, Harry <harry.van.haaren@intel.com> написал(а):
> 
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov
>> I found some problem with virtio xstats counters.
>> 
>> Example:
>> 
>> rx_good_packets: 3
>> rx_good_bytes: 180
>> rx_errors: 0
>> 
>> rx_q0_good_packets: 3
>> rx_q0_good_bytes: 180
>> rx_q0_errors: 0
>> 
>> rx_q0_multicast_packets: 3
>> rx_q0_broadcast_packets: 1
>> rx_q0_undersize_packets: 3
> 
>> It means that undersize packets are counted as good packets instead of errors.
> 
> Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
> interfaces, so 60 bytes per packet is OK.
> 
>> Or maybe
>> size of packet is calculated wrong.
>> I don't have time now to check it more deeply - I can do it sometime later, but maybe
>> someone want to help.
> 
> Are the packets multicast or broadcast?
> It looks like one of the counters there is wrong.
> 
>> PS. Is it a common practice to count broadcast packets twice - in broadcast and multicast
>> counters?
> 
> No packet should be counted twice - it must be put into one bucket of mutli, broad or unicast.
> 
> -Harry

Re: Virtio xstats problem

[Igor Ryzhov]

Sent a patch: http://dpdk.org/dev/patchwork/patch/10887/ <http://dpdk.org/dev/patchwork/patch/10887/>.

> 26 февр. 2016 г., в 17:35, Igor Ryzhov <iryzhov@nfware.com> написал(а):
> 
> Hello, Harry.
> 
> Understood about size of packets. It's a bit confusing, because in all other drivers undersized packet is an error. Maybe we should add another one size bin for virtio - 60 to 63 bytes?
> 
> I already checked about multicast/broadcast counters - broadcast packets are counted twice:
> 
> 	vq->multicast += is_multicast_ether_addr(ea);
> 	vq->broadcast += is_broadcast_ether_addr(ea);
> 
> I think it should be something like:
> 
> 	if (is_multicast_ether_addr(ea)) {
> 		if (is_broadcast_ether_addr(ea)) {
> 			vq->broadcast++;
> 		} else {
> 			vq->multicast++;
> 		}
> 	}
> 
> Best regards,
> Igor
> 
>> 26 февр. 2016 г., в 17:29, Van Haaren, Harry <harry.van.haaren@intel.com> написал(а):
>> 
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov
>>> I found some problem with virtio xstats counters.
>>> 
>>> Example:
>>> 
>>> rx_good_packets: 3
>>> rx_good_bytes: 180
>>> rx_errors: 0
>>> 
>>> rx_q0_good_packets: 3
>>> rx_q0_good_bytes: 180
>>> rx_q0_errors: 0
>>> 
>>> rx_q0_multicast_packets: 3
>>> rx_q0_broadcast_packets: 1
>>> rx_q0_undersize_packets: 3
>> 
>>> It means that undersize packets are counted as good packets instead of errors.
>> 
>> Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
>> interfaces, so 60 bytes per packet is OK.
>> 
>>> Or maybe
>>> size of packet is calculated wrong.
>>> I don't have time now to check it more deeply - I can do it sometime later, but maybe
>>> someone want to help.
>> 
>> Are the packets multicast or broadcast?
>> It looks like one of the counters there is wrong.
>> 
>>> PS. Is it a common practice to count broadcast packets twice - in broadcast and multicast
>>> counters?
>> 
>> No packet should be counted twice - it must be put into one bucket of mutli, broad or unicast.
>> 
>> -Harry
> 

Re: Virtio xstats problem

[Kyle Larose]

On Fri, Feb 26, 2016 at 9:35 AM, Igor Ryzhov <iryzhov@nfware.com> wrote:
> Hello, Harry.
>
> Understood about size of packets. It's a bit confusing, because in all other drivers undersized packet is an error. Maybe we should add another one size bin for virtio - 60 to 63 bytes?
>

I'm not sure we need to add a new bucket. According to the rmon MIB
(which defines histogram counters) the undersize bucket doesn't
necessarily mean that there was an error. It just means that the
packet was less than 64 bytes:

         "The total number of packets received that were
         less than 64 octets long (excluding framing bits,
         but including FCS octets) and were otherwise well
         formed."

This is from https://tools.ietf.org/html/rfc2819

Now, whether we want to align with the standard is up for debate, but
I think we should try to as much as possible. I guess we could add the
60-63 bucket, and then count those packets in undersized as well.
There's a similar problem when mapping large packets to the rmon mib.
It has an "oversized" bucket, which is everything above 1518. A packet
incrementing that bucket isn't an error; it's just the "everything
else" bucket in the histogram.

We could also talk about the bit about it "including FCS octets", but
things get a little tricky there. Do we want to assume that all
packets received on a virtio interface had a 4-byte FCS at some point,
and add 4 bytes to each packet? That will be true for anything coming
from the wire, but for inter-VM communication over a vswitch, it may
not be.

> I already checked about multicast/broadcast counters - broadcast packets are counted twice:
>
>         vq->multicast += is_multicast_ether_addr(ea);
>         vq->broadcast += is_broadcast_ether_addr(ea);
>
> I think it should be something like:
>
>         if (is_multicast_ether_addr(ea)) {
>                 if (is_broadcast_ether_addr(ea)) {
>                         vq->broadcast++;
>                 } else {
>                         vq->multicast++;
>                 }
>         }
>
> Best regards,
> Igor
>
>> 26 февр. 2016 г., в 17:29, Van Haaren, Harry <harry.van.haaren@intel.com> написал(а):
>>
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov
>>> I found some problem with virtio xstats counters.
>>>
>>> Example:
>>>
>>> rx_good_packets: 3
>>> rx_good_bytes: 180
>>> rx_errors: 0
>>>
>>> rx_q0_good_packets: 3
>>> rx_q0_good_bytes: 180
>>> rx_q0_errors: 0
>>>
>>> rx_q0_multicast_packets: 3
>>> rx_q0_broadcast_packets: 1
>>> rx_q0_undersize_packets: 3
>>
>>> It means that undersize packets are counted as good packets instead of errors.
>>
>> Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
>> interfaces, so 60 bytes per packet is OK.
>>
>>> Or maybe
>>> size of packet is calculated wrong.
>>> I don't have time now to check it more deeply - I can do it sometime later, but maybe
>>> someone want to help.
>>
>> Are the packets multicast or broadcast?
>> It looks like one of the counters there is wrong.
>>
>>> PS. Is it a common practice to count broadcast packets twice - in broadcast and multicast
>>> counters?
>>
>> No packet should be counted twice - it must be put into one bucket of mutli, broad or unicast.
>>
>> -Harry


Kyle