[ovs-dev] [PATCH 7/7] ovs-vswitchd: Export system stats through Open_vSwitch table.

[Ben Pfaff]

On Thu, Sep 23, 2010 at 01:53:27AM -0700, Justin Pettit wrote:
> On Sep 22, 2010, at 4:45 PM, Ben Pfaff wrote:
> 
> >     if (time_msec() >= iface_stats_timer) {
> > ...
> >         iface_stats_timer = time_msec() + IFACE_STATS_INTERVAL;
> 
> Should the name be changed, since it's not just interface stats anymore?

Sure, I changed it to STATS_INTERVAL.

(However a patch series I've had outstanding for many weeks actually
entirely deletes the periodic stats update.)

> I'm not that familiar with how Linux represents multi-core processors
> populating a multi-socket system.  Will this algorithm properly handle
> a pair of 4-core CPU, for example?  If the core_id is unique only to a
> processor, it seems like it will undercount.

Argh, you're right.  I made an assumption and it was wrong.  On a
2-socket, 4-core box (2*4 == 8 total cores) I have here, every single
core_id is 0.  So this is useless.

I'm just going to count threads.

> I'm not sure if there are many systems running OVS with hot-pluggable
> CPUs, but do you think it's worth caching this information?

Hot-plug isn't common at the physical layer, but Linux supports turning
CPUs on and off at runtime.  Sometimes this is even done dynamically for
power conservation.

> > +        ds_put_format(&s, ",%llu,%llu",
> > +                      (unsigned long long) vfs.f_frsize * vfs.f_blocks / 1024,
> > +                      (unsigned long long) vfs.f_frsize * vfs.f_bfree / 1024);
> 
> Isn't this last one the amount of free space?  vswitch.xml indicates
> that this should the amount used.

You're right.  Fixed.

> > +          Key-value pairs that report statistics about a system running an Open
> > +          vSwitch.  These are updated periodically (currently, every 5
> > +          seconds).  Key-value pairs that cannot be determined or that do not
> > +          apply to a platform are omitted.
> > +        </p>
> > +
> >         <dl>
> > +          <dt><code>cpu-cores</code></dt>
> 
> We're a bit inconsistent with underscores versus hyphens.  This seems
> like it could be a bit confusing.  Especially since the other
> statistics--interfaces--use underscores.

OK, I changed these to use underscores.

> > +            <p>
> > +              Open vSwitch userspace processes are not multithreaded.
> 
> Worth mentioning that the kernel datapath is?

Might as well, thanks.

> > +              <li>RAM allocated to the OS that is use.</li>
> 
> "is use" -> "is in use"

Thanks, fixed.

> > +            <p>
> > +              There will be one key-value pair for each file in Open vSwitch's
> > +              ``run directory'' (usually <code>/var/run/openvswitch</code>)
> > +              whose name ends in <code>.pid</code>, whose contents are a
> > +              process ID, and which is locked by a running process.  The
> > +              <var>name</var> is taken from the pidfile's name.
> > +            </p>
> 
> We probably can't support reporting the crash count in "monitored"
> Python scripts in the same manner being used for C, but it may be
> worth thinking if there's an alternative way...

We could use the Python setproctitle module.

> I hope it made the entire process more enjoyable for you, Gentle
> Reader.

I enjoyed the allusion to contemporary popular entertainment when you
referred to "Weeds" earlier.