[ovs-dev] [PATCH] netdev: Fix stats for ovs internal device.

[Ben Pfaff]

On Tue, Feb 28, 2012 at 06:08:56PM -0800, Pravin Shelar wrote:
> On Tue, Feb 28, 2012 at 5:09 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Fri, Feb 24, 2012 at 04:10:02PM -0800, Pravin B Shelar wrote:
> >> There is no need to retrieve linux system stats for internal devices
> >> as all relevant stats for virtual device like internal device are
> >> already reported by OVS over vport-stats.
> >>
> >> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
> >
> > It looks OK to me.  Thanks.
> >
> > Is this really a fix, though, as the first line of the commit message
> > says?  I don't think that there is a bug, except a performance, so
> > maybe "Optimize" instead of "Fix"?
> 
> ok, I forgot to mention. It does fixes a bug as it does not count
> error stats twice
> for internal devices.

OK, thanks, I assume you'll add that to the commit message in v2.

> > I noticed an oddity in existing code while I was reading it.  It looks
> > like netdev_linux_get_stats() will always log a (rate-limited) error
> > the first time that we call it on any given netdev that is not a vport
> > (or each time the cache gets invalidated).  Does it look like that to
> > you, too?  Maybe it doesn't show up because we only call this function
> > on netdevs that are vports.
> >
> I am not sure in which case vport wont be there for given netdev.

I mean, given this program "netdev-test.c":

    #include <config.h>
    #include "netdev.h"
    #include <inttypes.h>
    #include "util.h"

    int
    main(int argc, const char *argv[])
    {
        int i;

        for (i = 1; i < argc; i++) {
            struct netdev_stats stats;
            struct netdev *netdev;
            int error;

            error = netdev_open(argv[i], "system", &netdev);
            if (error) {
                ovs_fatal(error, "%s: open failed", argv[i]);
            }

            error = netdev_get_stats(netdev, &stats);
            if (error) {
                ovs_fatal(error, "%s: netdev_get_stats failed", argv[i]);
            }

            printf("%s: tx=%"PRIu64", rx=%"PRIu64"\n",
                   argv[i], stats.tx_packets, stats.rx_packets);

            netdev_close(netdev);
        }
        return 0;
    }

then we get results like the following if eth1 is not part of a
bridge:

    hda:~# netdev-test eth1
    2012-02-29T19:30:19Z|00001|netdev_linux|WARN|eth1: obtaining netdev stats via vport failed (No such device)
    eth1: tx=6, rx=0
    hda:~#