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

[Ethan Jackson]

Looks good to me, thanks.

Ethan

On Fri, Aug 3, 2012 at 3:56 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev