ofp-util: Fix OF1.4+ version of ofputil_decode_set_async_config().

The OF1.0 through OF1.3 "set async config" set the whole configuration,
OF1.4+ only update parts of it piecemeal, but the decoding function always
set the whole configuration.  This commit fixes the problem by changing the
interface to require the caller to provide an initial state.  (It would
be possible to simply make it mutate existing state in-place, but that
interface seems a little more error-prone.)

Found by inspection.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index 537f660b974044fde493fe4d6a98ca4736272dfc..62ebddcd6a93497aac82e5683171a93ebb651d74 100644 (file)
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -192,10 +192,7 @@ OpenFlow 1.4 features are listed in the previous section.
 
   * More extensible wire protocol
     Many on-wire structures got TLVs.
-    Already implemented: port desc properties, port mod properties,
-                         port stats properties, table mod properties,
-                         queue stats, unified property errors, queue desc.
-    Remaining required: set-async
+    All required features are now supported.
     Remaining optional: table desc, table-status
     [EXT-262]
     [required for OF1.4+]

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index accde8ed06116c5f8231284ff15635e8f0971248..f36335bb7e0e4f8e0ab19bd6dd53905c385bd349 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2130,9 +2130,12 @@ ofp_print_nxt_set_async_config(struct ds *string,
         }
     } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
                raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
-        struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT;
+        struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
+        struct ofputil_async_cfg ac;
+
         bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY;
-        enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, &ac);
+        enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
+                                                            &basis, &ac);
         if (error) {
             ofp_print_error(string, error);
             return;

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d28fc0208e171a7c7a02affc65512817994bde92..1e3d5b2c4226d0f10e0446a41a5e43c257df9a9b 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9572,6 +9572,11 @@ decode_legacy_async_masks(const ovs_be32 masks[2],
 /* Decodes the OpenFlow "set async config" request and "get async config
  * reply" message in '*oh' into an abstract form in 'ac'.
  *
+ * Some versions of the "set async config" request change only some of the
+ * settings and leave the others alone.  This function uses 'basis' as the
+ * initial state for decoding these.  Other versions of the request change all
+ * the settings; this function ignores 'basis' when decoding these.
+ *
  * If 'loose' is true, this function ignores properties and values that it does
  * not understand, as a controller would want to do when interpreting
  * capabilities provided by a switch.  If 'loose' is false, this function
@@ -9587,6 +9592,7 @@ decode_legacy_async_masks(const ovs_be32 masks[2],
  * supported.*/
 enum ofperr
 ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
+                                const struct ofputil_async_cfg *basis,
                                 struct ofputil_async_cfg *ac)
 {
     enum ofpraw raw;
@@ -9600,6 +9606,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
         raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) {
         const struct nx_async_config *msg = ofpmsg_body(oh);
 
+        *ac = OFPUTIL_ASYNC_CFG_INIT;
         decode_legacy_async_masks(msg->packet_in_mask, OAM_PACKET_IN,
                                   oh->version, ac);
         decode_legacy_async_masks(msg->port_status_mask, OAM_PORT_STATUS,
@@ -9608,6 +9615,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
                                   oh->version, ac);
     } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
                raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
+        *ac = *basis;
         while (b.size > 0) {
             struct ofpbuf property;
             enum ofperr error;

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 52268d87cbe937ba4ac92491b506b1c3c59a47a2..88c67f99459c172ba53f1f9d69e03dac8c0dd0cf 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -1317,6 +1317,7 @@ struct ofputil_async_cfg {
 
 enum ofperr ofputil_decode_set_async_config(const struct ofp_header *,
                                             bool loose,
+                                            const struct ofputil_async_cfg *,
                                             struct ofputil_async_cfg *);
 
 struct ofpbuf *ofputil_encode_get_async_config(

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index fbaf7dd914caaa8e2d17584abcfedf704447f1f0..957e3231298c131689b5585c83a930f7c909aa06 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5403,10 +5403,11 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn,
 static enum ofperr
 handle_nxt_set_async_config(struct ofconn *ofconn, const struct ofp_header *oh)
 {
-    struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT;
+    struct ofputil_async_cfg basis = ofconn_get_async_config(ofconn);
+    struct ofputil_async_cfg ac;
     enum ofperr error;
 
-    error = ofputil_decode_set_async_config(oh, false, &ac);
+    error = ofputil_decode_set_async_config(oh, false, &basis, &ac);
     if (error) {
         return error;
     }