[ovs-dev] [netlink 14/16] datapath: Use "struct rtnl_link_stats64" instead of "struct odp_vport_stats".

Ben Pfaff blp at nicira.com
Thu Nov 4 21:18:59 UTC 2010


On Fri, Oct 15, 2010 at 05:05:53PM -0700, Jesse Gross wrote:
> On Fri, Sep 10, 2010 at 3:55 PM, Ben Pfaff <blp at nicira.com> wrote:
> > -int netdev_get_stats(const struct vport *vport, struct odp_vport_stats *stats)
> > +int netdev_get_stats(const struct vport *vport, struct rtnl_link_stats64 *stats)
> >  {
> >        const struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
> >  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36)
> > @@ -227,9 +227,9 @@ int netdev_get_stats(const struct vport *vport, struct odp_vport_stats *stats)
> >        stats->tx_packets       = netdev_stats->tx_packets;
> >        stats->rx_dropped       = netdev_stats->rx_dropped;
> >        stats->rx_errors        = netdev_stats->rx_errors;
> > -       stats->rx_frame_err     = netdev_stats->rx_frame_errors;
> > -       stats->rx_over_err      = netdev_stats->rx_over_errors;
> > -       stats->rx_crc_err       = netdev_stats->rx_crc_errors;
> > +       stats->rx_frame_errors  = netdev_stats->rx_frame_errors;
> > +       stats->rx_over_errors   = netdev_stats->rx_over_errors;
> > +       stats->rx_crc_errors    = netdev_stats->rx_crc_errors;
> >        stats->tx_dropped       = netdev_stats->tx_dropped;
> >        stats->tx_errors        = netdev_stats->tx_errors;
> >        stats->collisions       = netdev_stats->collisions;
> 
> On newer kernels we can directly get rtnl_link_stats64 from
> dev_get_stats(), so we should be able to avoid all of this copying.

Good point.  I've written up a new series that fixes our compat layer so
that the dev_get_stats() interface always matches the 2.6.36+ interface.

> > @@ -872,9 +872,9 @@ int vport_get_stats(struct vport *vport, struct odp_vport_stats *stats)
> >                stats->tx_errors        += vport->err_stats.tx_errors;
> >                stats->tx_dropped       += vport->err_stats.tx_dropped;
> >                stats->rx_dropped       += vport->err_stats.rx_dropped;
> > -               stats->rx_over_err      += vport->err_stats.rx_over_err;
> > -               stats->rx_crc_err       += vport->err_stats.rx_crc_err;
> > -               stats->rx_frame_err     += vport->err_stats.rx_frame_err;
> > +               stats->rx_over_errors   += vport->err_stats.rx_over_err;
> > +               stats->rx_crc_errors    += vport->err_stats.rx_crc_err;
> > +               stats->rx_frame_errors  += vport->err_stats.rx_frame_err;
> >                stats->collisions       += vport->err_stats.collisions;
> 
> There are a bunch of new fields that are now available.  Should we try
> to use them?  In addition to having extra information available, it is
> a little strange that the offset stats will never include those fields
> (but then offset stats are more than a little strange to start off
> with).

You mean, should we add corresponding fields to err_stats and
percpu_stats?  I haven't done that, even now, but I've now changed the
code that adds the contents of 'dev_statsp' to 'stats' to add all of the
fields.

> > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > index 8806023..a191994 100644
> > --- a/lib/netdev-vport.c
> > +++ b/lib/netdev-vport.c
> > @@ -267,9 +267,9 @@ netdev_vport_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
> >     stats->multicast = UINT64_MAX;
> >     stats->collisions = ovsr.stats.collisions;
> >     stats->rx_length_errors = UINT64_MAX;
> > -    stats->rx_over_errors = ovsr.stats.rx_over_err;
> > -    stats->rx_crc_errors = ovsr.stats.rx_crc_err;
> > -    stats->rx_frame_errors = ovsr.stats.rx_frame_err;
> > +    stats->rx_over_errors = ovsr.stats.rx_over_errors;
> > +    stats->rx_crc_errors = ovsr.stats.rx_crc_errors;
> > +    stats->rx_frame_errors = ovsr.stats.rx_frame_errors;
> >     stats->rx_fifo_errors = UINT64_MAX;
> >     stats->rx_missed_errors = UINT64_MAX;
> >     stats->tx_aborted_errors = UINT64_MAX;
> 
> I think all of these OpenFlow fields that are marked as unsupported
> are now available in rtnl_link_stats64 (if we choose to populate
> them).

Thanks for pointing that out.  I've revised it now to copy all of the
members properly.

Thank you for all of your comments.  I've sent out a new 4-patch series
that fixes up all of this stuff.  It applies on the top of current
"master".




More information about the dev mailing list