[prev in list] [next in list] [prev in thread] [next in thread]
List: openvswitch-dev
Subject: [ovs-dev] [PATCH 2/4] ovs-controller: Make --with-flows read the file only once, at startup.
From: jpettit () nicira ! com (Justin Pettit)
Date: 2010-09-29 21:41:17
Message-ID: BB047DA7-4354-4DA8-A211-C8FB5096134D () nicira ! com
[Download RAW message or body]
Looks good.
--Justin
On Sep 23, 2010, at 3:05 PM, Ben Pfaff wrote:
> A couple of people have reported that ovs-controller --with-flows is
> confusing. This seems to be because it doesn't read the file with the
> flows until the first connection from a switch. Then, if the file has a
> syntax error, it exits.
>
> This commit changes the behavior so that it reads the file immediately at
> startup instead.
> ---
> lib/learning-switch.c | 30 +++++++++---------------------
> lib/learning-switch.h | 3 ++-
> lib/queue.h | 5 ++++-
> utilities/ovs-controller.8.in | 2 ++
> utilities/ovs-controller.c | 35 ++++++++++++++++++++++-------------
> 5 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> index b20506b..36594ac 100644
> --- a/lib/learning-switch.c
> +++ b/lib/learning-switch.c
> @@ -63,8 +63,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
>
> static void queue_tx(struct lswitch *, struct rconn *, struct ofpbuf *);
> static void send_features_request(struct lswitch *, struct rconn *);
> -static void send_default_flows(struct lswitch *sw, struct rconn *rconn,
> - FILE *default_flows);
>
> typedef void packet_handler_func(struct lswitch *, struct rconn *, void *);
> static packet_handler_func process_switch_features;
> @@ -80,18 +78,17 @@ static packet_handler_func process_echo_request;
> * after the given number of seconds (or never expire, if 'max_idle' is
> * OFP_FLOW_PERMANENT). Otherwise, the new switch will process every packet.
> *
> - * The caller may provide the file stream 'default_flows' that defines
> - * default flows that should be pushed when a switch connects. Each
> - * line is a flow entry in the format described for "add-flows" command
> - * in the Flow Syntax section of the ovs-ofct(8) man page. The caller
> - * is responsible for closing the stream.
> + * The caller may provide an ofpbuf 'default_flows' that consists of a chain of
> + * one or more OpenFlow messages to send to the switch at time of connection.
> + * Presumably these will be OFPT_FLOW_MOD requests to set up the flow table.
> *
> * 'rconn' is used to send out an OpenFlow features request. */
> struct lswitch *
> lswitch_create(struct rconn *rconn, bool learn_macs,
> bool exact_flows, int max_idle, bool action_normal,
> - FILE *default_flows)
> + const struct ofpbuf *default_flows)
> {
> + const struct ofpbuf *b;
> struct lswitch *sw;
>
> sw = xzalloc(sizeof *sw);
> @@ -113,9 +110,11 @@ lswitch_create(struct rconn *rconn, bool learn_macs,
> sw->queue = UINT32_MAX;
> sw->queued = rconn_packet_counter_create();
> send_features_request(sw, rconn);
> - if (default_flows) {
> - send_default_flows(sw, rconn, default_flows);
> +
> + for (b = default_flows; b; b = b->next) {
> + queue_tx(sw, rconn, ofpbuf_clone(b));
> }
> +
> return sw;
> }
>
> @@ -249,17 +248,6 @@ send_features_request(struct lswitch *sw, struct rconn *rconn)
> }
>
> static void
> -send_default_flows(struct lswitch *sw, struct rconn *rconn,
> - FILE *default_flows)
> -{
> - struct ofpbuf *b;
> -
> - while ((b = parse_ofp_add_flow_file(default_flows)) != NULL) {
> - queue_tx(sw, rconn, b);
> - }
> -}
> -
> -static void
> queue_tx(struct lswitch *sw, struct rconn *rconn, struct ofpbuf *b)
> {
> int retval = rconn_send_with_limit(rconn, b, sw->queued, 10);
> diff --git a/lib/learning-switch.h b/lib/learning-switch.h
> index 96707b8..edb3154 100644
> --- a/lib/learning-switch.h
> +++ b/lib/learning-switch.h
> @@ -26,7 +26,8 @@ struct rconn;
>
> struct lswitch *lswitch_create(struct rconn *, bool learn_macs,
> bool exact_flows, int max_idle,
> - bool action_normal, FILE *default_flows);
> + bool action_normal,
> + const struct ofpbuf *default_flows);
> void lswitch_set_queue(struct lswitch *sw, uint32_t queue);
> void lswitch_run(struct lswitch *);
> void lswitch_wait(struct lswitch *);
> diff --git a/lib/queue.h b/lib/queue.h
> index 879f7a2..e30b84c 100644
> --- a/lib/queue.h
> +++ b/lib/queue.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009 Nicira Networks.
> + * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -18,6 +18,7 @@
> #define QUEUE_H 1
>
> #include <stdbool.h>
> +#include <stddef.h>
>
> /* Packet queue. */
> struct ovs_queue {
> @@ -26,6 +27,8 @@ struct ovs_queue {
> struct ofpbuf *tail; /* Last queued packet, null if n == 0. */
> };
>
> +#define OVS_QUEUE_INITIALIZER { 0, NULL, NULL }
> +
> void queue_init(struct ovs_queue *);
> void queue_destroy(struct ovs_queue *);
> void queue_clear(struct ovs_queue *);
> diff --git a/utilities/ovs-controller.8.in b/utilities/ovs-controller.8.in
> index c5954dd..24f3a5c 100644
> --- a/utilities/ovs-controller.8.in
> +++ b/utilities/ovs-controller.8.in
> @@ -105,6 +105,8 @@ When a switch connects, push the flow entries as described in
> \fIfile\fR. Each line in \fIfile\fR is a flow entry in the format
> described for the \fBadd\-flows\fR command in the \fBFlow Syntax\fR
> section of the \fBovs\-ofctl\fR(8) man page.
> +.IP
> +Use this option more than once to add flows from multiple files.
> .
> .SS "Public Key Infrastructure Options"
> .so lib/ssl.man
> diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
> index 40e2a80..b1b4f0a 100644
> --- a/utilities/ovs-controller.c
> +++ b/utilities/ovs-controller.c
> @@ -28,6 +28,7 @@
> #include "compiler.h"
> #include "daemon.h"
> #include "learning-switch.h"
> +#include "ofp-parse.h"
> #include "ofpbuf.h"
> #include "openflow/openflow.h"
> #include "poll-loop.h"
> @@ -73,7 +74,7 @@ static uint32_t queue_id = UINT32_MAX;
>
> /* --with-flows: File with flows to send to switch, or null to not load
> * any default flows. */
> -static FILE *flow_file = NULL;
> +static struct ovs_queue default_flows = OVS_QUEUE_INITIALIZER;
>
> /* --unixctl: Name of unixctl socket, or null to use the default. */
> static char *unixctl_path = NULL;
> @@ -216,16 +217,9 @@ new_switch(struct switch_ *sw, struct vconn *vconn)
> {
> sw->rconn = rconn_create(60, 0);
> rconn_connect_unreliably(sw->rconn, vconn, NULL);
> -
> - /* If it was set, rewind 'flow_file' to the beginning, since a
> - * previous call to lswitch_create() will leave the stream at the
> - * end. */
> - if (flow_file) {
> - rewind(flow_file);
> - }
> sw->lswitch = lswitch_create(sw->rconn, learn_macs, exact_flows,
> set_up_flows ? max_idle : -1,
> - action_normal, flow_file);
> + action_normal, default_flows.head);
>
> lswitch_set_queue(sw->lswitch, queue_id);
> }
> @@ -253,6 +247,24 @@ do_switching(struct switch_ *sw)
> }
>
> static void
> +read_flow_file(const char *name)
> +{
> + struct ofpbuf *b;
> + FILE *stream;
> +
> + stream = fopen(optarg, "r");
> + if (!stream) {
> + ovs_fatal(errno, "%s: open", name);
> + }
> +
> + while ((b = parse_ofp_add_flow_file(stream)) != NULL) {
> + queue_push_tail(&default_flows, b);
> + }
> +
> + fclose(stream);
> +}
> +
> +static void
> parse_options(int argc, char *argv[])
> {
> enum {
> @@ -332,10 +344,7 @@ parse_options(int argc, char *argv[])
> break;
>
> case OPT_WITH_FLOWS:
> - flow_file = fopen(optarg, "r");
> - if (flow_file == NULL) {
> - ovs_fatal(errno, "%s: open", optarg);
> - }
> + read_flow_file(optarg);
> break;
>
> case OPT_UNIXCTL:
> --
> 1.7.1
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic