[ovs-dev] [PATCH] ofp-msgs: ofpmp_reserve() must add "more" flag when splitting stats.

Ben Pfaff blp at nicira.com
Fri Aug 3 22:56:53 UTC 2012


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 at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-msgs.c   |    4 ++++
 tests/ofproto.at |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
index 07ee87a..00e1a84 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 2528a73..46675bc 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])
-- 
1.7.2.5




More information about the dev mailing list