[ovs-dev] [PATCH v3 13/13] ofproto: Support packet_outs in bundles.

Jarno Rajahalme jarno at ovn.org
Mon Sep 12 20:52:43 UTC 2016


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, and adds a
new ofctl/packet-out ovs-appctl command that can be used when
ovs-ofctl is used as a flow monitor.

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 <jarno at ovn.org>
---
v3: Rebase.

 NEWS                            |   1 +
 include/openvswitch/ofp-parse.h |   5 ++
 include/openvswitch/ofp-util.h  |   1 +
 lib/ofp-parse.c                 | 108 ++++++++++++++++++++++++++++++++
 lib/ofp-util.c                  |   7 ++-
 ofproto/bundles.h               |   7 ++-
 ofproto/ofproto-dpif.c          |  23 +++++++
 ofproto/ofproto-provider.h      |   7 +++
 ofproto/ofproto.c               |  82 +++++++++++++++++-------
 tests/ofp-print.at              |  19 ++++++
 tests/ofproto.at                | 134 ++++++++++++++++++++++++++++++++++++++--
 utilities/ovs-ofctl.8.in        |  46 ++++++++++++--
 utilities/ovs-ofctl.c           |  55 +++++++++++++++++
 13 files changed, 460 insertions(+), 35 deletions(-)

diff --git a/NEWS b/NEWS
index 7f501d6..20fa0b7 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ 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.
 
 v2.6.0 - xx xxx xxxx
 ---------------------
diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
index df60b18..e68e57b 100644
--- 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
index 450e739..9283e13 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -1343,6 +1343,7 @@ struct ofputil_bundle_msg {
     union {
         struct ofputil_flow_mod fm;
         struct ofputil_group_mod gm;
+        struct ofputil_packet_out po;
     };
 };
 
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index d3ef140..7fccb48 100644
--- 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,106 @@ 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);
+        }
+
+        if (!strcmp(name, "in_port")) {
+            if (!ofputil_port_from_string(value, &po->in_port)) {
+                error = xasprintf("%s is not a valid OpenFlow port", value);
+            }
+            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);
+            }
+        } else if (!strcmp(name, "packet")) {
+            const char *error_msg = eth_from_hex(value, &packet);
+            if (error_msg) {
+                error = xasprintf("%s: %s", name, error_msg);
+            }
+        } else {
+            error = xasprintf("unknown keyword %s", name);
+        }
+
+        if (error) {
+            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,
@@ -1797,6 +1898,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);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1fa4998..90a3f5b 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9480,6 +9480,11 @@ ofputil_encode_bundle_msgs(struct ofputil_bundle_msg *bms, size_t n_bms,
             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);
+            free(bms[i].po.ofpacts);
+            free(CONST_CAST(void *, bms[i].po.packet));
+            break;
         default:
             break;
         }
@@ -9875,13 +9880,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
index 3069224..1818b16 100644
--- 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
index 4414f80..7ac88f2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4370,6 +4370,28 @@ error_out:
     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->u.learn.ofm->learn_adds_rule) {
+            ofproto_flow_mod_learn_revert(entry->u.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,
@@ -5825,6 +5847,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
index dd700d2..be5632e 100644
--- 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);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ae07e7e..90b1ffa 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -259,6 +259,9 @@ static enum ofperr ofproto_flow_mod_init(struct ofproto *,
 static enum ofperr ofproto_flow_mod_start(struct ofproto *,
                                           struct ofproto_flow_mod *)
     OVS_REQUIRES(ofproto_mutex);
+static void ofproto_flow_mod_revert(struct ofproto *,
+                                    struct ofproto_flow_mod *)
+    OVS_REQUIRES(ofproto_mutex);
 static void ofproto_flow_mod_finish(struct ofproto *,
                                     struct ofproto_flow_mod *,
                                     const struct openflow_mod_requester *)
@@ -3465,6 +3468,14 @@ ofproto_packet_out_start(struct ofproto *ofproto,
 }
 
 static void
+ofproto_packet_out_revert(struct ofproto *ofproto,
+                          struct ofproto_packet_out *opo)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    ofproto->ofproto_class->packet_xlate_revert(ofproto, opo);
+}
+
+static void
 ofproto_packet_out_finish(struct ofproto *ofproto,
                           struct ofproto_packet_out *opo)
     OVS_REQUIRES(ofproto_mutex)
@@ -4977,6 +4988,14 @@ ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
 }
 
 void
+ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    struct rule *rule = rule_collection_rules(&ofm->new_rules)[0];
+    ofproto_flow_mod_revert(rule->ofproto, ofm);
+}
+
+void
 ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
                               struct ofproto *orig_ofproto)
     OVS_REQUIRES(ofproto_mutex)
@@ -7495,6 +7514,9 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
                      * effect. */
                     be->ogm.version = version;
                     error = ofproto_group_mod_start(ofproto, &be->ogm);
+                } else if (be->type == OFPTYPE_PACKET_OUT) {
+                    be->opo.version = version;
+                    error = ofproto_packet_out_start(ofproto, &be->opo);
                 } else {
                     OVS_NOT_REACHED();
                 }
@@ -7517,8 +7539,9 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
                     ofproto_flow_mod_revert(ofproto, &be->ofm);
                 } else if (be->type == OFPTYPE_GROUP_MOD) {
                     ofproto_group_mod_revert(ofproto, &be->ogm);
+                } else if (be->type == OFPTYPE_PACKET_OUT) {
+                    ofproto_packet_out_revert(ofproto, &be->opo);
                 }
-
                 /* Nothing needs to be reverted for a port mod. */
             }
         } else {
@@ -7533,30 +7556,30 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
                      * processing. */
                     port_mod_finish(ofconn, &be->opm.pm, be->opm.port);
                 } else {
+                    version =
+                        (be->type == OFPTYPE_FLOW_MOD) ? be->ofm.version :
+                        (be->type == OFPTYPE_GROUP_MOD) ? be->ogm.version :
+                        (be->type == OFPTYPE_PACKET_OUT) ? be->opo.version :
+                        version;
+
+                    /* Bump the lookup version to the one of the current
+                     * message.  This makes all the changes in the bundle at
+                     * this version visible to lookups at once. */
+                    if (ofproto->tables_version < version) {
+                        ofproto->tables_version = version;
+                        ofproto->ofproto_class->set_tables_version(
+                            ofproto, ofproto->tables_version);
+                    }
+
                     struct openflow_mod_requester req = { ofconn,
                                                           &be->ofp_msg };
-                    if (be->type == OFPTYPE_FLOW_MOD) {
-                        /* Bump the lookup version to the one of the current
-                         * message.  This makes all the changes in the bundle
-                         * at this version visible to lookups at once. */
-                        if (ofproto->tables_version < be->ofm.version) {
-                            ofproto->tables_version = be->ofm.version;
-                            ofproto->ofproto_class->set_tables_version(
-                                ofproto, ofproto->tables_version);
-                        }
 
+                    if (be->type == OFPTYPE_FLOW_MOD) {
                         ofproto_flow_mod_finish(ofproto, &be->ofm, &req);
                     } else if (be->type == OFPTYPE_GROUP_MOD) {
-                        /* Bump the lookup version to the one of the current
-                         * message.  This makes all the changes in the bundle
-                         * at this version visible to lookups at once. */
-                        if (ofproto->tables_version < be->ogm.version) {
-                            ofproto->tables_version = be->ogm.version;
-                            ofproto->ofproto_class->set_tables_version(
-                                ofproto, ofproto->tables_version);
-                        }
-
                         ofproto_group_mod_finish(ofproto, &be->ogm, &req);
+                    } else if (be->type == OFPTYPE_PACKET_OUT) {
+                        ofproto_packet_out_finish(ofproto, &be->opo);
                     }
                 }
             }
@@ -7646,14 +7669,15 @@ handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
 
     bmsg = ofp_bundle_entry_alloc(type, badd.msg);
 
+    struct ofpbuf ofpacts;
+    uint64_t ofpacts_stub[1024 / 8];
+    ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+
     if (type == OFPTYPE_PORT_MOD) {
         error = ofputil_decode_port_mod(badd.msg, &bmsg->opm.pm, false);
     } else if (type == OFPTYPE_FLOW_MOD) {
         struct ofputil_flow_mod fm;
-        struct ofpbuf ofpacts;
-        uint64_t ofpacts_stub[1024 / 8];
 
-        ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
         error = ofputil_decode_flow_mod(&fm, badd.msg,
                                         ofconn_get_protocol(ofconn),
                                         &ofpacts,
@@ -7662,13 +7686,25 @@ handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
         if (!error) {
             error = ofproto_flow_mod_init(ofproto, &bmsg->ofm, &fm, NULL);
         }
-        ofpbuf_uninit(&ofpacts);
     } else if (type == OFPTYPE_GROUP_MOD) {
         error = ofputil_decode_group_mod(badd.msg, &bmsg->ogm.gm);
+    } else if (type == OFPTYPE_PACKET_OUT) {
+        struct ofputil_packet_out po;
+
+        COVERAGE_INC(ofproto_packet_out);
+
+        /* Decode message. */
+        error = ofputil_decode_packet_out(&po, badd.msg, &ofpacts);
+        if (!error) {
+            po.ofpacts = ofpbuf_steal_data(&ofpacts);   /* Move to heap. */
+            error = ofproto_packet_out_init(ofproto, ofconn, &bmsg->opo, &po);
+        }
     } else {
         OVS_NOT_REACHED();
     }
 
+    ofpbuf_uninit(&ofpacts);
+
     if (!error) {
         error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
                                        bmsg, oh);
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 5d2040b..6c7433d 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -3532,6 +3532,25 @@ OFPT_GROUP_MOD (OF1.5) (xid=0x3):
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - PACKET_OUT])
+AT_KEYWORDS([ofp-print bundle packet-out])
+AT_CHECK([ovs-ofctl ofp-print "\
+05 22 00 74 00 00 00 03 00 00 00 01 00 00 00 01 \
+05 0d 00 64 00 00 00 03 ff ff ff ff ff ff ff fe \
+00 10 00 00 00 00 00 00 00 00 00 10 ff ff ff fb \
+05 dc 00 00 00 00 00 00 50 54 00 00 00 05 50 54 \
+00 00 00 06 08 00 45 00 00 28 00 00 40 00 40 06 \
+b9 7c c0 a8 00 02 c0 a8 00 01 00 00 2b 60 00 00 \
+00 00 6a 4f 2b 58 50 14 00 00 6d 75 00 00 00 00 \
+00 00 00 00 \
+"], [0], [dnl
+OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3):
+ bundle_id=0x1 flags=atomic
+OFPT_PACKET_OUT (OF1.4) (xid=0x3): in_port=LOCAL actions=FLOOD data_len=60
+tcp,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=11104,tcp_flags=rst|ack tcp_csum:6d75
+])
+AT_CLEANUP
+
 AT_SETUP([NXST_IPFIX_BRIDGE - request])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 24867c7..bbd4d38 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1965,6 +1965,131 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto - bundle packet-out (OpenFlow 1.4)])
+OVS_VSWITCHD_START
+
+ovs-ofctl del-flows br0
+ovs-ofctl add-flow br0 priority=0,actions=drop
+
+# Start a monitor listening for packet-ins.
+AT_CHECK([ovs-ofctl -P standard monitor br0 --detach --no-chdir --pidfile])
+ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700000080
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+AT_CAPTURE_FILE([monitor.log])
+
+# This bundle adds a group, a flow using that group and then a
+# packet-out that needs them both.  Finally the bundle deletes all
+# groups, which also deletes the flow, leaving only the drop flow in
+# the table.  If this works properly, the packet-out should get
+# translated and processed before the flow disappears.  Also, if the
+# bundle were to fail, the packet-in should not get executed.
+AT_DATA([bundle.txt], [dnl
+group group_id=1,type=all,bucket=output:controller
+flow in_port=6 actions=group:1
+packet-out in_port=6, packet=0001020304050010203040501234 actions=table
+group delete
+])
+AT_CHECK([ovs-ofctl bundle br0 bundle.txt])
+
+# Verify that only the drop flow remains.
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], [0],
+  [ reset_counts priority=0 actions=drop
+OFPST_FLOW reply (OF1.4):
+])
+
+# Verify that the packet-in was received via controller action.
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
+  [OFPT_PACKET_IN (xid=0x0): total_len=14 in_port=6 (via action) data_len=14 (unbuffered)
+vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - bundle packet-out, failing bundle commit (OpenFlow 1.4)])
+OVS_VSWITCHD_START
+
+ovs-ofctl del-flows br0
+ovs-ofctl add-flow br0 priority=0,actions=drop
+
+# Start a monitor listening for packet-ins.
+AT_CHECK([ovs-ofctl -P standard monitor br0 --detach --no-chdir --pidfile])
+ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700000080
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+AT_CAPTURE_FILE([monitor.log])
+
+# This bundle adds a flow using that group and then a packet-out that
+# needs them both.  Finally the bundle adds another flow that referes
+# to a non-existing group, causing the bundle to fail, and the
+# packet-in should not get executed.
+AT_DATA([bundle.txt], [dnl
+group group_id=1,type=all,bucket=output:controller
+flow in_port=6 actions=group:1
+packet-out in_port=6, packet=0001020304050010203040501234 actions=table
+flow in_port=7 actions=group:2
+])
+AT_CHECK([ovs-ofctl bundle br0 bundle.txt 2>&1 | sed '/talking to/,$d'], [], [dnl
+Error OFPBAC_BAD_OUT_GROUP for: OFPT_FLOW_MOD (OF1.4) (xid=0x5): ADD in_port=7 actions=group:2
+Error OFPBFC_MSG_FAILED for: OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x8):
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
+])
+
+# Verify that only the drop flow remains.
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], [0],
+  [ reset_counts priority=0 actions=drop
+OFPST_FLOW reply (OF1.4):
+])
+
+# Verify that the packet-in was NOT received via controller action.
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0], [])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - bundle packet-out makes bundle commit to fail(OpenFlow 1.4)])
+OVS_VSWITCHD_START
+
+ovs-ofctl del-flows br0
+ovs-ofctl add-flow br0 priority=0,actions=drop
+
+# Start a monitor listening for packet-ins.
+AT_CHECK([ovs-ofctl -P standard monitor br0 --detach --no-chdir --pidfile])
+ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700000080
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+AT_CAPTURE_FILE([monitor.log])
+
+# This bundle adds a flow using that group and then a packet-out that
+# needs them both.  Finally the bundle adds another flow that referes
+# to a non-existing group, causing the bundle to fail, and the
+# packet-in should not get executed.
+AT_DATA([bundle.txt], [dnl
+group group_id=1,type=all,bucket=output:controller
+flow in_port=6 actions=group:1
+packet-out in_port=6, packet=0001020304050010203040501234 actions=table
+packet-out in_port=6, packet=0001020304050010203040501234 actions=group:2
+])
+AT_CHECK([ovs-ofctl bundle br0 bundle.txt 2>&1 | sed '/talking to/,$d'], [], [dnl
+Error OFPBAC_BAD_OUT_GROUP for: OFPT_PACKET_OUT (OF1.4) (xid=0x5): in_port=6 actions=group:2 data_len=14
+vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
+Error OFPBFC_MSG_FAILED for: OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x8):
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
+])
+
+# Verify that only the drop flow remains.
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], [0],
+  [ reset_counts priority=0 actions=drop
+OFPST_FLOW reply (OF1.4):
+])
+
+# Verify that the packet-in was NOT received via controller action.
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0], [])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - flow table configuration (OpenFlow 1.0)])
 OVS_VSWITCHD_START
 # Check the default configuration.
@@ -3844,15 +3969,16 @@ ovs-appctl -t ovs-ofctl ofctl/barrier
 ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
 AT_CAPTURE_FILE([monitor.log])
 
-# Send some packet-outs with OFPP_NONE and OFPP_CONTROLLER (65533) as in_port.
-AT_CHECK([ovs-ofctl -O OpenFlow11 packet-out br0 none controller '0001020304050010203040501234'])
-AT_CHECK([ovs-ofctl -O OpenFlow11 packet-out br0 4294967293 controller '0001020304050010203040505678'])
+# Send some packet-outs with OFPP_NONE and OFPP_CONTROLLER as in_port.
+AT_CHECK([ovs-appctl -t ovs-ofctl ofctl/packet-out "in_port=none, packet=0001020304050010203040501234 actions=controller"])
+AT_CHECK([ovs-appctl -t ovs-ofctl ofctl/packet-out "in_port=controller packet=0001020304050010203040505678 actions=controller"])
 
 # Stop the monitor and check its output.
 ovs-appctl -t ovs-ofctl ofctl/barrier
 OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 
-AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
+AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//
+/PACKET_OUT/d' monitor.log], [0], [dnl
 OFPT_PACKET_IN (OF1.1): total_len=14 in_port=ANY (via action) data_len=14 (unbuffered)
 vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
 OFPT_PACKET_IN (OF1.1): total_len=14 in_port=CONTROLLER (via action) data_len=14 (unbuffered)
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index b56e5b3..76b3aa3 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -473,10 +473,11 @@ buckets of the group are removed.
 .
 Transactional updates to both flow and group tables can be made with
 the \fBbundle\fR command.  \fIfile\fR is a text file that contains
-zero or more flows and groups in either \fBFlow Syntax\fR or \fBGroup
-Syntax\fR, each line preceded by either \fBflow\fR or \fBgroup\fR
-keyword.  The \fBflow\fR keyword may be optionally followed by one of
-the keywords \fBadd\fR, \fBmodify\fR, \fBmodify_strict\fR,
+zero or more flow mods, group mods, or packet-outs in \fBFlow
+Syntax\fR, \fBGroup Syntax\fR, or \fBPacket\-Out Syntax\fR, each line
+preceded by \fBflow\fR, \fBgroup\fR, or \fBpacket\-out\fR keyword,
+correspondingly.  The \fBflow\fR keyword may be optionally followed by
+one of the keywords \fBadd\fR, \fBmodify\fR, \fBmodify_strict\fR,
 \fBdelete\fR, or \fBdelete_strict\fR, of which the \fBadd\fR is
 assumed if a bare \fBflow\fR is given.  Similarly, the \fBgroup\fR
 keyword may be optionally followed by one of the keywords \fBadd\fR,
@@ -2756,6 +2757,35 @@ of \fBduration\fR.  (This is separate from \fBduration\fR because
 The integer number of seconds that have passed without any packets
 passing through the flow.
 .
+.SS "Packet\-Out Syntax"
+.PP
+\fBovs\-ofctl bundle\fR command accepts packet-outs to be specified in
+the bundle file.  Each packet-out comprises of a series of
+\fIfield\fB=\fIvalue\fR assignments, separated by commas or white
+space.  (Embedding spaces into a packet-out description normally
+requires quoting to prevent the shell from breaking the description
+into multiple arguments.).  Unless noted otherwise only the last
+instance of each field is honoured.
+.PP
+.IP \fBin_port=\fIport\fR
+The port number to be considered the in_port when processing actions.  This can be any valid OpenFlow port number, or any of the \fBLOCAL\fR, \fBCONTROLLER\fR, or \fBNONE\fR.
+.
+This field is required.
+
+.IP \fBpacket=\fIhex-string\fR
+The actual packet to send, expressed as a string of hexadecimal bytes.
+.
+This field is required.
+
+.IP \fBactions=\fR[\fIaction\fR][\fB,\fIaction\fR...]\fR
+The syntax of actions are identical to the \fBactions=\fR field
+described in \fBFlow Syntax\fR above. Specifying \fBactions=\fR is
+optional, but omitting actions is interpreted as a drop, so the packet
+will not be sent anywhere from the switch.
+.
+\fBactions\fR must be specified at the end of each line, like for flow mods.
+.RE
+.
 .SS "Group Syntax"
 .PP
 Some \fBovs\-ofctl\fR commands accept an argument that describes a group or
@@ -2897,7 +2927,7 @@ of OpenFlow are used.  Open vSwitch will automatically allocate bucket
 ids when they are not specified.
 .IP \fBactions=\fR[\fIaction\fR][\fB,\fIaction\fR...]\fR
 The syntax of actions are identical to the \fBactions=\fR field described in
-\fBFlow Syntax\fR above. Specyfing \fBactions=\fR is optional, any unknown
+\fBFlow Syntax\fR above. Specifying \fBactions=\fR is optional, any unknown
 bucket parameter will be interpreted as an action.
 .IP \fBweight=\fIvalue\fR
 The relative weight of the bucket as an integer. This may be used by the switch
@@ -3176,6 +3206,12 @@ Sends each \fIofmsg\fR, specified as a sequence of hex digits that
 express an OpenFlow message, on the OpenFlow connection.  This command
 is useful only when executing the \fBmonitor\fR command.
 .
+.IP "\fBofctl/packet\-out \fBin_port=\fIport\fR \fBpacket=\fIhex\-string\fR \fBactions=\fIactions\fR"
+Sends an OpenFlow PACKET_OUT message specified as a sequence of hex
+digits that express an Ethernet frame, on the OpenFlow connection.
+See \fBPacket\-Out Syntax\fR section for more information.  This
+command is useful only when executing the \fBmonitor\fR command.
+.
 .IP "\fBofctl/barrier\fR"
 Sends an OpenFlow barrier request on the OpenFlow connection and waits
 for a reply.  This command is useful only for the \fBmonitor\fR
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 306bb78..edc00f7 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1682,6 +1682,58 @@ ofctl_send(struct unixctl_conn *conn, int argc,
     ds_destroy(&reply);
 }
 
+static void
+unixctl_packet_out(struct unixctl_conn *conn, int OVS_UNUSED argc,
+                   const char *argv[], void *vconn_)
+{
+    struct vconn *vconn = vconn_;
+    enum ofputil_protocol protocol
+        = ofputil_protocol_from_ofp_version(vconn_get_version(vconn));
+    struct ds reply = DS_EMPTY_INITIALIZER;
+    bool ok = true;
+
+    enum ofputil_protocol usable_protocols;
+    struct ofputil_packet_out po;
+    char *error_msg;
+
+    error_msg = parse_ofp_packet_out_str(&po, argv[1], &usable_protocols);
+    if (error_msg) {
+        ds_put_format(&reply, "%s\n", error_msg);
+        free(error_msg);
+        ok = false;
+    }
+
+    if (ok && !(usable_protocols & protocol)) {
+        ds_put_format(&reply, "PACKET_OUT actions are incompatible with the OpenFlow connection.\n");
+        ok = false;
+    }
+
+    if (ok) {
+        struct ofpbuf *msg = ofputil_encode_packet_out(&po, protocol);
+
+        ofp_print(stderr, msg->data, msg->size, verbosity);
+
+        int error = vconn_send_block(vconn, msg);
+        if (error) {
+            ofpbuf_delete(msg);
+            ds_put_format(&reply, "%s\n", ovs_strerror(error));
+            ok = false;
+        }
+    }
+
+    if (ok) {
+        unixctl_command_reply(conn, ds_cstr(&reply));
+    } else {
+        unixctl_command_reply_error(conn, ds_cstr(&reply));
+    }
+    ds_destroy(&reply);
+
+    if (!error_msg) {
+        free(CONST_CAST(void *, po.packet));
+        free(po.ofpacts);
+    }
+}
+
 struct barrier_aux {
     struct vconn *vconn;        /* OpenFlow connection for sending barrier. */
     struct unixctl_conn *conn;  /* Connection waiting for barrier response. */
@@ -1782,6 +1834,8 @@ monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests,
     unixctl_command_register("exit", "", 0, 0, ofctl_exit, &exiting);
     unixctl_command_register("ofctl/send", "OFMSG...", 1, INT_MAX,
                              ofctl_send, vconn);
+    unixctl_command_register("ofctl/packet-out", "\"[in_port=<port>] [buffer_id=<buffer>] [packet=<hex data>] [actions=...]\"", 1, 1,
+                             unixctl_packet_out, vconn);
     unixctl_command_register("ofctl/barrier", "", 0, 0,
                              ofctl_barrier, &barrier_aux);
     unixctl_command_register("ofctl/set-output-file", "FILE", 1, 1,
@@ -2780,6 +2834,7 @@ ofctl_bundle(struct ovs_cmdl_context *ctx)
     protocol = open_vconn_for_flow_mod(ctx->argv[1], &vconn, usable_protocols);
 
     ovs_list_init(&requests);
+    /* Takes ownership of 'bms'. */
     ofputil_encode_bundle_msgs(bms, n_bms, &requests, protocol);
     bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
     ofpbuf_list_delete(&requests);
-- 
2.1.4




More information about the dev mailing list