diff mbox

[ovs-dev,1/2] ofproto: correct encoding and decoding of group desc properties

Message ID 1444992648-27107-2-git-send-email-simon.horman@netronome.com
State Accepted
Headers show

Commit Message

Simon Horman Oct. 16, 2015, 10:50 a.m. UTC 
* 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>
---
 lib/ofp-util.c     | 4 ++--
 tests/ofp-print.at | 2 +-
 tests/ofproto.at   | 3 +++
 3 files changed, 6 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Oct. 16, 2015, 3:27 p.m. UTC | #1 
On Fri, Oct 16, 2015 at 07:50:47PM +0900, Simon Horman wrote:
> * 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>

Applied to master and branch-2.4, thanks Simon!
diff mbox

Patch

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 97b5449f62fb..342be5409ece 100644
--- 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 8cdceade0244..25cf950c685b 100644
--- 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 5e4441c7e1d7..8699c4a29ed9 100644
--- 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])