Looks Good.
On Fri, Mar 11, 2011 at 1:20 PM, Ben Pfaff <[email protected]> wrote: > This function will see more use later in this series. This commit just > starts using it to make ofp-print output entirely consistent for > OFPST_FLOW and NXST_FLOW replies. > --- > lib/ofp-print.c | 157 ++++++++------------------------------------------- > lib/ofp-util.c | 116 +++++++++++++++++++++++++++++++++++++- > lib/ofp-util.h | 21 +++++++- > tests/ofp-print.at | 8 +- > 4 files changed, 163 insertions(+), 139 deletions(-) > > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index 181a743..a879bd2 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -1083,147 +1083,43 @@ ofp_print_flow_stats_request(struct ds *string, > const struct ofp_header *oh) > } > > static void > -ofp_print_ofpst_flow_reply(struct ds *string, const struct ofp_header *oh, > - int verbosity) > +ofp_print_flow_stats_reply(struct ds *string, const struct ofp_header *oh) > { > - size_t len = ofputil_stats_body_len(oh); > - const char *body = ofputil_stats_body(oh); > - const char *pos = body; > - for (;;) { > - const struct ofp_flow_stats *fs; > - ptrdiff_t bytes_left = body + len - pos; > - size_t length; > + struct ofpbuf b; > > - ds_put_char(string, '\n'); > + ofpbuf_use_const(&b, oh, ntohs(oh->length)); > + for (;;) { > + struct ofputil_flow_stats fs; > + int retval; > > - if (bytes_left < sizeof *fs) { > - if (bytes_left != 0) { > - ds_put_format(string, " ***%td leftover bytes at end***", > - bytes_left); > + retval = ofputil_decode_flow_stats_reply(&fs, &b, NXFF_OPENFLOW10); > + if (retval) { > + if (retval != EOF) { > + ds_put_cstr(string, " ***parse error***"); > } > break; > } > > - fs = (const void *) pos; > - length = ntohs(fs->length); > - if (length < sizeof *fs) { > - ds_put_format(string, " ***length=%zu shorter than minimum > %zu***", > - length, sizeof *fs); > - break; > - } else if (length > bytes_left) { > - ds_put_format(string, > - " ***length=%zu but only %td bytes left***", > - length, bytes_left); > - break; > - } else if ((length - sizeof *fs) % sizeof fs->actions[0]) { > - ds_put_format(string, > - " ***length=%zu has %zu bytes leftover in " > - "final action***", > - length, > - (length - sizeof *fs) % sizeof fs->actions[0]); > - break; > - } > - > - ds_put_format(string, " cookie=0x%"PRIx64", duration=", > - ntohll(get_32aligned_be64(&fs->cookie))); > - ofp_print_duration(string, ntohl(fs->duration_sec), > - ntohl(fs->duration_nsec)); > - ds_put_format(string, ", table_id=%"PRIu8", ", fs->table_id); > - if (fs->priority != htons(OFP_DEFAULT_PRIORITY)) { > - ds_put_format(string, "priority=%"PRIu16", ", > ntohs(fs->priority)); > - } > - ds_put_format(string, "n_packets=%"PRIu64", ", > - ntohll(get_32aligned_be64(&fs->packet_count))); > - ds_put_format(string, "n_bytes=%"PRIu64", ", > - ntohll(get_32aligned_be64(&fs->byte_count))); > - if (fs->idle_timeout != htons(OFP_FLOW_PERMANENT)) { > - ds_put_format(string, "idle_timeout=%"PRIu16",", > - ntohs(fs->idle_timeout)); > - } > - if (fs->hard_timeout != htons(OFP_FLOW_PERMANENT)) { > - ds_put_format(string, "hard_timeout=%"PRIu16",", > - ntohs(fs->hard_timeout)); > - } > - ofp_print_match(string, &fs->match, verbosity); > - ds_put_char(string, ' '); > - ofp_print_actions(string, fs->actions, length - sizeof *fs); > - > - pos += length; > - } > -} > - > -static void > -ofp_print_nxst_flow_reply(struct ds *string, const struct ofp_header *oh) > -{ > - struct ofpbuf b; > - > - ofpbuf_use_const(&b, ofputil_nxstats_body(oh), > - ofputil_nxstats_body_len(oh)); > - while (b.size > 0) { > - const struct nx_flow_stats *fs; > - union ofp_action *actions; > - struct cls_rule rule; > - size_t actions_len, n_actions; > - size_t length; > - int match_len; > - int error; > - > ds_put_char(string, '\n'); > > - fs = ofpbuf_try_pull(&b, sizeof *fs); > - if (!fs) { > - ds_put_format(string, " ***%td leftover bytes at end***", > b.size); > - break; > - } > - > - length = ntohs(fs->length); > - if (length < sizeof *fs) { > - ds_put_format(string, " ***nx_flow_stats claims length %zu***", > - length); > - break; > - } > - > - match_len = ntohs(fs->match_len); > - if (match_len > length - sizeof *fs) { > - ds_put_format(string, " ***length=%zu match_len=%d***", > - length, match_len); > - break; > - } > - > ds_put_format(string, " cookie=0x%"PRIx64", duration=", > - ntohll(fs->cookie)); > - ofp_print_duration(string, ntohl(fs->duration_sec), > - ntohl(fs->duration_nsec)); > - ds_put_format(string, ", table_id=%"PRIu8", ", fs->table_id); > - ds_put_format(string, "n_packets=%"PRIu64", ", > - ntohll(fs->packet_count)); > - ds_put_format(string, "n_bytes=%"PRIu64", ", ntohll(fs->byte_count)); > - if (fs->idle_timeout != htons(OFP_FLOW_PERMANENT)) { > - ds_put_format(string, "idle_timeout=%"PRIu16",", > - ntohs(fs->idle_timeout)); > - } > - if (fs->hard_timeout != htons(OFP_FLOW_PERMANENT)) { > - ds_put_format(string, "hard_timeout=%"PRIu16",", > - ntohs(fs->hard_timeout)); > + ntohll(fs.cookie)); > + ofp_print_duration(string, fs.duration_sec, fs.duration_nsec); > + ds_put_format(string, ", table_id=%"PRIu8", ", fs.table_id); > + ds_put_format(string, "n_packets=%"PRIu64", ", fs.packet_count); > + ds_put_format(string, "n_bytes=%"PRIu64", ", fs.byte_count); > + if (fs.idle_timeout != OFP_FLOW_PERMANENT) { > + ds_put_format(string, "idle_timeout=%"PRIu16",", > fs.idle_timeout); > } > - > - error = nx_pull_match(&b, match_len, ntohs(fs->priority), &rule); > - if (error) { > - ofp_print_error(string, error); > - break; > + if (fs.hard_timeout != OFP_FLOW_PERMANENT) { > + ds_put_format(string, "hard_timeout=%"PRIu16",", > fs.hard_timeout); > } > > - actions_len = length - sizeof *fs - ROUND_UP(match_len, 8); > - error = ofputil_pull_actions(&b, actions_len, &actions, &n_actions); > - if (error) { > - ofp_print_error(string, error); > - break; > - } > - > - cls_rule_format(&rule, string); > + cls_rule_format(&fs.rule, string); > ds_put_char(string, ' '); > - ofp_print_actions(string, (const struct ofp_action_header *) actions, > - n_actions * sizeof *actions); > + ofp_print_actions(string, > + (const struct ofp_action_header *) fs.actions, > + fs.n_actions * sizeof *fs.actions); > } > } > > @@ -1587,8 +1483,9 @@ ofp_to_string__(const struct ofp_header *oh, > break; > > case OFPUTIL_OFPST_FLOW_REPLY: > + case OFPUTIL_NXST_FLOW_REPLY: > ofp_print_stats_reply(string, oh); > - ofp_print_ofpst_flow_reply(string, oh, verbosity); > + ofp_print_flow_stats_reply(string, oh); > break; > > case OFPUTIL_OFPST_QUEUE_REPLY: > @@ -1633,10 +1530,6 @@ ofp_to_string__(const struct ofp_header *oh, > ofp_print_flow_mod(string, msg, code, verbosity); > break; > > - case OFPUTIL_NXST_FLOW_REPLY: > - ofp_print_nxst_flow_reply(string, oh); > - break; > - > case OFPUTIL_NXST_AGGREGATE_REPLY: > ofp_print_stats_reply(string, oh); > ofp_print_nxst_aggregate_reply(string, msg); > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index f017fc9..8dbcb2b 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -29,6 +29,7 @@ > #include "ofpbuf.h" > #include "packets.h" > #include "random.h" > +#include "unaligned.h" > #include "type-props.h" > #include "vlog.h" > > @@ -1157,7 +1158,7 @@ ofputil_decode_nxst_flow_request(struct > flow_stats_request *fsr, > } > > /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE > - * message 'oh', received when the current flow format was 'flow_format', > into > + * request 'oh', received when the current flow format was 'flow_format', > into > * an abstract flow_stats_request in 'fsr'. Returns 0 if successful, > otherwise > * an OpenFlow error code. > * > @@ -1197,7 +1198,7 @@ ofputil_decode_flow_stats_request(struct > flow_stats_request *fsr, > } > > /* Converts abstract flow_stats_request 'fsr' into an OFPST_FLOW, > - * OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE message 'oh' according to > + * OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE request 'oh' according to > * 'flow_format', and returns the message. */ > struct ofpbuf * > ofputil_encode_flow_stats_request(const struct flow_stats_request *fsr, > @@ -1239,6 +1240,117 @@ ofputil_encode_flow_stats_request(const struct > flow_stats_request *fsr, > return msg; > } > > +/* Converts an OFPST_FLOW or NXST_FLOW reply in 'msg' into an abstract > + * ofputil_flow_stats in 'fs'. For OFPST_FLOW messages, 'flow_format' should > + * be the current flow format at the time when the request corresponding to > the > + * reply in 'msg' was sent. Otherwise 'flow_format' is ignored. > + * > + * Multiple OFPST_FLOW or NXST_FLOW replies can be packed into a single > + * OpenFlow message. Calling this function multiple times for a single 'msg' > + * iterates through the replies. The caller must initially leave 'msg''s > layer > + * pointers null and not modify them between calls. > + * > + * Returns 0 if successful, EOF if no replies were left in this 'msg', > + * otherwise a positive errno value. */ > +int > +ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, > + struct ofpbuf *msg, > + enum nx_flow_format flow_format) > +{ > + const struct ofputil_msg_type *type; > + int code; > + > + ofputil_decode_msg_type(msg->l2 ? msg->l2 : msg->data, &type); > + code = ofputil_msg_type_code(type); > + if (!msg->l2) { > + msg->l2 = msg->data; > + if (code == OFPUTIL_OFPST_FLOW_REPLY) { > + ofpbuf_pull(msg, sizeof(struct ofp_stats_reply)); > + } else if (code == OFPUTIL_NXST_FLOW_REPLY) { > + ofpbuf_pull(msg, sizeof(struct nicira_stats_msg)); > + } else { > + NOT_REACHED(); > + } > + } > + > + if (!msg->size) { > + return EOF; > + } else if (code == OFPUTIL_OFPST_FLOW_REPLY) { > + const struct ofp_flow_stats *ofs; > + size_t length; > + > + ofs = ofpbuf_try_pull(msg, sizeof *ofs); > + if (!ofs) { > + VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply has %zu leftover " > + "bytes at end", msg->size); > + return EINVAL; > + } > + > + length = ntohs(ofs->length); > + if (length < sizeof *ofs) { > + VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply claims invalid " > + "length %zu", length); > + return EINVAL; > + } > + > + if (ofputil_pull_actions(msg, length - sizeof *ofs, > + &fs->actions, &fs->n_actions)) { > + return EINVAL; > + } > + > + fs->cookie = get_32aligned_be64(&ofs->cookie); > + ofputil_cls_rule_from_match(&ofs->match, ntohs(ofs->priority), > + flow_format, fs->cookie, &fs->rule); > + fs->table_id = ofs->table_id; > + fs->duration_sec = ntohl(ofs->duration_sec); > + fs->duration_nsec = ntohl(ofs->duration_nsec); > + fs->idle_timeout = ntohs(ofs->idle_timeout); > + fs->hard_timeout = ntohs(ofs->hard_timeout); > + fs->packet_count = ntohll(get_32aligned_be64(&ofs->packet_count)); > + fs->byte_count = ntohll(get_32aligned_be64(&ofs->byte_count)); > + } else if (code == OFPUTIL_NXST_FLOW_REPLY) { > + const struct nx_flow_stats *nfs; > + size_t match_len, length; > + > + nfs = ofpbuf_try_pull(msg, sizeof *nfs); > + if (!nfs) { > + VLOG_WARN_RL(&bad_ofmsg_rl, "NXST_FLOW reply has %zu leftover " > + "bytes at end", msg->size); > + return EINVAL; > + } > + > + length = ntohs(nfs->length); > + match_len = ntohs(nfs->match_len); > + if (length < sizeof *nfs + ROUND_UP(match_len, 8)) { > + VLOG_WARN_RL(&bad_ofmsg_rl, "NXST_FLOW reply with match_len=%zu " > + "claims invalid length %zu", match_len, length); > + return EINVAL; > + } > + if (nx_pull_match(msg, match_len, ntohs(nfs->priority), &fs->rule)) { > + return EINVAL; > + } > + > + if (ofputil_pull_actions(msg, > + length - sizeof *nfs - ROUND_UP(match_len, > 8), > + &fs->actions, &fs->n_actions)) { > + return EINVAL; > + } > + > + fs->cookie = nfs->cookie; > + fs->table_id = nfs->table_id; > + fs->duration_sec = ntohl(nfs->duration_sec); > + fs->duration_nsec = ntohl(nfs->duration_nsec); > + fs->idle_timeout = ntohs(nfs->idle_timeout); > + fs->hard_timeout = ntohs(nfs->hard_timeout); > + fs->packet_count = ntohll(nfs->packet_count); > + fs->byte_count = ntohll(nfs->byte_count); > + } else { > + NOT_REACHED(); > + } > + > + return 0; > +} > + > /* Converts an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message 'oh', received > * when the current flow format was 'flow_format', into an abstract > * ofputil_flow_removed in 'fr'. Returns 0 if successful, otherwise an > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 6eff980..ff6faa4 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2008, 2009, 2010 Nicira Networks. > + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -157,6 +157,25 @@ int ofputil_decode_flow_stats_request(struct > flow_stats_request *, > struct ofpbuf *ofputil_encode_flow_stats_request( > const struct flow_stats_request *, enum nx_flow_format); > > +/* Flow stats reply, independent of flow format. */ > +struct ofputil_flow_stats { > + struct cls_rule rule; > + ovs_be64 cookie; > + uint8_t table_id; > + uint32_t duration_sec; > + uint32_t duration_nsec; > + uint16_t idle_timeout; > + uint16_t hard_timeout; > + uint64_t packet_count; > + uint64_t byte_count; > + union ofp_action *actions; > + size_t n_actions; > +}; > + > +int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *, > + struct ofpbuf *msg, > + enum nx_flow_format); > + > /* Flow removed message, independent of flow format. */ > struct ofputil_flow_removed { > struct cls_rule rule; > diff --git a/tests/ofp-print.at b/tests/ofp-print.at > index 0aee111..84ad30a 100644 > --- a/tests/ofp-print.at > +++ b/tests/ofp-print.at > @@ -445,10 +445,10 @@ c0 a8 00 02 00 08 00 00 00 00 00 09 05 b8 d8 00 \ > 00 00 04 fa 00 00 00 08 00 01 00 00 \ > "], [0], [dnl > OFPST_FLOW reply (xid=0x4): > - cookie=0x0, duration=4.2s, table_id=0, priority=65535, n_packets=1, > n_bytes=60, > idle_timeout=5,arp,in_port=3,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,opcode=2,nw_tos=0,tp_src=0,tp_dst=0 > actions=output:1 > - cookie=0x0, duration=8.9s, table_id=0, priority=65535, n_packets=13, > n_bytes=1274, > idle_timeout=5,icmp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,icmp_type=0,icmp_code=0 > actions=output:3 > - cookie=0x0, duration=4.28s, table_id=0, priority=65535, n_packets=1, > n_bytes=60, > idle_timeout=5,arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=1,nw_tos=0,icmp_type=0,icmp_code=0 > actions=output:3 > - cookie=0x0, duration=9.096s, table_id=0, n_packets=13, n_bytes=1274, > idle_timeout=5,icmp,in_port=*,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,icmp_type=8,icmp_code=0 > actions=output:1 > + cookie=0x0, duration=4.2s, table_id=0, n_packets=1, n_bytes=60, > idle_timeout=5,priority=65535,arp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,opcode=2,nw_tos=0,tp_src=0,tp_dst=0 > actions=output:1 > + cookie=0x0, duration=8.9s, table_id=0, n_packets=13, n_bytes=1274, > idle_timeout=5,priority=65535,icmp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,icmp_type=0,icmp_code=0 > actions=output:3 > + cookie=0x0, duration=4.28s, table_id=0, n_packets=1, n_bytes=60, > idle_timeout=5,priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=1,nw_tos=0,icmp_type=0,icmp_code=0 > actions=output:3 > + cookie=0x0, duration=9.096s, table_id=0, n_packets=13, n_bytes=1274, > idle_timeout=5,icmp,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,icmp_type=8,icmp_code=0 > actions=output:1 > ]) > AT_CLEANUP > > -- > 1.7.1 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
