[ovs-dev] [PATCH] ovs-ofctl: Fix "snoop" command.

[Ethan Jackson]

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 <blp at 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 <ashieh at nicira.com>
> Signed-off-by: Ben Pfaff <blp at 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           ucko at debian.org
>  Aaron Rosen             arosen at clemson.edu
>  Ahmed Bilal             numan252 at gmail.com
> +Alan Shieh              ashieh at nicira.com
>  Alban Browaeys          prahal at yahoo.com
>  Alex Yip                alex at nicira.com
>  Alexey I. Froloff       raorn at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev