Before I read this patch I have a high level question.

We currently publish the duration which is the number of seconds since
the rule has been created.  It seems like the API would be more
consistent and conceptually cleaner, if similarly we published the
number of seconds since modified, and the number of seconds since
used.  From this information a controller could trivial calculate the
time until a rule will be ejected due to the hard or idle timeouts.
Basically, I'm asking why we are publishing a countdown vs a count-up
value.

Thoughts?  I'll hold of reading the patch until you've responded.

Ethan


On Wed, Feb 1, 2012 at 16:35, Ben Pfaff <b...@nicira.com> wrote:
> The "learn" action is useful for MAC learning, but until now there has been
> no way to find out through OpenFlow how much time remains before a MAC
> learning entry (a learned flow) expires.  This commit adds that ability.
>
> Feature #7193.
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
>  NEWS                          |    2 +
>  include/openflow/nicira-ext.h |   29 +++++++++++++++++++--
>  lib/learning-switch.c         |    1 +
>  lib/ofp-print.c               |   11 +++++++-
>  lib/ofp-util.c                |   34 +++++++++++++++++++++++-
>  lib/ofp-util.h                |    8 ++++-
>  ofproto/ofproto.c             |   24 ++++++++++++++++++
>  tests/ofp-print.at            |   16 ++++++------
>  tests/ofproto-dpif.at         |   55 
> +++++++++++++++++++++++++++++++++++++++++
>  tests/ofproto-macros.at       |    2 +
>  utilities/ovs-ofctl.8.in      |   41 +++++++++++++++++++-----------
>  utilities/ovs-ofctl.c         |    2 +-
>  12 files changed, 193 insertions(+), 32 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 592e013..c3cf232 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -15,6 +15,8 @@ post-v1.5.0
>       table, with configurable policy for evicting flows upon
>       overflow.  See the Flow_Table table in ovs-vswitch.conf.db(5)
>       for more information.
> +    - OpenFlow:
> +      - NXM flow dumps now include time left before hard and idle timeouts.
>     - ofproto-provider interface:
>         - "struct rule" has a new member "used" that ofproto implementations
>           should maintain by updating with ofproto_rule_update_used().
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 6921803..d850bb0 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -108,7 +108,12 @@ enum nicira_type {
>
>     /* Alternative PACKET_IN message formats. */
>     NXT_SET_PACKET_IN_FORMAT = 16, /* Set Packet In format. */
> -    NXT_PACKET_IN = 17             /* Nicira Packet In. */
> +    NXT_PACKET_IN = 17,            /* Nicira Packet In. */
> +
> +    /* Are the idle_left and hard_left members in struct nx_flow_stats
> +     * supported?  If so, the switch does not reply to this message.  If not,
> +     * the switch sends an error reply. */
> +    NXT_FLOW_LEFT = 18,
>  };
>
>  /* Header for Nicira vendor stats request and reply messages. */
> @@ -1763,7 +1768,24 @@ struct nx_flow_stats_request {
>  OFP_ASSERT(sizeof(struct nx_flow_stats_request) == 32);
>
>  /* Body for Nicira vendor stats reply of type NXST_FLOW (analogous to
> - * OFPST_FLOW reply). */
> + * OFPST_FLOW reply).
> + *
> + * The value of 'idle_left' is only meaningful when talking to a switch that
> + * implements the NXT_FLOW_LEFT extension.  There are three possibilities:
> + *
> + *   - idle_left == 0: The amount of idle time before the flow expires is
> + *     unknown, or 'idle_timeout' is 0 (OFP_FLOW_PERMANENT).  This is also 
> the
> + *     value that one should ordinarily expect to see talking to a switch 
> that
> + *     does not implement NXT_FLOW_ELAPSED, since those switches zero the
> + *     padding bytes that 'idle_left' replaced.
> + *
> + *   - 0 < idle_left <= idle_timeout: There are approximately 'idle_left'
> + *     seconds until the flow expires, if no (more) packets arrive in the 
> flow.
> + *
> + *   - idle_left > idle_timeout: Invalid.
> + *
> + * 'hard_left' may be interpreted similarly with respect to hard_timeout.
> + */
>  struct nx_flow_stats {
>     ovs_be16 length;          /* Length of this entry. */
>     uint8_t table_id;         /* ID of table flow came from. */
> @@ -1776,7 +1798,8 @@ struct nx_flow_stats {
>     ovs_be16 idle_timeout;    /* Number of seconds idle before expiration. */
>     ovs_be16 hard_timeout;    /* Number of seconds before expiration. */
>     ovs_be16 match_len;       /* Length of nx_match. */
> -    uint8_t pad2[4];          /* Align to 64 bits. */
> +    ovs_be16 idle_left;       /* Remaining seconds idle before expiration. */
> +    ovs_be16 hard_left;       /* Remaining seconds before hard expiration. */
>     ovs_be64 cookie;          /* Opaque controller-issued identifier. */
>     ovs_be64 packet_count;    /* Number of packets, UINT64_MAX if unknown. */
>     ovs_be64 byte_count;      /* Number of bytes, UINT64_MAX if unknown. */
> diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> index 60cf731..e578395 100644
> --- a/lib/learning-switch.c
> +++ b/lib/learning-switch.c
> @@ -264,6 +264,7 @@ lswitch_process_packet(struct lswitch *sw, struct rconn 
> *rconn,
>     case OFPUTIL_NXT_PACKET_IN:
>     case OFPUTIL_NXT_FLOW_MOD:
>     case OFPUTIL_NXT_FLOW_REMOVED:
> +    case OFPUTIL_NXT_FLOW_LEFT:
>     case OFPUTIL_NXST_FLOW_REQUEST:
>     case OFPUTIL_NXST_AGGREGATE_REQUEST:
>     case OFPUTIL_NXST_FLOW_REPLY:
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 3c5c34a..f4051f6 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1022,7 +1022,7 @@ ofp_print_flow_stats_reply(struct ds *string, const 
> struct ofp_header *oh)
>         struct ofputil_flow_stats fs;
>         int retval;
>
> -        retval = ofputil_decode_flow_stats_reply(&fs, &b);
> +        retval = ofputil_decode_flow_stats_reply(&fs, &b, true);
>         if (retval) {
>             if (retval != EOF) {
>                 ds_put_cstr(string, " ***parse error***");
> @@ -1044,6 +1044,12 @@ ofp_print_flow_stats_reply(struct ds *string, const 
> struct ofp_header *oh)
>         if (fs.hard_timeout != OFP_FLOW_PERMANENT) {
>             ds_put_format(string, "hard_timeout=%"PRIu16",", fs.hard_timeout);
>         }
> +        if (fs.idle_left) {
> +            ds_put_format(string, "idle_left=%"PRIu16",", fs.idle_left);
> +        }
> +        if (fs.hard_left) {
> +            ds_put_format(string, "hard_left=%"PRIu16",", fs.hard_left);
> +        }
>
>         cls_rule_format(&fs.rule, string);
>         if (string->string[string->length - 1] != ' ') {
> @@ -1456,6 +1462,9 @@ ofp_to_string__(const struct ofp_header *oh,
>         ofp_print_flow_mod(string, msg, code, verbosity);
>         break;
>
> +    case OFPUTIL_NXT_FLOW_LEFT:
> +        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 2565ec7..a705277 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -385,6 +385,10 @@ ofputil_decode_vendor(const struct ofp_header *oh, 
> size_t length,
>         { OFPUTIL_NXT_FLOW_MOD_TABLE_ID, OFP10_VERSION,
>           NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID",
>           sizeof(struct nx_flow_mod_table_id), 0 },
> +
> +        { OFPUTIL_NXT_FLOW_LEFT, OFP10_VERSION,
> +          NXT_FLOW_LEFT, "NXT_FLOW_LEFT",
> +          sizeof(struct nicira_header), 0 },
>     };
>
>     static const struct ofputil_msg_category nxt_category = {
> @@ -1290,11 +1294,18 @@ ofputil_encode_flow_stats_request(const struct 
> ofputil_flow_stats_request *fsr,
>  * iterates through the replies.  The caller must initially leave 'msg''s 
> layer
>  * pointers null and not modify them between calls.
>  *
> + * Most switches don't send the values needed to populate fs->idle_left and
> + * fs->hard_left, so those members will usually be set to 0.  If the switch
> + * from which 'fs' implements For NXST_FLOW
> + * replies when 'flow_left_extension' true, the contents of 'msg' determine 
> the
> + * 'idle_left' and 'hard_left' members in 'fs'.
> + *
>  * 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)
> +                                struct ofpbuf *msg,
> +                                bool flow_left_extension)
>  {
>     const struct ofputil_msg_type *type;
>     int code;
> @@ -1345,6 +1356,8 @@ ofputil_decode_flow_stats_reply(struct 
> ofputil_flow_stats *fs,
>         fs->duration_nsec = ntohl(ofs->duration_nsec);
>         fs->idle_timeout = ntohs(ofs->idle_timeout);
>         fs->hard_timeout = ntohs(ofs->hard_timeout);
> +        fs->idle_left = 0;
> +        fs->hard_left = 0;
>         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) {
> @@ -1382,6 +1395,22 @@ ofputil_decode_flow_stats_reply(struct 
> ofputil_flow_stats *fs,
>         fs->duration_nsec = ntohl(nfs->duration_nsec);
>         fs->idle_timeout = ntohs(nfs->idle_timeout);
>         fs->hard_timeout = ntohs(nfs->hard_timeout);
> +        if (flow_left_extension) {
> +            fs->idle_left = ntohs(nfs->idle_left);
> +            if (fs->idle_left > fs->idle_timeout) {
> +                VLOG_WARN_RL(&bad_ofmsg_rl, "invalid idle_left in 
> NXST_FLOW");
> +                fs->idle_left = 0;
> +            }
> +
> +            fs->hard_left = ntohs(nfs->hard_left);
> +            if (fs->hard_left > fs->hard_timeout) {
> +                VLOG_WARN_RL(&bad_ofmsg_rl, "invalid hard_left in 
> NXST_FLOW");
> +                fs->hard_left = 0;
> +            }
> +        } else {
> +            fs->idle_left = 0;
> +            fs->hard_left = 0;
> +        }
>         fs->packet_count = ntohll(nfs->packet_count);
>         fs->byte_count = ntohll(nfs->byte_count);
>     } else {
> @@ -1450,8 +1479,9 @@ ofputil_append_flow_stats_reply(const struct 
> ofputil_flow_stats *fs,
>         nfs->priority = htons(fs->rule.priority);
>         nfs->idle_timeout = htons(fs->idle_timeout);
>         nfs->hard_timeout = htons(fs->hard_timeout);
> +        nfs->idle_left = htons(fs->idle_left);
> +        nfs->hard_left = htons(fs->hard_left);
>         nfs->match_len = htons(nx_put_match(msg, &fs->rule, 0, 0));
> -        memset(nfs->pad2, 0, sizeof nfs->pad2);
>         nfs->cookie = fs->cookie;
>         nfs->packet_count = htonll(fs->packet_count);
>         nfs->byte_count = htonll(fs->byte_count);
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 422c14a..149cf51 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -79,6 +79,7 @@ enum ofputil_msg_code {
>     OFPUTIL_NXT_FLOW_REMOVED,
>     OFPUTIL_NXT_SET_PACKET_IN_FORMAT,
>     OFPUTIL_NXT_PACKET_IN,
> +    OFPUTIL_NXT_FLOW_LEFT,
>
>     /* NXST_* stat requests. */
>     OFPUTIL_NXST_FLOW_REQUEST,
> @@ -183,6 +184,8 @@ struct ofputil_flow_stats {
>     uint32_t duration_nsec;
>     uint16_t idle_timeout;
>     uint16_t hard_timeout;
> +    uint16_t idle_left;         /* Seconds to idle expiration, 0 if unknown. 
> */
> +    uint16_t hard_left;         /* Seconds to hard expiration, 0 if unknown. 
> */
>     uint64_t packet_count;      /* Packet count, UINT64_MAX if unknown. */
>     uint64_t byte_count;        /* Byte count, UINT64_MAX if unknown. */
>     union ofp_action *actions;
> @@ -190,7 +193,8 @@ struct ofputil_flow_stats {
>  };
>
>  int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *,
> -                                    struct ofpbuf *msg);
> +                                    struct ofpbuf *msg,
> +                                    bool flow_left_extension);
>  void ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *,
>                                      struct list *replies);
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index c35d440..cf041ba 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2337,6 +2337,24 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t 
> table_id,
>     return 0;
>  }
>
> +/* If 'timeout' is 0, returns 0.  Otherwise, returns a value between 1 and
> + * 'timeout', inclusive, that reports the number of seconds until a
> + * 'timeout'-second timer started at 'start' (in ms, as returned by
> + * time_msec()) expires. */
> +static uint16_t
> +time_left(long long int start, uint16_t timeout)
> +{
> +    if (!timeout) {
> +        return 0;
> +    } else {
> +        /* mecs in which the flow expires, plus .5 to round to nearest. */
> +        long long int ms_left = (start + timeout * 1000) - time_msec() + 500;
> +        return (ms_left < 1000 ? 1
> +                : ms_left >= 1000 * timeout ? timeout
> +                : (unsigned int) ms_left / 1000);
> +    }
> +}
> +
>  static enum ofperr
>  handle_flow_stats_request(struct ofconn *ofconn,
>                           const struct ofp_stats_msg *osm)
> @@ -2371,6 +2389,8 @@ handle_flow_stats_request(struct ofconn *ofconn,
>                              &fs.duration_nsec);
>         fs.idle_timeout = rule->idle_timeout;
>         fs.hard_timeout = rule->hard_timeout;
> +        fs.idle_left = time_left(rule->used, rule->idle_timeout);
> +        fs.hard_left = time_left(rule->modified, rule->hard_timeout);
>         ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
>                                                &fs.byte_count);
>         fs.actions = rule->actions;
> @@ -3199,6 +3219,10 @@ handle_openflow__(struct ofconn *ofconn, const struct 
> ofpbuf *msg)
>     case OFPUTIL_NXT_FLOW_MOD:
>         return handle_flow_mod(ofconn, oh);
>
> +    case OFPUTIL_NXT_FLOW_LEFT:
> +        /* Nothing to do. */
> +        return 0;
> +
>         /* Statistics requests. */
>     case OFPUTIL_OFPST_DESC_REQUEST:
>         return handle_desc_stats_request(ofconn, msg->data);
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 85562b6..9b6ae5e 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -798,7 +798,7 @@ AT_CHECK([ovs-ofctl ofp-print "\
>  a8 00 02 00 00 0c 01 06 00 00 12 02 09 e7 00 00 \
>  14 02 00 00 00 00 00 00 00 00 00 08 00 01 00 00 \
>  00 88 00 00 00 00 00 03 32 11 62 00 ff ff 00 05 \
> -00 00 00 4c 00 00 00 00 00 00 00 00 00 00 00 00 \
> +00 00 00 4c 00 03 00 00 00 00 00 00 00 00 00 00 \
>  00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 3c \
>  00 00 00 02 00 03 00 00 02 06 50 54 00 00 00 06 \
>  00 00 04 06 50 54 00 00 00 05 00 00 06 02 08 00 \
> @@ -806,7 +806,7 @@ a8 00 02 00 00 0c 01 06 00 00 12 02 09 e7 00 00 \
>  a8 00 01 00 00 10 04 c0 a8 00 02 00 00 0c 01 06 \
>  00 00 12 02 09 e4 00 00 14 02 00 00 00 00 00 00 \
>  00 00 00 08 00 01 00 00 00 88 00 00 00 00 00 02 \
> -33 f9 aa 00 ff ff 00 05 00 00 00 4c 00 00 00 00 \
> +33 f9 aa 00 ff ff 00 05 00 00 00 4c 00 05 00 00 \
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 \
>  00 00 00 00 00 00 00 3c 00 00 00 02 00 01 00 00 \
>  02 06 50 54 00 00 00 05 00 00 04 06 50 54 00 00 \
> @@ -815,7 +815,7 @@ a8 00 01 00 00 10 04 c0 a8 00 02 00 00 0c 01 06 \
>  a8 00 01 00 00 0c 01 06 00 00 12 02 00 00 00 00 \
>  14 02 09 e5 00 00 00 00 00 00 00 08 00 03 00 00 \
>  00 88 00 00 00 00 00 04 2d 0f a5 00 ff ff 00 05 \
> -00 00 00 4c 00 00 00 00 00 00 00 00 00 00 00 00 \
> +00 00 00 4c 00 01 00 00 00 00 00 00 00 00 00 00 \
>  00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 3c \
>  00 00 00 02 00 03 00 00 02 06 50 54 00 00 00 06 \
>  00 00 04 06 50 54 00 00 00 05 00 00 06 02 08 00 \
> @@ -823,7 +823,7 @@ a8 00 01 00 00 0c 01 06 00 00 12 02 00 00 00 00 \
>  a8 00 01 00 00 10 04 c0 a8 00 02 00 00 0c 01 06 \
>  00 00 12 02 09 e3 00 00 14 02 00 00 00 00 00 00 \
>  00 00 00 08 00 01 00 00 00 88 00 00 00 00 00 02 \
> -34 73 bc 00 ff ff 00 05 00 00 00 4c 00 00 00 00 \
> +34 73 bc 00 ff ff 00 05 00 0a 00 4c 00 03 00 08 \
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 \
>  00 00 00 00 00 00 00 3c 00 00 00 02 00 03 00 00 \
>  02 06 50 54 00 00 00 06 00 00 04 06 50 54 00 00 \
> @@ -920,10 +920,10 @@ ff ff 00 18 00 00 23 20 00 07 00 1f 00 01 00 04 \
>  "], [0],
>  [[NXST_FLOW reply (xid=0x4):
>  cookie=0x0, duration=1.048s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,nw_tos=0,tp_src=2535,tp_dst=0
>  actions=output:1
> - cookie=0x0, duration=3.84s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,nw_tos=0,tp_src=2532,tp_dst=0
>  actions=output:1
> - cookie=0x0, duration=2.872s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,tp_src=0,tp_dst=2533
>  actions=output:3
> - cookie=0x0, duration=4.756s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,nw_tos=0,tp_src=2531,tp_dst=0
>  actions=output:1
> - cookie=0x0, duration=2.88s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,nw_tos=0,tp_src=2533,tp_dst=0
>  actions=output:1
> + cookie=0x0, duration=3.84s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,idle_left=3,priority=65535,tcp,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,nw_tos=0,tp_src=2532,tp_dst=0
>  actions=output:1
> + cookie=0x0, duration=2.872s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,idle_left=5,priority=65535,tcp,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,tp_src=0,tp_dst=2533
>  actions=output:3
> + cookie=0x0, duration=4.756s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,idle_left=1,priority=65535,tcp,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,nw_tos=0,tp_src=2531,tp_dst=0
>  actions=output:1
> + cookie=0x0, duration=2.88s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,hard_timeout=10,idle_left=3,hard_left=8,priority=65535,tcp,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,nw_tos=0,tp_src=2533,tp_dst=0
>  actions=output:1
>  cookie=0x0, duration=5.672s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,nw_tos=0,tp_src=2530,tp_dst=0
>  actions=output:1
>  cookie=0x0, duration=1.04s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,tp_src=0,tp_dst=2535
>  actions=output:3
>  cookie=0x0, duration=1.952s, table=0, n_packets=1, n_bytes=60, 
> idle_timeout=5,priority=65535,tcp,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,tp_src=0,tp_dst=2534
>  actions=output:3
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 6b4c216..fdcd9ed 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -1035,3 +1035,58 @@ echo $n_recs
>  AT_CHECK([test $n_recs -ge 13])
>
>  AT_CLEANUP
> +
> +AT_SETUP([idle_left and hard_left decline over time])
> +OVS_VSWITCHD_START
> +
> +AT_CHECK([ovs-ofctl add-flow br0 
> hard_timeout=99,idle_timeout=88,actions=drop])
> +
> +# Fetch the hard and idle time left into variables whose names are
> +# given in $1 and $2, respectively, checking that each value is a
> +# positive integer.
> +get_time_left () {
> +    AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout])
> +
> +    hard=`sed -n 's/.*hard_left=\([[0-9]]*\),.*/\1/p' stdout`
> +    AT_CHECK([[expr X"$hard" : 'X[1-9][0-9]*$']], [0], [ignore])
> +    AS_VAR_COPY([$1], [hard])
> +
> +    idle=`sed -n 's/.*idle_left=\([[0-9]]*\),.*/\1/p' stdout`
> +    AT_CHECK([[expr X"$idle" : 'X[1-9][0-9]*$']], [0], [ignore])
> +    AS_VAR_COPY([$2], [idle])
> +}
> +
> +# Get the initial hard and idle time left.  This should presumably be
> +# 99 and 88, respectively, but if we're running slowly could be less,
> +# so we don't bother to check
> +get_time_left hard1 idle1
> +
> +# Warp time forward by 10 seconds.
> +ovs-appctl time/warp 10000
> +get_time_left hard2 idle2
> +
> +# Warp time forward 10 more seconds, then pass some packets through the flow,
> +# then warp forward a few more times because idle times are only updated
> +# occasionally.
> +ovs-appctl time/warp 10000
> +ovs-appctl netdev-dummy/receive br0 
> 'in_port(0),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no),tcp(src=80,dst=1234)'
> +ovs-ofctl dump-flows br0
> +get_time_left hard3 idle3
> +ovs-appctl time/warp 1000
> +ovs-appctl time/warp 1000
> +ovs-appctl time/warp 1000
> +
> +echo "hard_left: $hard1 => $hard2 => $hard3"
> +echo "idle_left: $idle1 => $idle2 => $idle3"
> +
> +# Hard time left should have decreased from 1 to 2 to 3.  It's hard to 
> predict
> +# how much, but at the very least it should be less than before
> +AT_CHECK([test $hard2 -lt $hard1])
> +AT_CHECK([test $hard3 -lt $hard2])
> +
> +# Idle time left should have decreased from 1 to 2, then increased to 3.
> +AT_CHECK([test $idle2 -lt $idle1])
> +AT_CHECK([test $idle3 -gt $idle2])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index 3656ecf..6fb6106 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -10,6 +10,8 @@ s/ cookie=0x0,//
>  s/ table=0,//
>  s/ n_packets=0,//
>  s/ n_bytes=0,//
> +s/idle_left=[0-9]*,//
> +s/hard_left=[0-9]*,//
>  '
>  }]
>  m4_divert_pop([PREPARE_TESTS])
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 0699d69..ea66ef7 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -1009,19 +1009,13 @@ If set, a matching flow must include an output action 
> to \fIport\fR.
>  .
>  The \fBdump\-tables\fR and \fBdump\-aggregate\fR commands print information
>  about the entries in a datapath's tables.  Each line of output is a
> -unique flow entry, which begins with some common information:
> +flow entry as described in \fBFlow Syntax\fR, above, plus some
> +additional fields:
>  .
> -.IP \fBduration\fR
> -The number of seconds the entry has been in the table.
> -.
> -.IP \fBtable\fR
> -The table that contains the flow.  When a packet arrives, the switch
> -begins searching for an entry at the lowest numbered table.  Tables are
> -numbered as shown by the \fBdump\-tables\fR command.
> -.
> -.IP \fBpriority\fR
> -The priority of the entry in relation to other entries within the same
> -table.  A higher value will match before a lower one.
> +.IP \fBduration=\fIsecs\fR
> +The time, in seconds, that the entry has been in the table.
> +\fIsecs\fR includes as much precision as the switch provides, possibly
> +to nanosecond resolution.
>  .
>  .IP \fBn_packets\fR
>  The number of packets that have matched the entry.
> @@ -1030,9 +1024,26 @@ The number of packets that have matched the entry.
>  The total number of bytes from packets that have matched the entry.
>  .
>  .PP
> -The rest of the line consists of a description of the flow entry as
> -described in \fBFlow Syntax\fR, above.
> -.
> +The following additional fields are included only if the flow has the
> +respective timeout, the switch is Open vSwitch 1.6 or later, and the
> +NXM flow format (see the description of the \fB\-\-flow-format\fR
> +option below) is used to dump the flow.  The values of these
> +additional fields are approximations only and in particular
> +\fBidle_left\fR will sometimes drop below \fBidle_timeout\fR even for
> +a very busy flow.
> +.
> +.IP \fBhard_left=\fIsecs\fR
> +The number of seconds that remain before the flow expires because the
> +\fBhard_timeout\fR has been reached, as an integer number of seconds.
> +(This is separate from \fBduration\fR because \fBmod\-flows\fR restarts
> +the \fBhard_timeout\fR timer without zeroing \fBduration\fR.)
> +.
> +.IP \fBidle_left=\fIsecs\fR
> +The number of seconds that remain before the flow expires because
> +\fBidle_timeout\fR seconds have passed without any packets passing
> +through the flow, as an integer number of seconds.  (If a packet
> +passes through the flow this duration elapses, the idle timeout will
> +not trigger.)
>  .
>  .SH OPTIONS
>  .TP
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 3d3a7b7..d64964c 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -1353,7 +1353,7 @@ read_flows_from_switch(struct vconn *vconn, enum 
> nx_flow_format flow_format,
>                 struct ofputil_flow_stats fs;
>                 int retval;
>
> -                retval = ofputil_decode_flow_stats_reply(&fs, reply);
> +                retval = ofputil_decode_flow_stats_reply(&fs, reply, false);
>                 if (retval) {
>                     if (retval != EOF) {
>                         ovs_fatal(0, "parse error in reply");
> --
> 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

Reply via email to