[ovs-dev] ovs-vsctl for sFlow config

[Neil McKee]

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