ofproto: Support packet_outs in bundles.

Add support for OFPT_PACKET_OUT messages in bundles. While ovs-ofctl already has a packet-out command, we did not have a string parser for it, as the parsing was done directly from command line arguments. This patch adds the string parser for packet-out messages, adds support for it into the 'ovs-ofctl packet-out' command, and adds a new ofctl/packet-out ovs-appctl command that can be used when ovs-ofctl is used as a flow monitor. The old 'ovs-ofctl packet-out syntax is deprecated' and will be removed in a later OVS release. The new packet-out parser is further supported with the ovs-ofctl bundle command, which allows bundles to mix flow mods, group mods and packet-out messages. Also the packet-outs in bundles are only executed if the whole bundle is successful. A failing packet-out translation may also make the whole bundle to fail. Signed-off-by: Jarno Rajahalme <[email protected]> Acked-by: Ben Pfaff <[email protected]>
grizax · Sep 14, 2016 · 6dd3c78 · 6dd3c78
1 parent 1f4a893
commit 6dd3c78
Show file tree

Hide file tree

Showing 15 changed files with 573 additions and 119 deletions.
diff --git a/NEWS b/NEWS
@@ -6,6 +6,15 @@ Post-v2.6.0
    - OpenFlow:
      * Bundles now expire after 10 seconds since the last time the
        bundle was either opened, modified, or closed.
+     * OFPT_PACKET_OUT messages are now supported in bundles.
+   - ovs-ofctl:
+     * 'bundle' command now supports packet-out messages.
+     * New syntax for 'ovs-ofctl packet-out' command, which uses the
+       same string parser as the 'bundle' command.  The old 'packet-out'
+       syntax is deprecated and will be removed in a later OVS
+       release.
+     * New unixctl "ofctl/packet-out" command, which can be used to
+       instruct a flow monitor to issue OpenFlow packet-out messages.
 
 v2.6.0 - xx xxx xxxx
 ---------------------

diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
@@ -28,6 +28,7 @@
 struct flow;
 struct ofpbuf;
 struct ofputil_flow_mod;
+struct ofputil_packet_out;
 struct ofputil_flow_monitor_request;
 struct ofputil_flow_stats_request;
 struct ofputil_group_mod;
@@ -47,6 +48,10 @@ char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
                              enum ofputil_protocol *usable_protocols)
     OVS_WARN_UNUSED_RESULT;
 
+char *parse_ofp_packet_out_str(struct ofputil_packet_out *po, const char *str_,
+                               enum ofputil_protocol *usable_protocols)
+    OVS_WARN_UNUSED_RESULT;
+
 char *parse_ofp_table_mod(struct ofputil_table_mod *,
                           const char *table_id, const char *flow_miss_handling,
                           uint32_t *usable_versions)

diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
@@ -1338,18 +1338,20 @@ enum ofperr ofputil_decode_bundle_add(const struct ofp_header *,
                                       struct ofputil_bundle_add_msg *,
                                       enum ofptype *type);
 
+/* Bundle message as produced by ofp-parse. */
 struct ofputil_bundle_msg {
     enum ofptype type;
     union {
         struct ofputil_flow_mod fm;
         struct ofputil_group_mod gm;
+        struct ofputil_packet_out po;
     };
 };
 
-/* Destroys 'bms'. */
-void ofputil_encode_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms,
-                                struct ovs_list *requests,
+void ofputil_encode_bundle_msgs(const struct ofputil_bundle_msg *bms,
+                                size_t n_bms, struct ovs_list *requests,
                                 enum ofputil_protocol);
+void ofputil_free_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms);
 
 struct ofputil_tlv_map {
     struct ovs_list list_node;

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
@@ -22,6 +22,7 @@
 #include <netinet/in.h>
 
 #include "byte-order.h"
+#include "dp-packet.h"
 #include "learn.h"
 #include "multipath.h"
 #include "netdev.h"
@@ -550,6 +551,107 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     return error;
 }
 
+/* Parse a string representation of a OFPT_PACKET_OUT to '*po'.  If successful,
+ * both 'po->ofpacts' and 'po->packet' must be free()d by the caller. */
+static char * OVS_WARN_UNUSED_RESULT
+parse_ofp_packet_out_str__(struct ofputil_packet_out *po, char *string,
+                           enum ofputil_protocol *usable_protocols)
+{
+    enum ofputil_protocol action_usable_protocols;
+    uint64_t stub[256 / 8];
+    struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
+    struct dp_packet *packet = NULL;
+    char *act_str = NULL;
+    char *name, *value;
+    char *error = NULL;
+
+    *usable_protocols = OFPUTIL_P_ANY;
+
+    *po = (struct ofputil_packet_out) {
+        .buffer_id = UINT32_MAX,
+        .in_port = OFPP_CONTROLLER,
+    };
+
+    act_str = extract_actions(string);
+
+    while (ofputil_parse_key_value(&string, &name, &value)) {
+        if (!*value) {
+            error = xasprintf("field %s missing value", name);
+            goto out;
+        }
+
+        if (!strcmp(name, "in_port")) {
+            if (!ofputil_port_from_string(value, &po->in_port)) {
+                error = xasprintf("%s is not a valid OpenFlow port", value);
+                goto out;
+            }
+            if (po->in_port > OFPP_MAX && po->in_port != OFPP_LOCAL
+                && po->in_port != OFPP_NONE
+                && po->in_port != OFPP_CONTROLLER) {
+                error = xasprintf(
+                              "%s is not a valid OpenFlow port for PACKET_OUT",
+                              value);
+                goto out;
+            }
+        } else if (!strcmp(name, "packet")) {
+            const char *error_msg = eth_from_hex(value, &packet);
+            if (error_msg) {
+                error = xasprintf("%s: %s", name, error_msg);
+                goto out;
+            }
+        } else {
+            error = xasprintf("unknown keyword %s", name);
+            goto out;
+        }
+    }
+
+    if (!packet || !dp_packet_size(packet)) {
+        error = xstrdup("must specify packet");
+        goto out;
+    }
+
+    if (act_str) {
+        error = ofpacts_parse_actions(act_str, &ofpacts,
+                                      &action_usable_protocols);
+        *usable_protocols &= action_usable_protocols;
+        if (error) {
+            goto out;
+        }
+    }
+    po->ofpacts_len = ofpacts.size;
+    po->ofpacts = ofpbuf_steal_data(&ofpacts);
+
+    po->packet_len = dp_packet_size(packet);
+    po->packet = dp_packet_steal_data(packet);
+out:
+    ofpbuf_uninit(&ofpacts);
+    dp_packet_delete(packet);
+    return error;
+}
+
+/* Convert 'str_' (as described in the Packet-Out Syntax section of the
+ * ovs-ofctl man page) into 'po' for sending a OFPT_PACKET_OUT message to a
+ * switch.  Returns the set of usable protocols in '*usable_protocols'.
+ *
+ * Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string. */
+char * OVS_WARN_UNUSED_RESULT
+parse_ofp_packet_out_str(struct ofputil_packet_out *po, const char *str_,
+                         enum ofputil_protocol *usable_protocols)
+{
+    char *string = xstrdup(str_);
+    char *error;
+
+    error = parse_ofp_packet_out_str__(po, string, usable_protocols);
+    if (error) {
+        po->ofpacts = NULL;
+        po->ofpacts_len = 0;
+    }
+
+    free(string);
+    return error;
+}
+
 static char * OVS_WARN_UNUSED_RESULT
 parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
                           struct ofpbuf *bands, int command,
@@ -1709,26 +1811,6 @@ parse_ofp_group_mod_file(const char *file_name, int command,
     return NULL;
 }
 
-static void
-free_bundle_msgs(struct ofputil_bundle_msg **bms, size_t *n_bms)
-{
-    for (size_t i = 0; i < *n_bms; i++) {
-        switch ((int)(*bms)[i].type) {
-        case OFPTYPE_FLOW_MOD:
-            free(CONST_CAST(struct ofpact *, (*bms)[i].fm.ofpacts));
-            break;
-        case OFPTYPE_GROUP_MOD:
-            ofputil_uninit_group_mod(&(*bms)[i].gm);
-            break;
-        default:
-            break;
-        }
-    }
-    free(*bms);
-    *bms = NULL;
-    *n_bms = 0;
-}
-
 /* Opens file 'file_name' and reads each line as a flow_mod or a group_mod,
  * depending on the first keyword on each line.  Stores each flow and group
  * mods in '*bms', an array allocated on the caller's behalf, and the number of
@@ -1797,6 +1879,13 @@ parse_ofp_bundle_file(const char *file_name,
                 break;
             }
             (*bms)[*n_bms].type = OFPTYPE_GROUP_MOD;
+        } else if (!strncmp(s, "packet-out", len)) {
+            s += len;
+            error = parse_ofp_packet_out_str(&(*bms)[*n_bms].po, s, &usable);
+            if (error) {
+                break;
+            }
+            (*bms)[*n_bms].type = OFPTYPE_PACKET_OUT;
         } else {
             error = xasprintf("Unsupported bundle message type: %.*s",
                               (int)len, s);
@@ -1816,7 +1905,9 @@ parse_ofp_bundle_file(const char *file_name,
         char *err_msg = xasprintf("%s:%d: %s", file_name, line_number, error);
         free(error);
 
-        free_bundle_msgs(bms, n_bms);
+        ofputil_free_bundle_msgs(*bms, *n_bms);
+        *bms = NULL;
+        *n_bms = 0;
         return err_msg;
     }
     return NULL;

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
@@ -4131,6 +4131,9 @@ ofputil_packet_in_private_destroy(struct ofputil_packet_in_private *pin)
  * message's actions.  The caller must initialize 'ofpacts' and retains
  * ownership of it.  'po->ofpacts' will point into the 'ofpacts' buffer.
  *
+ * 'po->packet' refers to the packet data in 'oh', so the buffer containing
+ * 'oh' must not be destroyed while 'po' is being used.
+ *
  * Returns 0 if successful, otherwise an OFPERR_* value. */
 enum ofperr
 ofputil_decode_packet_out(struct ofputil_packet_out *po,
@@ -9462,8 +9465,30 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
 
 /* Destroys 'bms'. */
 void
-ofputil_encode_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms,
-                           struct ovs_list *requests,
+ofputil_free_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms)
+{
+    for (size_t i = 0; i < n_bms; i++) {
+        switch ((int)bms[i].type) {
+        case OFPTYPE_FLOW_MOD:
+            free(CONST_CAST(struct ofpact *, bms[i].fm.ofpacts));
+            break;
+        case OFPTYPE_GROUP_MOD:
+            ofputil_uninit_group_mod(&bms[i].gm);
+            break;
+        case OFPTYPE_PACKET_OUT:
+            free(bms[i].po.ofpacts);
+            free(CONST_CAST(void *, bms[i].po.packet));
+            break;
+        default:
+            break;
+        }
+    }
+    free(bms);
+}
+
+void
+ofputil_encode_bundle_msgs(const struct ofputil_bundle_msg *bms,
+                           size_t n_bms, struct ovs_list *requests,
                            enum ofputil_protocol protocol)
 {
     enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
@@ -9474,11 +9499,12 @@ ofputil_encode_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms,
         switch ((int)bms[i].type) {
         case OFPTYPE_FLOW_MOD:
             request = ofputil_encode_flow_mod(&bms[i].fm, protocol);
-            free(CONST_CAST(struct ofpact *, bms[i].fm.ofpacts));
             break;
         case OFPTYPE_GROUP_MOD:
             request = ofputil_encode_group_mod(version, &bms[i].gm);
-            ofputil_uninit_group_mod(&bms[i].gm);
+            break;
+        case OFPTYPE_PACKET_OUT:
+            request = ofputil_encode_packet_out(&bms[i].po, protocol);
             break;
         default:
             break;
@@ -9487,7 +9513,6 @@ ofputil_encode_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms,
             ovs_list_push_back(requests, &request->list_node);
         }
     }
-    free(bms);
 }
 
 /* Parse a queue status request message into 'oqsr'.
@@ -9875,13 +9900,13 @@ ofputil_is_bundlable(enum ofptype type)
     case OFPTYPE_FLOW_MOD:
         /* Other supported types. */
     case OFPTYPE_GROUP_MOD:
+    case OFPTYPE_PACKET_OUT:
         return true;
 
         /* Nice to have later. */
     case OFPTYPE_FLOW_MOD_TABLE_ID:
     case OFPTYPE_TABLE_MOD:
     case OFPTYPE_METER_MOD:
-    case OFPTYPE_PACKET_OUT:
     case OFPTYPE_NXT_TLV_TABLE_MOD:
 
         /* Not to be bundlable. */

diff --git a/ofproto/bundles.h b/ofproto/bundles.h
@@ -33,12 +33,13 @@ extern "C" {
 
 struct ofp_bundle_entry {
     struct ovs_list   node;
-    enum ofptype      type;  /* OFPTYPE_FLOW_MOD, OFPTYPE_PORT_MOD, or
-                              * OFPTYPE_GROUP_MOD. */
+    enum ofptype      type;  /* OFPTYPE_FLOW_MOD, OFPTYPE_PORT_MOD,
+                              * OFPTYPE_GROUP_MOD, OFPTYPE_PACKET_OUT. */
     union {
         struct ofproto_flow_mod ofm;
         struct ofproto_port_mod opm;
         struct ofproto_group_mod ogm;
+        struct ofproto_packet_out opo;
     };
 
     /* OpenFlow header and some of the message contents for error reporting. */
@@ -106,6 +107,8 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
             ofproto_flow_mod_uninit(&entry->ofm);
         } else if (entry->type == OFPTYPE_GROUP_MOD) {
             ofputil_uninit_group_mod(&entry->ogm.gm);
+        } else if (entry->type == OFPTYPE_PACKET_OUT) {
+            ofproto_packet_out_uninit(&entry->opo);
         }
         free(entry);
     }

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
@@ -4370,6 +4370,28 @@ packet_xlate(struct ofproto *ofproto_, struct ofproto_packet_out *opo)
     return error;
 }
 
+static void
+packet_xlate_revert(struct ofproto *ofproto OVS_UNUSED,
+                    struct ofproto_packet_out *opo)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    struct ofproto_dpif_packet_out *aux = opo->aux;
+    ovs_assert(aux);
+
+    /* Revert the learned flows. */
+    struct xc_entry *entry;
+    struct ofpbuf entries = aux->xcache.entries;
+
+    XC_ENTRY_FOR_EACH (entry, &entries) {
+        if (entry->type == XC_LEARN && entry->learn.ofm->learn_adds_rule) {
+            ofproto_flow_mod_learn_revert(entry->learn.ofm);
+        }
+    }
+
+    ofproto_dpif_packet_out_delete(aux);
+    opo->aux = NULL;
+}
+
 /* Push stats and perform side effects of flow translation. */
 static void
 ofproto_dpif_xcache_execute(struct ofproto_dpif *ofproto,
@@ -5824,6 +5846,7 @@ const struct ofproto_class ofproto_dpif_class = {
     rule_dealloc,
     rule_get_stats,
     packet_xlate,
+    packet_xlate_revert,
     packet_execute,
     set_frag_handling,
     nxt_resume,

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
@@ -1313,6 +1313,11 @@ struct ofproto_class {
     enum ofperr (*packet_xlate)(struct ofproto *,
                                 struct ofproto_packet_out *opo);
 
+    /* Free resources taken by a successful packet_xlate().  If multiple
+     * packet_xlate() calls have been made in sequence, the corresponding
+     * packet_xlate_revert() calls have to be made in reverse order. */
+    void (*packet_xlate_revert)(struct ofproto *, struct ofproto_packet_out *);
+
     /* Executes the datapath actions, translation side-effects, and stats as
      * produced by ->packet_xlate().  The caller retains ownership of 'opo'.
      */
@@ -1907,6 +1912,8 @@ enum ofperr ofproto_flow_mod_learn(struct ofproto_flow_mod *, bool keep_ref)
 enum ofperr ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm);
 enum ofperr ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
     OVS_REQUIRES(ofproto_mutex);
+void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
+    OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
                                           struct ofproto *orig_ofproto)
     OVS_REQUIRES(ofproto_mutex);