ofproto: Correct encoding and decoding of group desc properties.

* encode: if properties are present include their length in
          value of the length field of the group desc
* decode: use the value of the length field to calculate the length of
          properties rather than assuming that the rest of the message
          is properties. This assumption is not correct as a message
          may contain multiple group descs.

Fixes: 18ac06d3546e ("ofp-util: Encoding and decoding of (draft) OpenFlow 1.5 group messages.")
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 97b5449..342be54 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7783,7 +7783,6 @@ ofputil_append_ofp15_group_desc_reply(const struct ofputil_group_desc *gds,
                                  gds->type, reply, version);
     }
     ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
-    ogds->length = htons(reply->size - start_ogds);
     ogds->type = gds->type;
     ogds->group_id = htonl(gds->group_id);
     ogds->bucket_list_len =  htons(reply->size - start_buckets);
@@ -7793,6 +7792,7 @@ ofputil_append_ofp15_group_desc_reply(const struct ofputil_group_desc *gds,
         ofputil_put_group_prop_ntr_selection_method(version, &gds->props,
                                                     reply);
     }
+    ogds->length = htons(reply->size - start_ogds);
 
     ofpmp_postappend(replies, start_ogds);
 }
@@ -8338,7 +8338,7 @@ ofputil_decode_ofp15_group_desc_reply(struct ofputil_group_desc *gd,
      * claim that the group mod command is OFPGC15_ADD to
      * satisfy the check in parse_group_prop_ntr_selection_method() */
     return parse_ofp15_group_properties(msg, gd->type, OFPGC15_ADD, &gd->props,
-                                        msg->size);
+                                        length - sizeof *ogds - bucket_list_len);
 }
 
 /* Converts a group description reply in 'msg' into an abstract

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 8cdcead..25cf950 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2026,7 +2026,7 @@ AT_SETUP([OFPST_GROUP_DESC reply - OF1.5])
 AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
 AT_CHECK([ovs-ofctl ofp-print "\
 06 13 00 d8 00 00 00 02 00 07 00 00 00 00 00 00 \
-00 88 01 00 00 00 20 00 00 78 00 00 00 00 00 00 \
+00 c8 01 00 00 00 20 00 00 78 00 00 00 00 00 00 \
 00 28 00 10 00 00 00 00 00 00 00 10 00 00 00 01 \
 00 00 00 00 00 00 00 00 00 00 00 08 00 64 00 00 \
 00 01 00 08 00 00 00 01 \

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 5e4441c..8699c4a 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -343,6 +343,7 @@ dnl Actions definition listed in both supported formats (w/ actions=)
 AT_SETUP([ofproto - del group (OpenFlow 1.5)])
 OVS_VSWITCHD_START
 AT_DATA([groups.txt], [dnl
+group_id=1233,type=select,selection_method=hash,bucket=output:10,bucket=output:11
 group_id=1234,type=select,selection_method=hash,bucket=output:10,bucket=output:11
 group_id=1235,type=all,bucket=actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
 ])
@@ -357,12 +358,14 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPST_GROUP_DESC reply (OF1.5):
  group_id=1235,type=all,bucket=bucket_id:2,actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
+ group_id=1233,type=select,selection_method=hash,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
 ])
 AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn del-groups br0 group_id=1234])
 AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPST_GROUP_DESC reply (OF1.5):
  group_id=1235,type=all,bucket=bucket_id:2,actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
+ group_id=1233,type=select,selection_method=hash,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
 ])
 AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn del-groups br0], [0])
 AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])