[ovs-dev] ovs-vsctl for sFlow config

Neil McKee neil.mckee at inmon.com
Fri May 7 18:42:54 UTC 2010


On May 7, 2010, at 9:30 AM, Ben Pfaff wrote:

> 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.
> 

So in your DDK VM the /lib/modules/`uname -r`/build link is present and points to some kernel sources?  The openvswitch configure script seems to nose around there to check some things.   Or maybe you copy the tarball across *after* you have run the configure script?  I was just taking what I got from "make dist".   We're using xenserver update 2 instead of update 1,  so perhaps there is a difference there too.  The kernel version is 2.6.18-128.1.6.el5.xs5.5.0.505.1024xen.

I did get some warnings during the openvswitch compilation:

  Building modules, stage 2.
  MODPOST
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'rest_init' (at offset 0xc010215b) and 'try_name'
WARNING: vmlinux - Section mismatch: reference to .init.text:__alloc_bootmem from .text between 'cpu_init' (at offset 0xc010f3c8) and 'c_start'
WARNING: vmlinux - Section mismatch: reference to .init.data:force_mwait from .text between 'init_amd' (at offset 0xc010fb55) and 'do_cyrix_devid'
WARNING: vmlinux - Section mismatch: reference to .init.text:MP_processor_info from .text between 'mp_register_lapic' (at offset 0xc0113e28) and 'mp_register_gsi'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b4ea) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b4f5) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b500) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b50b) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b516) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b521) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b52c) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'iret_exc' (at offset 0xc032b537) and '_etext'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'kmem_cache_create' (at offset 0xc0179a81) and 'cache_reap'
WARNING: vmlinux - Section mismatch: reference to .init.text: from .text between 'kmem_cache_create' (at offset 0xc0179aca) and 'cache_reap'
WARNING: vmlinux - Section mismatch: reference to .init.text:__alloc_bootmem_low from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe474) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.data: from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe4f5) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.text:free_bootmem from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe525) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.text:__alloc_bootmem from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe613) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.text:__alloc_bootmem from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe662) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.text:__alloc_bootmem_low from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe678) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.data: from .text between 'swiotlb_init_with_default_size' (at offset 0xc01fe698) and 'swiotlb_init'
WARNING: vmlinux - Section mismatch: reference to .init.text:reserve_ibft_region from __ksymtab between '__ksymtab_reserve_ibft_region' (at offset 0xc03785c8) and '__ksymtab_ibft_addr'



>> 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.
> 

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





More information about the dev mailing list