From: Ben Pfaff <blp@nicira.com>
Date: Fri, 3 Aug 2012 22:56:02 +0000 (-0700)
Subject: ofp-msgs: ofpmp_reserve() must add "more" flag when splitting stats.
X-Git-Tag: sliver-openvswitch-1.8.90-0~48^2~115
X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=79b8c36c58f979296ce2cec1e6d899fa8321bc21;p=sliver-openvswitch.git

ofp-msgs: ofpmp_reserve() must add "more" flag when splitting stats.

Commit 982697a4d2 "ofp-msgs: New approach to encoding and decoding OpenFlow
headers." introduced a bug in multipart message composition such that, when
a multipart message actually requires multiple parts, the OFPSF_REPLY_MORE
bit failed to be set on any of the parts, which in turn caused the receiver
to see a truncated reply.  This commit fixes the problem.

This commit also adds a test case that would have detected the problem and
fixes a minor bug in that test case (if an error actually occurred, then
the test case would hang).

Bug #12766.
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
---

diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
index 07ee87a3d..00e1a8448 100644
--- a/lib/ofp-msgs.c
+++ b/lib/ofp-msgs.c
@@ -789,6 +789,8 @@ ofpmsg_body(const struct ofp_header *oh)
     return (const uint8_t *) oh + ofphdrs_len(&hdrs);
 }
 
+static ovs_be16 *ofpmp_flags__(const struct ofp_header *);
+
 /* Initializes 'replies' as a new list of stats messages that reply to
  * 'request', which must be a stats request message.  Initially the list will
  * consist of only a single reply part without any body.  The caller should
@@ -832,6 +834,8 @@ ofpmp_reserve(struct list *replies, size_t len)
         ofpbuf_put(next, msg->data, hdrs_len);
         list_push_back(replies, &next->list_node);
 
+        *ofpmp_flags__(msg->data) |= htons(OFPSF_REPLY_MORE);
+
         return next;
     }
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2528a73c8..46675bce4 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -910,6 +910,7 @@ echo n_msgs=$n_msgs
 OVS_VSWITCHD_START
 
 # Start a monitor watching the flow table, then make it block.
+trap 'kill `cat ovsdb-server.pid ovs-vswitchd.pid ovs-ofctl.pid`' 0
 ovs-ofctl monitor br0 watch: --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 ovs-appctl -t ovs-ofctl ofctl/block
@@ -922,6 +923,8 @@ perl -e '
     }
 ') > flows.txt
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+# Check that multipart flow dumps work properly:
+AT_CHECK([ovs-ofctl diff-flows br0 flows.txt])
 AT_CHECK([ovs-ofctl add-flow br0 in_port=1,cookie=3,actions=drop])
 AT_CHECK([ovs-ofctl mod-flows br0 in_port=2,cookie=2,actions=output:2])
 AT_CHECK([ovs-ofctl del-flows br0 cookie=1/-1])