On Thu, Jul 05, 2012 at 11:12:29PM -0700, Ben Pfaff wrote:
> OpenFlow headers are not as uniform as they could be, with size, alignment,
> and numbering changes from one version to another and across varieties
> (e.g. ordinary messages vs. "stats" messages). Until now the Open vSwitch
> internal APIs haven't done a good job of abstracting those differences in
> header formats. This commit changes that; from this commit forward very
> little code actually needs to understand the header format or numbering.
> Instead, it can just encode or decode, or pull or put, the header using
> a more abstract API using the ofpraw_, ofptype_, and other APIs in the
> new ofp-msgs module.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> v3: New commit. This isn't complete yet: in particular the program for
> extracting the numbers from the ofp-msgs.h header file isn't written,
> although there's a skeleton. That means that nothing has been tested or
> carefully looked over, either.
>
> v3->v4: Almost ready. The ofp-msgs.c functions need some comments
> and careful review.
>
> v4->v5: Ready for review.
Not a proper review, but I noticed a compile-time warning.
[snip]
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 7b0b220..425448e 100644
[snip]
> @@ -620,18 +608,21 @@ fetch_port_by_stats(const char *vconn_name,
> run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive
> failed");
> recv_xid = ((struct ofp_header *) reply->data)->xid;
> if (send_xid == recv_xid) {
> - const struct ofputil_msg_type *type;
> - struct ofp_stats_msg *osm;
> -
> - ofputil_decode_msg_type(reply->data, &type);
> - if (ofputil_msg_type_code(type) !=
> OFPUTIL_OFPST_PORT_DESC_REPLY) {
> + struct ofp_header *oh;
> + enum ofptype type;
> + struct ofpbuf b;
> + uint16_t flags;
> +
> + ofpbuf_use_const(&b, oh, ntohs(oh->length));
gcc tells me that oh is used uninitialised here and that does seem
to be the case to me too.
Should it be initialised to ((struct ofp_header *) reply->data) ?
> + if (ofptype_pull(&type, &b)
> + || type != OFPTYPE_PORT_DESC_STATS_REPLY) {
> ovs_fatal(0, "received bad reply: %s",
> ofp_to_string(reply->data, reply->size,
> verbosity + 1));
> }
>
> - osm = ofpbuf_at_assert(reply, 0, sizeof *osm);
> - done = !(ntohs(osm->flags) & OFPSF_REPLY_MORE);
> + flags = ofpmp_flags(oh);
> + done = !(flags & OFPSF_REPLY_MORE);
>
> if (found) {
> /* We've already found the port, but we need to drain
[ snip ]
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
