Support IFCOUNTER_OBYTES and IFCOUNTER_OMCASTS even with VTNET_LEGACY_TX defined

[joyul_juniper.net]

Summary

vtxs_obytes and vtxs_omcasts are updated in vtnet_txq_eof, which is not bound with VTNET_LEGACY_TX. Thus, both should be supported regardless of whether VTNET_LEGACY_TX is defined or not.

Test Plan

Use if_get_counter API with IFCOUNTER_OBYTES and make sure output byte counts are updated.

[glebius]

Looks like driver does "hardware" accounting and software accounting at the same time. That's why we see correct counts without your patches. If vtnet_accum_stats() provides us with all what we need, then the driver should declare IFCAP_HWSTATS in its capabilities. That will turn off software accounting by the network stack. May I ask you to add this and also combine D44816 and D44817 into a single review?

[zlei]

sys/dev/virtio/network/if_vtnet.c

3015

I believe the condition is intentional.

VTNET_LEGACY_TX is defined when ALTQ is enabled [1]. ALTQ will do statistics by itself [2].

So this removal will end up double accounting of IFCOUNTER_OBYTES and IFCOUNTER_OMCASTS ( when ALTQ is enabled ).

1. https://cgit.freebsd.org/src/tree/sys/dev/virtio/network/if_vtnetvar.h#n33
2. https://cgit.freebsd.org/src/tree/sys/net/ifq.h#n251

[zlei]

In D44817#1021811, @glebius wrote:

Looks like driver does "hardware" accounting and software accounting at the same time. That's why we see correct counts without your patches. If vtnet_accum_stats() provides us with all what we need, then the driver should declare IFCAP_HWSTATS in its capabilities. That will turn off software accounting by the network stack. May I ask you to add this and also combine D44816 and D44817 into a single review?

In D44817#1021811, @glebius wrote:

Looks like driver does "hardware" accounting and software accounting at the same time. That's why we see correct counts without your patches. If vtnet_accum_stats() provides us with all what we need, then the driver should declare IFCAP_HWSTATS in its capabilities. That will turn off software accounting by the network stack. May I ask you to add this and also combine D44816 and D44817 into a single review?

sys/dev/virtio/network/if_vtnet.c

3015

I believe the condition is intentional.

VTNET_LEGACY_TX is defined when ALTQ is enabled [1]. ALTQ will do statistics by itself [2].

So this removal will end up double accounting of IFCOUNTER_OBYTES and IFCOUNTER_OMCASTS ( when ALTQ is enabled ).

I must be drunk. That should be harmless regardless ALTQ. vtnet_accum_stats() accounts hardware statistics only. No double accounting indeed.

1. https://cgit.freebsd.org/src/tree/sys/dev/virtio/network/if_vtnetvar.h#n33
2. https://cgit.freebsd.org/src/tree/sys/net/ifq.h#n251

@glebius
Currently IFCAP_HWSTATS is only for inbound packets. Shall we extend it also for outbound packets ?