[ovs-dev] [PATCH] Update fake bond devices' statistics with the sum of bond slaves' stats.
blp at nicira.com
Wed Apr 14 19:43:46 UTC 2010
On Wed, Apr 14, 2010 at 03:12:54PM -0400, Jesse Gross wrote:
> On Tue, Apr 13, 2010 at 8:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> > > Unfortunately this is all one big merge conflict with my stuff.
> > If you want to use my version, I'm happy to keep this back until your
> > big patch series is pushed out, and then I'll deal with the conflicts.
> I thought that we needed this ASAP?
I got the impression that we needed a plan to fix it ASAP. A patch is a
pretty good plan. So I'll hold back until you push your series or until
I'm told otherwise. No sense in making the difficult work harder.
> On the other fields that are null we have a comment saying what should be
> there. I find that helpful since I haven't memorized struct netdev_class.
Thanks, good point. Fixed.
> > + /* Sets the device stats for 'netdev' to 'stats'.
> > + *
> > + * Most network devices won't support this feature and will set this
> > + * function pointer to 0, which is equivalent to returning EOPNOTSUPP.
> > + * Additional, some network devices might only allow setting their
> > stats to
> > + * 0. */
> > + int (*set_stats)(struct netdev *netdev, const struct netdev_stats *);
> > +
> This still seems to refer to setting the pointer to 0. Also "additional"
> should be "additionally".
> One more thing: when you talk about setting stats to 0 it makes me think of
> resetting the counters when in reality this is setting the offset to 0.
> This sentence is also in netdev.c.
You're right. For our use case I think it's the same though. I added a
clarifying comment to netdev_linux_set_stats():
/* This actually only sets the *offset* that the dp_dev applies, but in our
* usage for fake bond devices the dp_dev never has any traffic of it own
* so it has the same effect. */
More information about the dev