[ovs-dev] ovs-vsctl for sFlow config

[Ben Pfaff]

Thanks for testing it.  I was just waiting for some feedback before I
put it into "master", so I've pushed it out now.

On Wed, May 12, 2010 at 04:41:05PM -0700, Neil McKee wrote:
> So... the patch you sent works great (see below),  but I see it's not in the trunk yet.   Will it be checked in soon?
> 
> Neil
> 
> 
> On May 7, 2010, at 11:42 AM, Neil McKee wrote:
> >> 
> >> 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.
> >> 
> > 
> > Yup,  that works.  Good.   You should probably also take out the pointer back to ofproto that is the first field in struct ofproto_sflow (in ofproto-sflow.c).  As far as I can tell it is never set or used,  and just encourages heinous acts of encapsulation-breaking by folks like me.
> > 
> > (Oh, and my compiler felt that "ntohs()" fields were really "int" and so "%zu" was inappropriate in 3 places in ofproto.c.  It wouldn't have said anything,  only it knows that you care about such things.)
> > 
> > Finally, just for completeness,  here is my script to remove and destroy all sflow configuration.  There must be a better way to list fields in a config table(?)
> > 
> > #!/bin/bash
> > for SFLOWUUID in `ovs-vsctl list sflow | awk -- '/^_uuid/ { print $3; }'` ; do
> >    for BRIDGE in `ovs-vsctl list br | awk -- '/^name/ { print $3; }'` ; do
> >        BRIDGE=`echo $BRIDGE | tr -d "\""`
> >        ovs-vsctl remove bridge $BRIDGE sflow $SFLOWUUID ;
> >    done
> >    ovs-vsctl destroy sflow $SFLOWUUID;
> > done
> > 
> > 
> > Regards,
> > Neil
> > 
> > 
> > 
> >> --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
> > 
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
>