Sure, I'll do that quickly,
On Mon, Sep 30, 2013 at 6:05 PM, Ethan Jackson <et...@nicira.com> wrote:
> Both of these look good to me. Would you please rebase them and send
> them out. Then I'll merge.
>
> Ethan
>
> On Tue, Sep 24, 2013 at 5:54 PM, Alex Wang <al...@nicira.com> wrote:
> > So far, the subfacet rates (e.g. add rate, del rate) are computed by
> > exponential moving averaging function in ofproto-dpif.c. This commit
> > replaces that logic with coverage counters. And the rates can be
> > checked by running "ovs-appctl coverage/show" command.
> >
> > Signed-off-by: Alex Wang <al...@nicira.com>
> > ---
> > ofproto/ofproto-dpif.c | 101
> ++++--------------------------------------------
> > tests/ofproto-dpif.at | 4 --
> > tests/tunnel.at | 26 ++++++-------
> > 3 files changed, 21 insertions(+), 110 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 80874b8..5ddd71d 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -72,6 +72,10 @@ COVERAGE_DEFINE(facet_changed_rule);
> > COVERAGE_DEFINE(facet_revalidate);
> > COVERAGE_DEFINE(facet_unexpected);
> > COVERAGE_DEFINE(facet_suppress);
> > +COVERAGE_DEFINE(facet_create);
> > +COVERAGE_DEFINE(facet_remove);
> > +COVERAGE_DEFINE(subfacet_create);
> > +COVERAGE_DEFINE(subfacet_destroy);
> > COVERAGE_DEFINE(subfacet_install_fail);
> > COVERAGE_DEFINE(packet_in_overflow);
> > COVERAGE_DEFINE(flow_mod_overflow);
> > @@ -437,20 +441,6 @@ struct dpif_backer {
> > unsigned avg_n_subfacet; /* Average number of flows. */
> > long long int avg_subfacet_life; /* Average life span of subfacets.
> */
> >
> > - /* The average number of subfacets... */
> > - struct avg_subfacet_rates hourly; /* ...over the last hour. */
> > - struct avg_subfacet_rates daily; /* ...over the last day. */
> > - struct avg_subfacet_rates lifetime; /* ...over the switch lifetime.
> */
> > - long long int last_minute; /* Last time 'hourly' was
> updated. */
> > -
> > - /* Number of subfacets added or deleted since 'last_minute'. */
> > - unsigned subfacet_add_count;
> > - unsigned subfacet_del_count;
> > -
> > - /* Number of subfacets added or deleted from 'created' to
> 'last_minute.' */
> > - unsigned long long int total_subfacet_add_count;
> > - unsigned long long int total_subfacet_del_count;
> > -
> > /* Number of upcall handling threads. */
> > unsigned int n_handler_threads;
> > };
> > @@ -459,7 +449,6 @@ struct dpif_backer {
> > static struct shash all_dpif_backers =
> SHASH_INITIALIZER(&all_dpif_backers);
> >
> > static void drop_key_clear(struct dpif_backer *);
> > -static void update_moving_averages(struct dpif_backer *backer);
> >
> > struct ofproto_dpif {
> > struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'.
> */
> > @@ -1217,14 +1206,6 @@ open_dpif_backer(const char *type, struct
> dpif_backer **backerp)
> >
> > backer->max_n_subfacet = 0;
> > backer->created = time_msec();
> > - backer->last_minute = backer->created;
> > - memset(&backer->hourly, 0, sizeof backer->hourly);
> > - memset(&backer->daily, 0, sizeof backer->daily);
> > - memset(&backer->lifetime, 0, sizeof backer->lifetime);
> > - backer->subfacet_add_count = 0;
> > - backer->subfacet_del_count = 0;
> > - backer->total_subfacet_add_count = 0;
> > - backer->total_subfacet_del_count = 0;
> > backer->avg_n_subfacet = 0;
> > backer->avg_subfacet_life = 0;
> >
> > @@ -3739,8 +3720,6 @@ update_stats(struct dpif_backer *backer)
> > run_fast_rl();
> > }
> > dpif_flow_dump_done(&dump);
> > -
> > - update_moving_averages(backer);
> > }
> >
> > /* Calculates and returns the number of milliseconds of idle time after
> which
> > @@ -3923,6 +3902,7 @@ facet_create(const struct flow_miss *miss)
> > struct facet *facet;
> > struct match match;
> >
> > + COVERAGE_INC(facet_create);
> > facet = xzalloc(sizeof *facet);
> > facet->ofproto = miss->ofproto;
> > facet->used = miss->stats.used;
> > @@ -3986,6 +3966,7 @@ facet_remove(struct facet *facet)
> > {
> > struct subfacet *subfacet, *next_subfacet;
> >
> > + COVERAGE_INC(facet_remove);
> > ovs_assert(!list_is_empty(&facet->subfacets));
> >
> > /* First uninstall all of the subfacets to get final statistics. */
> > @@ -4523,6 +4504,7 @@ subfacet_create(struct facet *facet, struct
> flow_miss *miss)
> > subfacet = xmalloc(sizeof *subfacet);
> > }
> >
> > + COVERAGE_INC(subfacet_create);
> > hmap_insert(&backer->subfacets, &subfacet->hmap_node, key_hash);
> > list_push_back(&facet->subfacets, &subfacet->list_node);
> > subfacet->facet = facet;
> > @@ -4535,7 +4517,6 @@ subfacet_create(struct facet *facet, struct
> flow_miss *miss)
> > subfacet->path = SF_NOT_INSTALLED;
> > subfacet->backer = backer;
> >
> > - backer->subfacet_add_count++;
> > return subfacet;
> > }
> >
> > @@ -4545,11 +4526,8 @@ static void
> > subfacet_destroy__(struct subfacet *subfacet)
> > {
> > struct facet *facet = subfacet->facet;
> > - struct ofproto_dpif *ofproto = facet->ofproto;
> > -
> > - /* Update ofproto stats before uninstall the subfacet. */
> > - ofproto->backer->subfacet_del_count++;
> >
> > + COVERAGE_INC(subfacet_destroy);
> > subfacet_uninstall(subfacet);
> > hmap_remove(&subfacet->backer->subfacets, &subfacet->hmap_node);
> > list_remove(&subfacet->list_node);
> > @@ -5637,21 +5615,12 @@ ofproto_unixctl_dpif_dump_dps(struct
> unixctl_conn *conn, int argc OVS_UNUSED,
> > }
> >
> > static void
> > -show_dp_rates(struct ds *ds, const char *heading,
> > - const struct avg_subfacet_rates *rates)
> > -{
> > - ds_put_format(ds, "%s add rate: %5.3f/min, del rate: %5.3f/min\n",
> > - heading, rates->add_rate, rates->del_rate);
> > -}
> > -
> > -static void
> > dpif_show_backer(const struct dpif_backer *backer, struct ds *ds)
> > {
> > const struct shash_node **ofprotos;
> > struct ofproto_dpif *ofproto;
> > struct shash ofproto_shash;
> > uint64_t n_hit, n_missed;
> > - long long int minutes;
> > size_t i;
> >
> > n_hit = n_missed = 0;
> > @@ -5669,15 +5638,6 @@ dpif_show_backer(const struct dpif_backer
> *backer, struct ds *ds)
> > backer->avg_n_subfacet, backer->max_n_subfacet,
> > backer->avg_subfacet_life);
> >
> > - minutes = (time_msec() - backer->created) / (1000 * 60);
> > - if (minutes >= 60) {
> > - show_dp_rates(ds, "\thourly avg:", &backer->hourly);
> > - }
> > - if (minutes >= 60 * 24) {
> > - show_dp_rates(ds, "\tdaily avg:", &backer->daily);
> > - }
> > - show_dp_rates(ds, "\toverall avg:", &backer->lifetime);
> > -
> > shash_init(&ofproto_shash);
> > ofprotos = get_ofprotos(&ofproto_shash);
> > for (i = 0; i < shash_count(&ofproto_shash); i++) {
> > @@ -6230,51 +6190,6 @@ odp_port_to_ofp_port(const struct ofproto_dpif
> *ofproto, odp_port_t odp_port)
> > }
> > }
> >
> > -/* Compute exponentially weighted moving average, adding 'new' as the
> newest,
> > - * most heavily weighted element. 'base' designates the rate of decay:
> after
> > - * 'base' further updates, 'new''s weight in the EWMA decays to about
> 1/e
> > - * (about .37). */
> > -static void
> > -exp_mavg(double *avg, int base, double new)
> > -{
> > - *avg = (*avg * (base - 1) + new) / base;
> > -}
> > -
> > -static void
> > -update_moving_averages(struct dpif_backer *backer)
> > -{
> > - const int min_ms = 60 * 1000; /* milliseconds in one minute. */
> > - long long int minutes = (time_msec() - backer->created) / min_ms;
> > -
> > - if (minutes > 0) {
> > - backer->lifetime.add_rate = (double)
> backer->total_subfacet_add_count
> > - / minutes;
> > - backer->lifetime.del_rate = (double)
> backer->total_subfacet_del_count
> > - / minutes;
> > - } else {
> > - backer->lifetime.add_rate = 0.0;
> > - backer->lifetime.del_rate = 0.0;
> > - }
> > -
> > - /* Update hourly averages on the minute boundaries. */
> > - if (time_msec() - backer->last_minute >= min_ms) {
> > - exp_mavg(&backer->hourly.add_rate, 60,
> backer->subfacet_add_count);
> > - exp_mavg(&backer->hourly.del_rate, 60,
> backer->subfacet_del_count);
> > -
> > - /* Update daily averages on the hour boundaries. */
> > - if ((backer->last_minute - backer->created) / min_ms % 60 ==
> 59) {
> > - exp_mavg(&backer->daily.add_rate, 24,
> backer->hourly.add_rate);
> > - exp_mavg(&backer->daily.del_rate, 24,
> backer->hourly.del_rate);
> > - }
> > -
> > - backer->total_subfacet_add_count += backer->subfacet_add_count;
> > - backer->total_subfacet_del_count += backer->subfacet_del_count;
> > - backer->subfacet_add_count = 0;
> > - backer->subfacet_del_count = 0;
> > - backer->last_minute += min_ms;
> > - }
> > -}
> > -
> > const struct ofproto_class ofproto_dpif_class = {
> > init,
> > enumerate_types,
> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> > index 0a63299..0a1a2ef 100644
> > --- a/tests/ofproto-dpif.at
> > +++ b/tests/ofproto-dpif.at
> > @@ -2108,7 +2108,6 @@ ADD_OF_PORTS([br1], [3])
> > AT_CHECK([ovs-appctl dpif/show], [0], [dnl
> > dummy@ovs-dummy: hit:0 missed:0
> > flows: cur: 0, avg: 0, max: 0, life span: 0ms
> > - overall avg: add rate: 0.000/min, del rate: 0.000/min
> > br0: hit:0 missed:0
> > br0 65534/100: (dummy)
> > p1 1/1: (dummy)
> > @@ -2201,7 +2200,6 @@ warped
> > AT_CHECK([ovs-appctl dpif/show], [0], [dnl
> > dummy@ovs-dummy: hit:13 missed:2
> > flows: cur: 2, avg: 1, max: 2, life span: 1250ms
> > - overall avg: add rate: 0.000/min, del rate: 0.000/min
> > br0: hit:9 missed:1
> > br0 65534/100: (dummy)
> > p2 2/2: (dummy)
> > @@ -2253,8 +2251,6 @@ AT_CHECK([ovs-appctl time/warp 10000], [0], [warped
> > AT_CHECK([ovs-appctl dpif/show | sed 's/ 10[[0-9]]\{3\}(ms)$/
> 10000(ms)/'], [0], [dnl
> > dummy@ovs-dummy: hit:0 missed:61
> > flows: cur: 0, avg: 0, max: 1, life span: 1666ms
> > - hourly avg: add rate: 0.641/min, del rate: 0.641/min
> > - overall avg: add rate: 1.000/min, del rate: 1.000/min
> > br0: hit:0 missed:61
> > br0 65534/100: (dummy)
> > p1 1/1: (dummy)
> > diff --git a/tests/tunnel.at b/tests/tunnel.at
> > index 697c217..982d22a 100644
> > --- a/tests/tunnel.at
> > +++ b/tests/tunnel.at
> > @@ -14,7 +14,7 @@ actions=IN_PORT
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: remote_ip=1.1.1.1)
> > p2 2/1: (gre: local_ip=2.2.2.2, remote_ip=1.1.1.1)
> > @@ -37,7 +37,7 @@ dnl reconfigure, local_ip, remote_ip
> > AT_CHECK([ovs-vsctl set Interface p2 type=gre options:local_ip=2.2.2.3 \
> > options:df_default=false options:ttl=1 options:csum=true \
> > -- set Interface p3 type=gre64])
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: remote_ip=1.1.1.1)
> > p2 2/1: (gre: csum=true, df_default=false,
> local_ip=2.2.2.3, remote_ip=1.1.1.1, ttl=1)
> > @@ -72,7 +72,7 @@ actions=2
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: remote_ip=1.1.1.1)
> > p2 2/2: (dummy)
> > @@ -116,7 +116,7 @@ actions=output:1
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: key=5, local_ip=2.2.2.2, remote_ip=1.1.1.1)
> > p2 2/2: (dummy)
> > @@ -148,7 +148,7 @@ actions=output:1
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: remote_ip=1.1.1.1, tos=inherit,
> ttl=inherit)
> > p2 2/2: (dummy)
> > @@ -190,7 +190,7 @@
> actions=set_tunnel:1,output:1,set_tunnel:2,output:2,set_tunnel:3,output:3,set_tu
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: key=flow, remote_ip=1.1.1.1)
> > p2 2/1: (gre: key=flow, remote_ip=2.2.2.2)
> > @@ -222,7 +222,7 @@ actions=IN_PORT,output:1,output:2,output:3
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: key=1, remote_ip=1.1.1.1)
> > p2 2/1: (gre: in_key=2, out_key=3, remote_ip=1.1.1.1)
> > @@ -274,7 +274,7 @@ tun_id=4,actions=output:5
> >
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (gre: key=flow, remote_ip=1.1.1.1)
> > p2 2/1: (gre: key=3, remote_ip=3.3.3.3)
> > @@ -310,7 +310,7 @@ AT_SETUP([tunnel - VXLAN])
> > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
> > options:remote_ip=1.1.1.1 ofport_request=1])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (vxlan: remote_ip=1.1.1.1)
> > ])
> > @@ -322,7 +322,7 @@ AT_SETUP([tunnel - LISP])
> > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=lisp \
> > options:remote_ip=1.1.1.1 ofport_request=1])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (lisp: remote_ip=1.1.1.1)
> > ])
> > @@ -334,7 +334,7 @@ AT_SETUP([tunnel - different VXLAN UDP port])
> > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
> > options:remote_ip=1.1.1.1 ofport_request=1
> options:dst_port=4341])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (vxlan: dst_port=4341, remote_ip=1.1.1.1)
> > ])
> > @@ -343,7 +343,7 @@ dnl change UDP port
> >
> > AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=5000])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/2: (vxlan: dst_port=5000, remote_ip=1.1.1.1)
> > ])
> > @@ -352,7 +352,7 @@ dnl change UDP port to default
> >
> > AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=4789])
> >
> > -AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
> > +AT_CHECK([ovs-appctl dpif/show | tail -n +4], [0], [dnl
> > br0 65534/100: (dummy)
> > p1 1/1: (vxlan: remote_ip=1.1.1.1)
> > ])
> > --
> > 1.7.9.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
