Introduce ofpacts, an abstraction of OpenFlow actions.

OpenFlow actions have always been somewhat awkward to handle.
Moreover, over time we've started creating actions that require more
complicated parsing.  When we maintain those actions internally in
their wire format, we end up parsing them multiple times, whenever
we have to look at the set of actions.

When we add support for OpenFlow 1.1 or later protocols, the situation
will get worse, because these newer protocols support many of the same
actions but with different representations.  It becomes unrealistic to
handle each protocol in its wire format.

This commit adopts a new strategy, by converting OpenFlow actions into
an internal form from the wire format when they are read, and converting
them back to the wire format when flows are dumped.  I believe that this
will be more maintainable over time.

Thanks to Simon Horman and Pravin Shelar for reviews.

Signed-off-by: Ben Pfaff <[email protected]>

DESIGN

@@ -612,6 +612,26 @@ The following are explicitly *not* supported by in-band control:
      gateway.
 
 
+Action Reproduction
+===================
+
+It seems likely that many controllers, at least at startup, use the
+OpenFlow "flow statistics" request to obtain existing flows, then
+compare the flows' actions against the actions that they expect to
+find.  Before version 1.8.0, Open vSwitch always returned exact,
+byte-for-byte copies of the actions that had been added to the flow
+table.  The current version of Open vSwitch does not always do this in
+some exceptional cases.  This section lists the exceptions that
+controller authors must keep in mind if they compare actual actions
+against desired actions in a bytewise fashion:
+
+        - Open vSwitch zeros padding bytes in action structures,
+          regardless of their values when the flows were added.
+
+Please report other discrepancies, if you notice any, so that we can
+fix or document them.
+
+
 Suggestions
 ===========
 

NEWS

@@ -1,6 +1,9 @@
 post-v1.7.0
 ------------------------
     - New FAQ.  Please send updates and additions!
+    - Authors of controllers, please read the new section titled "Action
+      Reproduction" in DESIGN, which describes an Open vSwitch change in
+      behavior in corner cases that may affect some controllers.
     - ovs-l3ping:
         - A new test utility that can create L3 tunnel between two Open
           vSwitches and detect connectivity issues.

lib/automake.mk

@@ -96,6 +96,8 @@ lib_libopenvswitch_a_SOURCES = \
 	lib/nx-match.h \
 	lib/odp-util.c \
 	lib/odp-util.h \
+	lib/ofp-actions.c \
+	lib/ofp-actions.h \
 	lib/ofp-errors.c \
 	lib/ofp-errors.h \
 	lib/ofp-parse.c \

lib/autopath.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 Nicira, Inc.
+ * Copyright (c) 2011, 2012 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,39 +24,29 @@
 #include "flow.h"
 #include "meta-flow.h"
 #include "nx-match.h"
+#include "ofp-actions.h"
 #include "ofp-errors.h"
 #include "ofp-util.h"
 #include "openflow/nicira-ext.h"
 #include "vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(autopath);
 
-/* Loads 'ofp_port' into the appropriate register in accordance with the
- * autopath action. */
 void
-autopath_execute(const struct nx_action_autopath *ap, struct flow *flow,
-                 uint16_t ofp_port)
-{
-    struct mf_subfield dst;
-
-    nxm_decode(&dst, ap->dst, ap->ofs_nbits);
-    mf_set_subfield_value(&dst, ofp_port, flow);
-}
-
-void
-autopath_parse(struct nx_action_autopath *ap, const char *s_)
+autopath_parse(struct ofpact_autopath *ap, const char *s_)
 {
     char *s;
-    char *id_str, *dst_s, *save_ptr;
-    struct mf_subfield dst;
     int id_int;
+    char *id_str, *dst, *save_ptr;
+
+    ofpact_init_AUTOPATH(ap);
 
     s = xstrdup(s_);
     save_ptr = NULL;
     id_str = strtok_r(s, ", ", &save_ptr);
-    dst_s = strtok_r(NULL, ", ", &save_ptr);
+    dst = strtok_r(NULL, ", ", &save_ptr);
 
-    if (!dst_s) {
+    if (!dst) {
         ovs_fatal(0, "%s: not enough arguments to autopath action", s_);
     }
 
@@ -65,33 +55,51 @@ autopath_parse(struct nx_action_autopath *ap, const char *s_)
         ovs_fatal(0, "%s: autopath id %d is not in valid range "
                   "1 to %"PRIu32, s_, id_int, UINT32_MAX);
     }
+    ap->port = id_int;
 
-    mf_parse_subfield(&dst, dst_s);
-    if (dst.n_bits < 16) {
+    mf_parse_subfield(&ap->dst, dst);
+    if (ap->dst.n_bits < 16) {
         ovs_fatal(0, "%s: %d-bit destination field has %u possible values, "
                   "less than required 65536",
-                  s_, dst.n_bits, 1u << dst.n_bits);
+                  s_, ap->dst.n_bits, 1u << ap->dst.n_bits);
     }
 
-    ofputil_init_NXAST_AUTOPATH(ap);
-    ap->id = htonl(id_int);
-    ap->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits);
-    ap->dst = htonl(dst.field->nxm_header);
-
     free(s);
 }
 
 enum ofperr
-autopath_check(const struct nx_action_autopath *ap, const struct flow *flow)
+autopath_from_openflow(const struct nx_action_autopath *nap,
+                       struct ofpact_autopath *autopath)
 {
-    struct mf_subfield dst;
+    ofpact_init_AUTOPATH(autopath);
+    autopath->dst.field = mf_from_nxm_header(ntohl(nap->dst));
+    autopath->dst.ofs = nxm_decode_ofs(nap->ofs_nbits);
+    autopath->dst.n_bits = nxm_decode_n_bits(nap->ofs_nbits);
+    autopath->port = ntohl(nap->id);
 
-    nxm_decode(&dst, ap->dst, ap->ofs_nbits);
-    if (dst.n_bits < 16) {
+    if (autopath->dst.n_bits < 16) {
         VLOG_WARN("at least 16 bit destination is required for autopath "
                   "action.");
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
 
-    return mf_check_dst(&dst, flow);
+    return autopath_check(autopath, NULL);
+}
+
+enum ofperr
+autopath_check(const struct ofpact_autopath *autopath, const struct flow *flow)
+{
+    return mf_check_dst(&autopath->dst, flow);
+}
+
+void
+autopath_to_nxast(const struct ofpact_autopath *autopath,
+                  struct ofpbuf *openflow)
+{
+    struct nx_action_autopath *ap = ofputil_put_NXAST_AUTOPATH(openflow);
+
+    ap->ofs_nbits = nxm_encode_ofs_nbits(autopath->dst.ofs,
+                                         autopath->dst.n_bits);
+    ap->dst = htonl(autopath->dst.field->nxm_header);
+    ap->id = htonl(autopath->port);
 }

lib/autopath.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 Nicira, Inc.
+ * Copyright (c) 2011, 2012 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,15 +22,20 @@
 
 struct flow;
 struct nx_action_autopath;
+struct ofpact_autopath;
+struct ofpbuf;
 
 /* NXAST_AUTOPATH  helper functions.
  *
  * See include/openflow/nicira-ext.h for NXAST_AUTOPATH specification. */
 
-void autopath_execute(const struct nx_action_autopath *, struct flow *,
-                      uint16_t ofp_port);
-void autopath_parse(struct nx_action_autopath *, const char *);
-enum ofperr autopath_check(const struct nx_action_autopath *,
+void autopath_parse(struct ofpact_autopath *, const char *);
+
+enum ofperr autopath_from_openflow(const struct nx_action_autopath *,
+                                   struct ofpact_autopath *);
+enum ofperr autopath_check(const struct ofpact_autopath *,
                            const struct flow *);
+void autopath_to_nxast(const struct ofpact_autopath *,
+                       struct ofpbuf *openflow);
 
 #endif /* autopath.h */