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

[Ben Pfaff]

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"?

Also, I'd start the commit message with "netdev-linux:" instead of
"netdev:" because it only changes Linux-specific netdev code.

It would be nice to put a comment on the "vport_stats_error" member,
maybe just as simple as /* 0 or an errno value. */, since it took me a
moment to understand its meaning.

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.

Some of your new code uses the new return value from
get_stats_via_vport(), some of your new code uses
netdev_dev->vport_stats_error after calling get_stats_via_vport().  It
is a little inconsistent.  I would be inclined to not add the new
return value and use netdev_dev->vport_stats_error in each case, but
the existing code looks correct and so it's not a big deal.

Thanks,

Ben.