[ovs-dev] [PATCH v4 2/2] ovn-trace: New --ovs option to also print OpenFlow flows.
Justin Pettit
jpettit at ovn.org
Wed Dec 28 07:13:01 UTC 2016
> On Dec 21, 2016, at 3:25 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> Sometimes seeing the OpenFlow flows that back a given logical flow can
> provide additional insight. This commit adds a new --ovs option to
> ovn-trace that makes it connect to Open vSwitch over OpenFlow and retrieve
> and print the OpenFlow flows behind each logical flow encountered during
> a trace.
Neat!
> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
> index d2fbd16..acdc79c 100644
> --- a/include/openvswitch/vconn.h
> +++ b/include/openvswitch/vconn.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -18,9 +18,10 @@
> #define OPENVSWITCH_VCONN_H 1
>
> #include <stdbool.h>
> -#include <openvswitch/list.h>
> -#include <openvswitch/types.h>
> -#include <openflow/openflow.h>
> +#include "openvswitch/list.h"
> +#include "openvswitch/types.h"
> +#include "openvswitch/ofp-util.h"
> +#include "openflow/openflow.h"
>
> #ifdef __cplusplus
> extern "C" {
> @@ -31,6 +32,8 @@ struct pvconn;
> struct pvconn_class;
> struct vconn;
> struct vconn_class;
> +struct ofputil_flow_stats;
> +struct ofputil_flow_stats_request;
Aren't these structures defined in "ofp-util.h", which you included above?
> diff --git a/lib/vconn.c b/lib/vconn.c
> index d5a17f6..57cf429 100644
> --- a/lib/vconn.c
> +++ b/lib/vconn.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> + * Copyright (c) 2008-2016 Nicira, Inc.
I thought you had some legal concern about specifying ranges for copyright years. Regardless, it's a nit, but you add 2016 to "vconn.h" in this same commit, but didn't use a range.
> +/* Send 'request' to 'vconn', encoding it with the given 'protocol', and then
> + * waits for, parses, and accumulates all of the replies into '*fsesp' and
> + * '*n_fsesp'. The caller is responsible for freeing all of the flows.
> + * Returns 0 if successful, otherwise a positive errno value. Always frees
> + * 'request'. */
I think "request" should be "fsr" in the comment. It says that it frees "request" (fsr?), but I'm not sure that's true.
> +int
> +vconn_dump_flows(struct vconn *vconn,
> + const struct ofputil_flow_stats_request *fsr,
> + enum ofputil_protocol protocol,
> + struct ofputil_flow_stats **fsesp, size_t *n_fsesp)
> +{
> + struct ofputil_flow_stats *fses = NULL;
> + size_t n_fses = 0;
> + size_t allocated_fses = 0;
> +
> + struct ofpbuf *request = ofputil_encode_flow_stats_request(fsr, protocol);
> + const struct ofp_header *oh = request->data;
> + ovs_be32 send_xid = oh->xid;
> + int error = vconn_send_block(vconn, request);
> + if (error) {
> + goto exit;
> + }
> +
> + struct ofpbuf *reply = NULL;
> + struct ofpbuf ofpacts;
> + ofpbuf_init(&ofpacts, 0);
> + for (;;) {
> + if (n_fses >= allocated_fses) {
> + fses = x2nrealloc(fses, &allocated_fses, sizeof *fses);
> + }
> +
> + struct ofputil_flow_stats *fs = &fses[n_fses];
> + error = recv_flow_stats_reply(vconn, send_xid, &reply, fs, &ofpacts);
> + if (error) {
> + if (error == EOF) {
> + error = 0;
> + }
> + break;
> + }
> + fs->ofpacts = xmemdup(fs->ofpacts, fs->ofpacts_len);
> + n_fses++;
> + }
> + ofpbuf_uninit(&ofpacts);
> + ofpbuf_delete(reply);
> +
> + if (error) {
> + for (size_t i = 0; i < n_fses; i++) {
> + free(CONST_CAST(struct ofpact *, fses[i].ofpacts));
> + }
> + free(fses);
> +
> + fses = NULL;
> + n_fses = 0;
> + }
> +
> +exit:
> + *fsesp = fses;
> + *n_fsesp = n_fses;
> + return error;
> +}
Acked-by: Justin Pettit <jpettit at ovn.org>
--Justin
More information about the dev
mailing list