[ovs-dev] ovs-vsctl for sFlow config

[Ben Pfaff]

On Thu, May 06, 2010 at 04:57:40PM -0700, Neil McKee wrote:
> I followed these steps to compile the openvswitch rpm on xenserver 5.5 update2 DDK,  and run it on xenserver 5.5 update2:
> 
> http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob_plain;f=INSTALL.XenServer;hb=HEAD
> 
> It mostly worked as advertised, except that missing from these
> instructions is a step where you have to install the kernel-src rpm
> from the DDK sources download, copy in the xen .config file that comes
> with it, compile the kernel with "make" and ensure that
> /lib/modules/<kernel>/build points to that source directory.  

That's very odd.  We compile the RPM from the Xen DDK, using the steps
listed there all the time (often several times a day in our internal
autobuilder), and we do not make any changes to the DDK.  We certainly
do not rebuild the kernel-src RPM (I would have to go off searching to
find it).  And we have probably a dozen different variants on the Xen
DDK VM that we use this way.

So I think that something probably went wrong with your Xen DDK VM.  It
shouldn't be that hard.

> There was one glitch: when I set the "agent" to "eth0" instead of
> "xenbr0" the log file showed (correctly) that it could not get an IP
> address because there was no IP address associated with interface
> eth0.  However, when I set it back to "xenbr0" again, the sFlow agent
> was created but somehow no samplers or pollers were added to it.
> Looking at ofproto-sflow.c the relevant code is:
> 
>     /* Add samplers and pollers for the currently known ports. */
>     PORT_ARRAY_FOR_EACH (osp, &os->ports, odp_port) {
>         ofproto_sflow_add_poller(os, osp, odp_port);
>         ofproto_sflow_add_sampler(os, osp);
>     }
> 
> Trouble is, the prior configuration error resulted in call to
> ofproto_sflow_clear(), so list of "currently known" ports here is now
> empty.  [...]

Ouch.  Thank you for the bug report and for offering to write up a
patch.  But I think that I have a fix for it.  Would you mind testing
the following?  Thank you very much.

--8<--------------------------cut here-------------------------->8--

>From 5ddded1758d206c9e393344ebb92a7c3d7315ef1 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Fri, 7 May 2010 09:29:02 -0700
Subject: [PATCH] ofproto-sflow: Maintain table of ports even when clearing configuration.

When ofproto_sflow_set_options() fails, it calls ofproto_sflow_clear() to
deconfigure the ofproto_sflow object.  But ofproto_sflow_clear() deletes
all of the object's record of datapath ports.  That means that the next
call to ofproto_sflow_set_options(), if it succeeds, will believe that the
datapath has no ports.

This commit fixes the problem by only clearing ofproto_sflow's record of
datapath ports when it is destroyed, not just when a configuration error
occurs.

Reported-by: Neil McKee <neil.mckee at inmon.com>
---
 ofproto/ofproto-sflow.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto-sflow.c b/ofproto/ofproto-sflow.c
index 22d99eb..60baf0e 100644
--- a/ofproto/ofproto-sflow.c
+++ b/ofproto/ofproto-sflow.c
@@ -239,9 +239,6 @@ success:
 void
 ofproto_sflow_clear(struct ofproto_sflow *os)
 {
-    struct ofproto_sflow_port *osp;
-    unsigned int odp_port;
-
     if (os->sflow_agent) {
         sfl_agent_release(os->sflow_agent);
         os->sflow_agent = NULL;
@@ -251,11 +248,6 @@ ofproto_sflow_clear(struct ofproto_sflow *os)
     ofproto_sflow_options_destroy(os->options);
     os->options = NULL;
 
-    PORT_ARRAY_FOR_EACH (osp, &os->ports, odp_port) {
-        ofproto_sflow_del_port(os, odp_port);
-    }
-    port_array_clear(&os->ports);
-
     /* Turn off sampling to save CPU cycles. */
     dpif_set_sflow_probability(os->dpif, 0);
 }
@@ -282,7 +274,13 @@ void
 ofproto_sflow_destroy(struct ofproto_sflow *os)
 {
     if (os) {
+        struct ofproto_sflow_port *osp;
+        unsigned int odp_port;
+
         ofproto_sflow_clear(os);
+        PORT_ARRAY_FOR_EACH (osp, &os->ports, odp_port) {
+            ofproto_sflow_del_port(os, odp_port);
+        }
         port_array_destroy(&os->ports);
         free(os);
     }
-- 
1.6.6.1