Improve flow handling in dpctl.
authorJustin Pettit <jpettit@nicira.com>
Tue, 1 Jul 2008 06:05:14 +0000 (23:05 -0700)
committerJustin Pettit <jpettit@nicira.com>
Tue, 1 Jul 2008 06:05:14 +0000 (23:05 -0700)
This makes the handling of defining and printing actions more consistent in
dpctl.  The flow portion of the output of "dump-flows" can now be used as the
input of "add-flows".  Flows can be added on the command line with the new
"add-flow" command.

lib/ofp-print.c
utilities/dpctl.8
utilities/dpctl.c

index 48ea86de67047877e42202619fabca058541da35..d1a8f8d64147670a4d19b70730500930fe44264b 100644 (file)
@@ -218,47 +218,57 @@ ofp_print_action(struct ds *string, const struct ofp_action *a)
 {
     switch (ntohs(a->type)) {
     case OFPAT_OUTPUT:
-        ds_put_cstr(string, "output(");
-        ofp_print_port_name(string, ntohs(a->arg.output.port));
-        if (a->arg.output.port == htons(OFPP_CONTROLLER)) {
-            ds_put_format(string, ", max %"PRIu16" bytes", 
-                    ntohs(a->arg.output.max_len));
+        {
+            uint16_t port = ntohs(a->arg.output.port); 
+            if (port < OFPP_MAX) {
+                ds_put_format(string, "output:%"PRIu16, port);
+            } else {
+                ofp_print_port_name(string, port);
+                if (port == OFPP_CONTROLLER) {
+                    if (a->arg.output.max_len) {
+                        ds_put_format(string, ":%"PRIu16, 
+                                ntohs(a->arg.output.max_len));
+                    } else {
+                        ds_put_cstr(string, ":all");
+                    }
+                }
+            }
         }
-        ds_put_cstr(string, ")");
         break;
 
     case OFPAT_SET_DL_VLAN:
+        ds_put_cstr(string, "mod_vlan:");
         if (ntohs(a->arg.vlan_id) == OFP_VLAN_NONE) {
-            ds_put_cstr(string, "strip vlan");
+            ds_put_cstr(string, "strip");
         } else {
-            ds_put_format(string, "mod vlan(%"PRIu16")", ntohs(a->arg.vlan_id));
+            ds_put_format(string, "%"PRIu16, ntohs(a->arg.vlan_id));
         }
         break;
 
     case OFPAT_SET_DL_SRC:
-        ds_put_format(string, "mod dl src("ETH_ADDR_FMT")"
+        ds_put_format(string, "mod_dl_src:"ETH_ADDR_FMT
                 ETH_ADDR_ARGS(a->arg.dl_addr));
         break;
 
     case OFPAT_SET_DL_DST:
-        ds_put_format(string, "mod dl dst("ETH_ADDR_FMT")"
+        ds_put_format(string, "mod_dl_dst:"ETH_ADDR_FMT
                 ETH_ADDR_ARGS(a->arg.dl_addr));
         break;
 
     case OFPAT_SET_NW_SRC:
-        ds_put_format(string, "mod nw src("IP_FMT")", IP_ARGS(a->arg.nw_addr));
+        ds_put_format(string, "mod_nw_src:"IP_FMT, IP_ARGS(a->arg.nw_addr));
         break;
 
     case OFPAT_SET_NW_DST:
-        ds_put_format(string, "mod nw dst("IP_FMT")", IP_ARGS(a->arg.nw_addr));
+        ds_put_format(string, "mod_nw_dst:"IP_FMT, IP_ARGS(a->arg.nw_addr));
         break;
 
     case OFPAT_SET_TP_SRC:
-        ds_put_format(string, "mod tp src(%d)", ntohs(a->arg.tp));
+        ds_put_format(string, "mod_tp_src:%d", ntohs(a->arg.tp));
         break;
 
     case OFPAT_SET_TP_DST:
-        ds_put_format(string, "mod tp dst(%d)", ntohs(a->arg.tp));
+        ds_put_format(string, "mod_tp_dst:%d", ntohs(a->arg.tp));
         break;
 
     default:
@@ -273,21 +283,21 @@ static void ofp_print_actions(struct ds *string,
                               size_t n_bytes) 
 {
     size_t i;
+    int n_actions = n_bytes / sizeof *actions;
 
-    ds_put_cstr(string, " actions[");
-    for (i = 0; i < n_bytes / sizeof *actions; i++) {
+    ds_put_format(string, "action%s=", n_actions == 1 ? "" : "s");
+    for (i = 0; i < n_actions; i++) {
         if (i) {
-            ds_put_cstr(string, "");
+            ds_put_cstr(string, ",");
         }
         ofp_print_action(string, &actions[i]);
     }
     if (n_bytes % sizeof *actions) {
         if (i) {
-            ds_put_cstr(string, "");
+            ds_put_cstr(string, ",");
         }
-        ds_put_cstr(string, "; ***trailing garbage***");
+        ds_put_cstr(string, ", ***trailing garbage***");
     }
-    ds_put_cstr(string, "]");
 }
 
 /* Pretty-print the OFPT_PACKET_OUT packet of 'len' bytes at 'oh' to 'string'
@@ -405,11 +415,15 @@ ofp_print_switch_config(struct ds *string, const void *oh, size_t len,
 }
 
 static void print_wild(struct ds *string, const char *leader, int is_wild,
-            const char *format, ...) __attribute__((format(printf, 4, 5)));
+            int verbosity, const char *format, ...) 
+            __attribute__((format(printf, 5, 6)));
 
 static void print_wild(struct ds *string, const char *leader, int is_wild,
-                       const char *format, ...) 
+                       int verbosity, const char *format, ...) 
 {
+    if (is_wild && verbosity < 2) {
+        return;
+    }
     ds_put_cstr(string, leader);
     if (!is_wild) {
         va_list args;
@@ -418,28 +432,36 @@ static void print_wild(struct ds *string, const char *leader, int is_wild,
         ds_put_format_valist(string, format, args);
         va_end(args);
     } else {
-        ds_put_char(string, '?');
+        ds_put_char(string, '*');
     }
 }
 
 /* Pretty-print the ofp_match structure */
-static void ofp_print_match(struct ds *f, const struct ofp_match *om)
+static void ofp_print_match(struct ds *f, const struct ofp_match *om, 
+        int verbosity)
 {
     uint16_t w = ntohs(om->wildcards);
 
-    print_wild(f, " inport", w & OFPFW_IN_PORT, "%d", ntohs(om->in_port));
-    print_wild(f, ":vlan", w & OFPFW_DL_VLAN, "%04x", ntohs(om->dl_vlan));
-    print_wild(f, " mac[", w & OFPFW_DL_SRC,
-               ETH_ADDR_FMT, ETH_ADDR_ARGS(om->dl_src));
-    print_wild(f, "->", w & OFPFW_DL_DST,
-               ETH_ADDR_FMT, ETH_ADDR_ARGS(om->dl_dst));
-    print_wild(f, "] type", w & OFPFW_DL_TYPE, "%04x", ntohs(om->dl_type));
-    print_wild(f, " ip[", w & OFPFW_NW_SRC, IP_FMT, IP_ARGS(&om->nw_src));
-    print_wild(f, "->", w & OFPFW_NW_DST, IP_FMT, IP_ARGS(&om->nw_dst));
-    print_wild(f, "] proto", w & OFPFW_NW_PROTO, "%u", om->nw_proto);
-    print_wild(f, " tport[", w & OFPFW_TP_SRC, "%d", ntohs(om->tp_src));
-    print_wild(f, "->", w & OFPFW_TP_DST, "%d", ntohs(om->tp_dst));
-    ds_put_cstr(f, "]");
+    print_wild(f, "in_port=", w & OFPFW_IN_PORT, verbosity,
+               "%d,", ntohs(om->in_port));
+    print_wild(f, "dl_vlan=", w & OFPFW_DL_VLAN, verbosity,
+               "%04x,", ntohs(om->dl_vlan));
+    print_wild(f, "dl_src=", w & OFPFW_DL_SRC, verbosity,
+               ETH_ADDR_FMT",", ETH_ADDR_ARGS(om->dl_src));
+    print_wild(f, "dl_dst=", w & OFPFW_DL_DST, verbosity,
+               ETH_ADDR_FMT",", ETH_ADDR_ARGS(om->dl_dst));
+    print_wild(f, "dl_type=", w & OFPFW_DL_TYPE, verbosity,
+               "%04x,", ntohs(om->dl_type));
+    print_wild(f, "nw_src=", w & OFPFW_NW_SRC, verbosity,
+               IP_FMT",", IP_ARGS(&om->nw_src));
+    print_wild(f, "nw_dst=", w & OFPFW_NW_DST, verbosity,
+               IP_FMT",", IP_ARGS(&om->nw_dst));
+    print_wild(f, "nw_proto=", w & OFPFW_NW_PROTO, verbosity,
+               "%u,", om->nw_proto);
+    print_wild(f, "tp_src=", w & OFPFW_TP_SRC, verbosity,
+               "%d,", ntohs(om->tp_src));
+    print_wild(f, "tp_dst=", w & OFPFW_TP_DST, verbosity,
+               "%d,", ntohs(om->tp_dst));
 }
 
 /* Pretty-print the OFPT_FLOW_MOD packet of 'len' bytes at 'oh' to 'string'
@@ -450,7 +472,7 @@ ofp_print_flow_mod(struct ds *string, const void *oh, size_t len,
 {
     const struct ofp_flow_mod *ofm = oh;
 
-    ofp_print_match(string, &ofm->match);
+    ofp_print_match(string, &ofm->match, verbosity);
     ds_put_format(string, " cmd:%d idle:%d pri:%d buf:%#x", 
             ntohs(ofm->command), ntohs(ofm->max_idle), 
             ofm->match.wildcards ? ntohs(ofm->priority) : (uint16_t)-1,
@@ -468,7 +490,7 @@ ofp_print_flow_expired(struct ds *string, const void *oh, size_t len,
 {
     const struct ofp_flow_expired *ofe = oh;
 
-    ofp_print_match(string, &ofe->match);
+    ofp_print_match(string, &ofe->match, verbosity);
     ds_put_format(string, 
          " pri%"PRIu16" secs%"PRIu32" pkts%"PRIu64" bytes%"PRIu64"\n", 
          ofe->match.wildcards ? ntohs(ofe->priority) : (uint16_t)-1,
@@ -521,7 +543,7 @@ ofp_flow_stats_request(struct ds *string, const void *oh, size_t len,
         ds_put_format(string, " table_id=%"PRIu8", ", fsr->table_id);
     }
 
-    ofp_print_match(string, &fsr->match);
+    ofp_print_match(string, &fsr->match, verbosity);
 }
 
 static void
@@ -571,7 +593,7 @@ ofp_flow_stats_reply(struct ds *string, const void *body_, size_t len,
                     ntohll(fs->packet_count));
         ds_put_format(string, "n_bytes=%"PRIu64", ", ntohll(fs->byte_count));
         ds_put_format(string, "max_idle=%"PRIu16",", ntohs(fs->max_idle));
-        ofp_print_match(string, &fs->match);
+        ofp_print_match(string, &fs->match, verbosity);
         ofp_print_actions(string, fs->actions, length - sizeof *fs);
         ds_put_char(string, '\n');
 
@@ -591,7 +613,7 @@ ofp_aggregate_stats_request(struct ds *string, const void *oh, size_t len,
         ds_put_format(string, " table_id=%"PRIu8", ", asr->table_id);
     }
 
-    ofp_print_match(string, &asr->match);
+    ofp_print_match(string, &asr->match, verbosity);
 }
 
 static void
index dd5f7755ea1667452614ce5bcaa594953c25bae6..3bce93b42db0d98d37eacf8fb534cf7dad4367f5 100644 (file)
@@ -123,11 +123,17 @@ Prints to the console aggregate statistics for flows in datapath
 the statistics are aggregated across all flows in the datapath's flow
 tables.  See \fBFLOW SYNTAX\fR, below, for the syntax of \fIflows\fR.
 
+.TP
+\fBadd-flow \fIswitch flow\fR
+Add the flow entry as described by \fIflow\fR to the datapath \fIswitch\fR's 
+tables.  The flow entry is in the format described in \fBFLOW SYNTAX\fR, 
+below.
+
 .TP
 \fBadd-flows \fIswitch file\fR
 Add flow entries as described in \fIfile\fR to the datapath \fIswitch\fR's 
 tables.  Each line in \fIfile\fR is a flow entry in the format
-described in \fBFLOW SYNTAX\fB, below.
+described in \fBFLOW SYNTAX\fR, below.
 
 .TP
 \fBdel-flows \fIswitch \fR[\fIflow\fR]
@@ -196,17 +202,21 @@ packets originating from a HTTP server.
 Matches UDP or TCP destination port \fIport\fR.
 
 .PP
-The \fBadd-flow\fR command requires an additional field:
+The \fBadd-flow\fR and \fBadd-flows\fR commands require an additional field:
 
-.IP \fBaction=\fItarget\fR
-Specifies the action to take on a packet when the flow entry matches.
-The \fItarget\fR may be a decimal port number designating the physical
-port on which to output the packet, or one of the following keywords:
+.IP \fIactions\fB=\fItarget\fR[\fB,\fItarget\fR...]\fR
+Specifies a comma-separated list of actions to take on a packet when the 
+flow entry matches.  The \fItarget\fR may be a decimal port number 
+designating the physical port on which to output the packet, or one of 
+the following keywords:
 
 .RS
+.IP \fBoutput\fR:\fIport\fR
+Outputs the packet on the port specified by \fIport\fR.
+
 .IP \fBnormal\fR
-Subjects the packet to the device's normal L2/L3 processing.  This
-action is not implemented by all OpenFlow switches.
+Subjects the packet to the device's normal L2/L3 processing.  (This
+action is not implemented by all OpenFlow switches.)
 
 .IP \fBflood\fR
 Outputs the packet on all switch physical ports other than the port on
@@ -218,14 +228,23 @@ tree protocol).
 Outputs the packet on all switch physical ports other than the port on
 which it was received.
 
-.IP \fBcontroller\fR
+.IP \fBcontroller\fR:\fImax_len\fR
 Sends the packet to the OpenFlow controller as a ``packet in''
-message.
+message.  If \fImax_len\fR is a number, then it specifies the maximum
+number of bytes that should be sent.  If \fImax_len\fR is \fBALL\fR or
+omitted, then the entire packet is sent.
 
 .IP \fBlocal\fR
 Outputs the packet on the ``local port,'' which corresponds to the
 \fBof\fIn\fR network device (see \fBCONTACTING THE CONTROLLER\fR in
 \fBsecchan\fR(8) for information on the \fBof\fIn\fR network device).
+
+.IP \fBmod_vlan\fR:\fIvlan_id\fR
+Modifies the VLAN tag on a packet.  If \fIvlan_id\fR is a number, then 
+the VLAN tag is added or modified as necessary to match the value 
+specified.  If \fIvlan_id\fR is \fBSTRIP\fR, then the VLAN tag is 
+stripped from the packet if one is present.  (This action is not 
+implemented by all OpenFlow switches.)
 .RE
 
 .IP
index 2d09e98e0aa1d04c702b03fac47a621cde84f0e9..334fb81687fc3aaab78a8419726c04c2941e8a47 100644 (file)
@@ -59,6 +59,9 @@
 #include "vlog.h"
 #define THIS_MODULE VLM_dpctl
 
+#define DEFAULT_MAX_IDLE 60
+#define MAX_ADD_ACTS 5
+
 static const char* ifconfigbin = "/sbin/ifconfig";
 
 struct command {
@@ -173,6 +176,7 @@ usage(void)
            "  dump-flows SWITCH FLOW      print matching FLOWs\n"
            "  dump-aggregate SWITCH       print aggregate flow statistics\n"
            "  dump-aggregate SWITCH FLOW  print aggregate stats for FLOWs\n"
+           "  add-flow SWITCH FLOW        add flow described by FLOW\n"
            "  add-flows SWITCH FILE       add flows from FILE\n"
            "  del-flows SWITCH FLOW       delete matching FLOWs\n"
            "where each SWITCH is an active OpenFlow connection method.\n",
@@ -498,32 +502,76 @@ str_to_ip(const char *str, uint32_t *ip)
 }
 
 static void
-str_to_action(const char *str, struct ofp_action *action
+str_to_action(char *str, struct ofp_action *action, int *n_actions
 {
     uint16_t port;
+    int i;
+    int max_actions = *n_actions;
+    char *act, *arg;
+    char *saveptr = NULL;
+    
+    memset(action, 0, sizeof(*action) * max_actions);
+    for (i=0, act = strtok_r(str, ", \t\r\n", &saveptr); 
+         i<max_actions && act;
+         i++, act = strtok_r(NULL, ", \t\r\n", &saveptr)) 
+    {
+        port = OFPP_MAX;
+
+        /* Arguments are separated by colons */
+        arg = strchr(act, ':');
+        if (arg) {
+            *arg = '\0';
+            arg++;
+        } 
+
+        if (!strcasecmp(act, "mod_vlan")) {
+            action[i].type = htons(OFPAT_SET_DL_VLAN);
+
+            if (!strcasecmp(arg, "strip")) {
+                action[i].arg.vlan_id = htons(OFP_VLAN_NONE);
+            } else {
+                action[i].arg.vlan_id = htons(str_to_int(arg));
+            }
+        } else if (!strcasecmp(act, "output")) {
+            port = str_to_int(arg);
+        } else if (!strcasecmp(act, "TABLE")) {
+            port = OFPP_TABLE;
+        } else if (!strcasecmp(act, "NORMAL")) {
+            port = OFPP_NORMAL;
+        } else if (!strcasecmp(act, "FLOOD")) {
+            port = OFPP_FLOOD;
+        } else if (!strcasecmp(act, "ALL")) {
+            port = OFPP_ALL;
+        } else if (!strcasecmp(act, "CONTROLLER")) {
+            port = OFPP_CONTROLLER;
+            if (arg) {
+                if (!strcasecmp(arg, "all")) {
+                    action[i].arg.output.max_len= htons(0);
+                } else {
+                    action[i].arg.output.max_len= htons(str_to_int(arg));
+                }
+            }
+        } else if (!strcasecmp(act, "LOCAL")) {
+            port = OFPP_LOCAL;
+        } else if (strspn(act, "0123456789") == strlen(act)) {
+            port = str_to_int(act);
+        } else {
+            fatal(0, "Unknown action: %s", act);
+        }
 
-    if (!strcasecmp(str, "normal")) {
-        port = OFPP_NORMAL;
-    } else if (!strcasecmp(str, "flood")) {
-        port = OFPP_FLOOD;
-    } else if (!strcasecmp(str, "all")) {
-        port = OFPP_ALL;
-    } else if (!strcasecmp(str, "controller")) {
-        port = OFPP_CONTROLLER;
-    } else if (!strcasecmp(str, "local")) {
-        port = OFPP_LOCAL;
-    } else {
-        port = str_to_int(str);
+        if (port != OFPP_MAX) {
+            action[i].type = htons(OFPAT_OUTPUT);
+            action[i].arg.output.port = htons(port);
+        }
     }
 
-    memset(action, 0, sizeof *action);
-    action->type = OFPAT_OUTPUT;
-    action->arg.output.port = htons(port);
+    *n_actions = i;
 }
 
 static void
-str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
-            uint8_t *table_idx, uint16_t *priority)
+str_to_flow(char *string, struct ofp_match *match, 
+        struct ofp_action *action, int *n_actions, uint8_t *table_idx, 
+        uint16_t *priority, uint16_t *max_idle)
 {
     struct field {
         const char *name;
@@ -548,7 +596,7 @@ str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
 
     char *name, *value;
     uint32_t wildcards;
-    bool got_action = false;
+    char *act_str;
 
     if (table_idx) {
         *table_idx = 0xff;
@@ -556,6 +604,25 @@ str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
     if (priority) {
         *priority = OFP_DEFAULT_PRIORITY;
     }
+    if (max_idle) {
+        *max_idle = DEFAULT_MAX_IDLE;
+    }
+    if (action) {
+        act_str = strstr(string, "action");
+        if (!act_str) {
+            fatal(0, "must specify an action");
+        }
+        *(act_str-1) = '\0';
+
+        act_str = strchr(act_str, '=');
+        if (!act_str) {
+            fatal(0, "must specify an action");
+        }
+
+        act_str++;
+
+        str_to_action(act_str, action, n_actions);
+    }
     memset(match, 0, sizeof *match);
     wildcards = OFPFW_ALL;
     for (name = strtok(string, "="), value = strtok(NULL, ", \t\r\n");
@@ -565,12 +632,6 @@ str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
         const struct field *f;
         void *data;
 
-        if (action && !strcmp(name, "action")) {
-            got_action = true;
-            str_to_action(value, action);
-            continue;
-        }
-
         if (table_idx && !strcmp(name, "table")) {
             *table_idx = atoi(value);
             continue;
@@ -581,6 +642,11 @@ str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
             continue;
         }
 
+        if (max_idle && !strcmp(name, "max_idle")) {
+            *max_idle = atoi(value);
+            continue;
+        }
+
         for (f = fields; f < &fields[ARRAY_SIZE(fields)]; f++) {
             if (!strcmp(f->name, name)) {
                 goto found;
@@ -619,9 +685,6 @@ str_to_flow(char *string, struct ofp_match *match, struct ofp_action *action,
     if (name && !value) {
         fatal(0, "field %s missing value", name);
     }
-    if (action && !got_action) {
-        fatal(0, "must specify an action");
-    }
     match->wildcards = htons(wildcards);
 }
 
@@ -631,8 +694,8 @@ static void do_dump_flows(int argc, char *argv[])
     struct buffer *request;
 
     req = alloc_stats_request(sizeof *req, OFPST_FLOW, &request);
-    str_to_flow(argc > 2 ? argv[2] : "", &req->match, NULL, &req->table_id
-            NULL);
+    str_to_flow(argc > 2 ? argv[2] : "", &req->match, NULL, 0
+            &req->table_id, NULL, NULL);
     memset(req->pad, 0, sizeof req->pad);
 
     dump_stats_transaction(argv[1], request);
@@ -644,13 +707,42 @@ static void do_dump_aggregate(int argc, char *argv[])
     struct buffer *request;
 
     req = alloc_stats_request(sizeof *req, OFPST_AGGREGATE, &request);
-    str_to_flow(argc > 2 ? argv[2] : "", &req->match, NULL, &req->table_id,
-                NULL);
+    str_to_flow(argc > 2 ? argv[2] : "", &req->match, NULL, 0,
+            &req->table_id, NULL, NULL);
     memset(req->pad, 0, sizeof req->pad);
 
     dump_stats_transaction(argv[1], request);
 }
 
+static void do_add_flow(int argc, char *argv[])
+{
+    struct vconn *vconn;
+    struct buffer *buffer;
+    struct ofp_flow_mod *ofm;
+    uint16_t priority, max_idle;
+    size_t size;
+    int n_actions = MAX_ADD_ACTS;
+
+    run(vconn_open_block(argv[1], &vconn), "connecting to %s", argv[1]);
+
+    /* Parse and send. */
+    size = sizeof *ofm + (sizeof ofm->actions[0] * MAX_ADD_ACTS);
+    ofm = alloc_openflow_buffer(size, OFPT_FLOW_MOD, &buffer);
+    str_to_flow(argv[2], &ofm->match, &ofm->actions[0], &n_actions, 
+            NULL, &priority, &max_idle);
+    ofm->command = htons(OFPFC_ADD);
+    ofm->max_idle = htons(max_idle);
+    ofm->buffer_id = htonl(UINT32_MAX);
+    ofm->priority = htons(priority);
+    ofm->reserved = htonl(0);
+
+    /* xxx Should we use the buffer library? */
+    buffer->size -= (MAX_ADD_ACTS - n_actions) * sizeof ofm->actions[0];
+
+    send_openflow_buffer(vconn, buffer);
+    vconn_close(vconn);
+}
+
 static void do_add_flows(int argc, char *argv[])
 {
     struct vconn *vconn;
@@ -667,8 +759,9 @@ static void do_add_flows(int argc, char *argv[])
     while (fgets(line, sizeof line, file)) {
         struct buffer *buffer;
         struct ofp_flow_mod *ofm;
-        uint16_t priority;
+        uint16_t priority, max_idle;
         size_t size;
+        int n_actions = MAX_ADD_ACTS;
 
         char *comment;
 
@@ -684,15 +777,19 @@ static void do_add_flows(int argc, char *argv[])
         }
 
         /* Parse and send. */
-        size = sizeof *ofm + sizeof ofm->actions[0];
+        size = sizeof *ofm + (sizeof ofm->actions[0] * MAX_ADD_ACTS);
         ofm = alloc_openflow_buffer(size, OFPT_FLOW_MOD, &buffer);
-        str_to_flow(line, &ofm->match, &ofm->actions[0], NULL, &priority);
+        str_to_flow(line, &ofm->match, &ofm->actions[0], &n_actions, 
+                NULL, &priority, &max_idle);
         ofm->command = htons(OFPFC_ADD);
-        ofm->max_idle = htons(50);
+        ofm->max_idle = htons(max_idle);
         ofm->buffer_id = htonl(UINT32_MAX);
         ofm->priority = htons(priority);
         ofm->reserved = htonl(0);
 
+        /* xxx Should we use the buffer library? */
+        buffer->size -= (MAX_ADD_ACTS - n_actions) * sizeof ofm->actions[0];
+
         send_openflow_buffer(vconn, buffer);
     }
     vconn_close(vconn);
@@ -713,7 +810,8 @@ static void do_del_flows(int argc, char *argv[])
     /* Parse and send. */
     size = sizeof *ofm;
     ofm = alloc_openflow_buffer(size, OFPT_FLOW_MOD, &buffer);
-    str_to_flow(argc > 2 ? argv[2] : "", &ofm->match, NULL, NULL, &priority);
+    str_to_flow(argc > 2 ? argv[2] : "", &ofm->match, NULL, 0, NULL, 
+            &priority, NULL);
     ofm->command = htons(OFPFC_DELETE);
     ofm->max_idle = htons(0);
     ofm->buffer_id = htonl(UINT32_MAX);
@@ -752,6 +850,7 @@ static struct command all_commands[] = {
     { "dump-tables", 1, 1, do_dump_tables },
     { "dump-flows", 1, 2, do_dump_flows },
     { "dump-aggregate", 1, 2, do_dump_aggregate },
+    { "add-flow", 2, 2, do_add_flow },
     { "add-flows", 2, 2, do_add_flows },
     { "del-flows", 1, 2, do_del_flows },
     { "dump-ports", 1, 1, do_dump_ports },