I updated the documentation.  Thanks for pointing out that it was
needed.  I remember reviewing that and didn't notice the problem at
the time.

A unit test is a good idea.

Thanks,

Ben.

On Thu, Jan 26, 2012 at 05:21:17PM -0800, Ethan Jackson wrote:
> Thanks for fixing this Ben.
> 
> I think some places in the documentation may mention that the the
> preferred packet_in format applies to snoop as well.
> 
> I broke this, so I'll think about writing a unit test which at least
> does a sanity check on the snoop command, shouldn't be too hard.
> 
> Ethan
> 
> On Thu, Jan 26, 2012 at 17:17, Ben Pfaff <b...@nicira.com> wrote:
> > The vconn that "snoop" opens does not process and reply to requests, so
> > sending a request to set the packet-in format will hang forever, which
> > means that "snoop" never actually prints any of the traffic that it
> > receives.
> >
> > Bug #9346.
> > Reported-by: Alan Shieh <ash...@nicira.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> >  AUTHORS               |    1 +
> >  utilities/ovs-ofctl.c |   38 +++++++++++++++++++-------------------
> >  2 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index 4479aeb..a24f970 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -62,6 +62,7 @@ provided helpful bug reports or suggestions.
> >  Aaron M. Ucko           u...@debian.org
> >  Aaron Rosen             aro...@clemson.edu
> >  Ahmed Bilal             numan...@gmail.com
> > +Alan Shieh              ash...@nicira.com
> >  Alban Browaeys          pra...@yahoo.com
> >  Alex Yip                a...@nicira.com
> >  Alexey I. Froloff       ra...@altlinux.org
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index eb6d5a8..3d3a7b7 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -830,24 +830,6 @@ monitor_vconn(struct vconn *vconn)
> >     bool exiting = false;
> >     int error, fd;
> >
> > -    if (preferred_packet_in_format >= 0) {
> > -        set_packet_in_format(vconn, preferred_packet_in_format);
> > -    } else {
> > -        struct ofpbuf *spif, *reply;
> > -
> > -        spif = ofputil_make_set_packet_in_format(NXPIF_NXM);
> > -        run(vconn_transact_noreply(vconn, spif, &reply),
> > -            "talking to %s", vconn_get_name(vconn));
> > -        if (reply) {
> > -            char *s = ofp_to_string(reply->data, reply->size, 2);
> > -            VLOG_DBG("%s: failed to set packet in format to nxm, 
> > controller"
> > -                     " replied: %s. Falling back to the switch default.",
> > -                     vconn_get_name(vconn), s);
> > -            free(s);
> > -            ofpbuf_delete(reply);
> > -        }
> > -    }
> > -
> >     /* Daemonization will close stderr but we really want to keep it, so 
> > make a
> >      * copy. */
> >     fd = dup(STDERR_FILENO);
> > @@ -910,6 +892,24 @@ do_monitor(int argc, char *argv[])
> >             monitor_set_invalid_ttl_to_controller(vconn);
> >         }
> >     }
> > +    if (preferred_packet_in_format >= 0) {
> > +        set_packet_in_format(vconn, preferred_packet_in_format);
> > +    } else {
> > +        struct ofpbuf *spif, *reply;
> > +
> > +        spif = ofputil_make_set_packet_in_format(NXPIF_NXM);
> > +        run(vconn_transact_noreply(vconn, spif, &reply),
> > +            "talking to %s", vconn_get_name(vconn));
> > +        if (reply) {
> > +            char *s = ofp_to_string(reply->data, reply->size, 2);
> > +            VLOG_DBG("%s: failed to set packet in format to nxm, 
> > controller"
> > +                     " replied: %s. Falling back to the switch default.",
> > +                     vconn_get_name(vconn), s);
> > +            free(s);
> > +            ofpbuf_delete(reply);
> > +        }
> > +    }
> > +
> >     monitor_vconn(vconn);
> >  }
> >
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to