Ben, Tested, works, and looks good. I can now see port duration in "ovs-ofctl -O OpenFlow13 dump-ports br0" :-)
Jarno On May 24, 2013, at 2:56 , ext Ben Pfaff wrote: > Signed-off-by: Ben Pfaff <b...@nicira.com> > --- > OPENFLOW-1.1+ | 3 ++- > lib/ofp-print.c | 6 ++++++ > lib/ofp-util.c | 17 +++++++---------- > lib/ofp-util.h | 2 ++ > ofproto/ofproto-provider.h | 1 + > ofproto/ofproto.c | 17 +++++++++++------ > tests/ofp-print.at | 39 +++++++++++++++++++++++++++++++++++++++ > 7 files changed, 68 insertions(+), 17 deletions(-) > > diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+ > index b6a4222..d312dab 100644 > --- a/OPENFLOW-1.1+ > +++ b/OPENFLOW-1.1+ > @@ -162,7 +162,8 @@ didn't compare the specs carefully yet.) > * Rework tag order. I'm not sure whether we need to do anything > for this. > > - * Duration for stats. > + * Duration for queue stats. (Duration for port stats is already > + implemented.) > > * On-demand flow counters. I think this might be a real > optimization in some cases for the software switch. > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index e899df3..0a9917a 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -1218,6 +1218,12 @@ ofp_print_ofpst_port_reply(struct ds *string, const > struct ofp_header *oh, > print_port_stat(string, "drop=", ps.stats.tx_dropped, 1); > print_port_stat(string, "errs=", ps.stats.tx_errors, 1); > print_port_stat(string, "coll=", ps.stats.collisions, 0); > + > + if (ps.duration_sec != UINT32_MAX) { > + ds_put_cstr(string, " duration="); > + ofp_print_duration(string, ps.duration_sec, ps.duration_nsec); > + ds_put_char(string, '\n'); > + } > } > } > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index b4ff09b..3c8d5a2 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -4595,11 +4595,8 @@ ofputil_port_stats_to_ofp13(const struct > ofputil_port_stats *ops, > struct ofp13_port_stats *ps13) > { > ofputil_port_stats_to_ofp11(ops, &ps13->ps); > - > - /* OF 1.3 adds duration fields */ > - /* FIXME: Need to implement port alive duration (sec + nsec) */ > - ps13->duration_sec = htonl(~0); > - ps13->duration_nsec = htonl(~0); > + ps13->duration_sec = htonl(ops->duration_sec); > + ps13->duration_nsec = htonl(ops->duration_nsec); > } > > > @@ -4655,6 +4652,7 @@ ofputil_port_stats_from_ofp10(struct ofputil_port_stats > *ops, > ops->stats.rx_over_errors = > ntohll(get_32aligned_be64(&ps10->rx_over_err)); > ops->stats.rx_crc_errors = ntohll(get_32aligned_be64(&ps10->rx_crc_err)); > ops->stats.collisions = ntohll(get_32aligned_be64(&ps10->collisions)); > + ops->duration_sec = ops->duration_nsec = UINT32_MAX; > > return 0; > } > @@ -4683,6 +4681,7 @@ ofputil_port_stats_from_ofp11(struct ofputil_port_stats > *ops, > ops->stats.rx_over_errors = ntohll(ps11->rx_over_err); > ops->stats.rx_crc_errors = ntohll(ps11->rx_crc_err); > ops->stats.collisions = ntohll(ps11->collisions); > + ops->duration_sec = ops->duration_nsec = UINT32_MAX; > > return 0; > } > @@ -4691,13 +4690,11 @@ static enum ofperr > ofputil_port_stats_from_ofp13(struct ofputil_port_stats *ops, > const struct ofp13_port_stats *ps13) > { > - enum ofperr error = > - ofputil_port_stats_from_ofp11(ops, &ps13->ps); > + enum ofperr error = ofputil_port_stats_from_ofp11(ops, &ps13->ps); > if (!error) { > - /* FIXME: Get ps13->duration_sec and ps13->duration_nsec, > - * Add to netdev_stats? */ > + ops->duration_sec = ntohl(ps13->duration_sec); > + ops->duration_nsec = ntohl(ps13->duration_nsec); > } > - > return error; > } > > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 4d0d8ad..010c34d 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -689,6 +689,8 @@ bool ofputil_parse_key_value(char **stringp, char **keyp, > char **valuep); > struct ofputil_port_stats { > uint16_t port_no; > struct netdev_stats stats; > + uint32_t duration_sec; /* UINT32_MAX if unknown. */ > + uint32_t duration_nsec; > }; > > struct ofpbuf *ofputil_encode_dump_ports_request(enum ofp_version ofp_version, > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index b9d6f0d..db0d589 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -119,6 +119,7 @@ struct ofport { > struct ofputil_phy_port pp; > uint16_t ofp_port; /* OpenFlow port number. */ > unsigned int change_seq; > + long long int created; /* Time created, in msec. */ > int mtu; > }; > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index ca1dc89..2f91d65 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -203,6 +203,8 @@ static bool handle_openflow(struct ofconn *, struct > ofpbuf *); > static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *, > const struct ofputil_flow_mod *, > const struct ofp_header *); > +static void calc_duration(long long int start, long long int now, > + uint32_t *sec, uint32_t *nsec); > > /* ofproto. */ > static uint64_t pick_datapath_id(const struct ofproto *); > @@ -1787,6 +1789,7 @@ ofport_install(struct ofproto *p, > ofport->change_seq = netdev_change_seq(netdev); > ofport->pp = *pp; > ofport->ofp_port = pp->port_no; > + ofport->created = time_msec(); > > /* Add port to 'p'. */ > hmap_insert(&p->ports, &ofport->hmap_node, hash_int(ofport->ofp_port, 0)); > @@ -2544,6 +2547,9 @@ append_port_stat(struct ofport *port, struct list > *replies) > { > struct ofputil_port_stats ops = { .port_no = port->pp.port_no }; > > + calc_duration(port->created, time_msec(), > + &ops.duration_sec, &ops.duration_nsec); > + > /* Intentionally ignore return value, since errors will set > * 'stats' to all-1s, which is correct for OpenFlow, and > * netdev_get_stats() will log errors. */ > @@ -2604,8 +2610,8 @@ handle_port_desc_stats_request(struct ofconn *ofconn, > } > > static void > -calc_flow_duration__(long long int start, long long int now, > - uint32_t *sec, uint32_t *nsec) > +calc_duration(long long int start, long long int now, > + uint32_t *sec, uint32_t *nsec) > { > long long int msecs = now - start; > *sec = msecs / 1000; > @@ -2824,8 +2830,7 @@ handle_flow_stats_request(struct ofconn *ofconn, > fs.priority = rule->cr.priority; > fs.cookie = rule->flow_cookie; > fs.table_id = rule->table_id; > - calc_flow_duration__(rule->created, now, &fs.duration_sec, > - &fs.duration_nsec); > + calc_duration(rule->created, now, &fs.duration_sec, > &fs.duration_nsec); > fs.idle_timeout = rule->idle_timeout; > fs.hard_timeout = rule->hard_timeout; > fs.idle_age = age_secs(now - rule->used); > @@ -3438,8 +3443,8 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t > reason) > fr.cookie = rule->flow_cookie; > fr.reason = reason; > fr.table_id = rule->table_id; > - calc_flow_duration__(rule->created, time_msec(), > - &fr.duration_sec, &fr.duration_nsec); > + calc_duration(rule->created, time_msec(), > + &fr.duration_sec, &fr.duration_nsec); > fr.idle_timeout = rule->idle_timeout; > fr.hard_timeout = rule->hard_timeout; > rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count, > diff --git a/tests/ofp-print.at b/tests/ofp-print.at > index 074a60b..caf0ecc 100644 > --- a/tests/ofp-print.at > +++ b/tests/ofp-print.at > @@ -1395,6 +1395,45 @@ OFPST_PORT reply (OF1.2) (xid=0x2): 3 ports > ]) > AT_CLEANUP > > +AT_SETUP([OFPST_PORT reply - OF1.3]) > +AT_KEYWORDS([ofp-print OFPT_STATS_REPLY]) > +AT_CHECK([ovs-ofctl ofp-print "\ > +04 13 01 60 00 00 00 02 00 04 00 00 00 00 00 00 \ > +00 00 00 02 00 00 00 00 00 00 00 00 00 01 95 56 \ > +00 00 00 00 00 00 00 88 00 00 00 00 02 5d 08 98 \ > +00 00 00 00 00 00 2c f8 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 01 00 0f 42 40 \ > +ff ff ff fe 00 00 00 00 \ > +00 00 00 00 00 00 00 44 00 00 00 00 00 00 9d 2c \ > +00 00 00 00 00 00 16 7c 00 00 00 00 01 1e 36 44 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +ff ff ff ff ff ff ff ff \ > +00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 44 \ > +00 00 00 00 00 00 9d 2c 00 00 00 00 00 00 16 7c \ > +00 00 00 00 01 1e 36 44 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ > +00 00 00 00 00 00 00 00 00 00 00 00 07 54 d4 c0 \ > +"], [0], [dnl > +OFPST_PORT reply (OF1.3) (xid=0x2): 3 ports > + port 2: rx pkts=103766, bytes=39651480, drop=0, errs=0, frame=0, over=0, > crc=0 > + tx pkts=136, bytes=11512, drop=0, errs=0, coll=0 > + duration=1.001s > + port LOCAL: rx pkts=68, bytes=5756, drop=0, errs=0, frame=0, over=0, crc=0 > + tx pkts=40236, bytes=18757188, drop=0, errs=0, coll=0 > + port 1: rx pkts=68, bytes=5756, drop=0, errs=0, frame=0, over=0, crc=0 > + tx pkts=40236, bytes=18757188, drop=0, errs=0, coll=0 > + duration=0.123s > +]) > +AT_CLEANUP > + > AT_SETUP([OFPST_QUEUE request - OF1.0]) > AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST]) > AT_CHECK([ovs-ofctl ofp-print "\ > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev