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 537f660..62ebddc 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 accde8e..f36335b 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 d28fc02..1e3d5b2 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 52268d8..88c67f9 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 fbaf7dd..957e323 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;
     }