[ovs-dev] [PATCH] Update fake bond devices' statistics with the sum of bond slaves' stats.

Ben Pfaff 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".

Thanks, fixed.

> 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 mailing list