[ovs-dev] Statistics Collection Performance Impact
Simon Horman
horms at verge.net.au
Wed Dec 21 08:54:18 UTC 2011
On Mon, Dec 12, 2011 at 09:46:44AM -0800, Ben Pfaff wrote:
> On Sun, Dec 11, 2011 at 11:20:50AM +0900, Simon Horman wrote:
> > sorry for the long delay. I finally had a stab at implementing this idea
> > but as yet it does not seem to be working.
> >
> > My implementation simply keeps track of a static struct dpif_flow_dump dump
> > in update_stats() and resets it as necessary using dpif_flow_dump_start()
> > and dpif_flow_dump_done().
> >
> > The result seems to be that flows are expiring all over the place.
> > I assume this is because their stats are not being updated. Do I need
> > to keep track of more state in order to take into account that
> > the flow_table is changing between calls to update_stats()?
>
> That sounds like what would happen if, each time you run expiration,
> you consider all the flows in the kernel table, not just the flows
> that have actually been retrieved from the kernel just now. I guess
> that, each time you run expiration, you should only consider the flows
> that have actually been read from the kernel recently, and not
> consider other flows for eviction.
Hi Ben,
thanks for your advice. It seems that you are correct as the
patch below is working.
The patch is against a slightly old revision of the master branch.
I will rebase it if you think that I am on the right track.
The attached graph demonstrates the performance improvement
that this change gives.
Also, not shown in the graph, with this change I can now add more than
1,000,000 flows.
>From 492ebe71aa949b79bf3ccefc996ebed9d2c4ef96 Mon Sep 17 00:00:00 2001
From: Simon Horman <horms at verge.net.au>
Date: Wed, 21 Dec 2011 16:58:42 +0900
Subject: [PATCH] Implement partial stats update
As the number of flows grows so does the length of time taken to update the
statistics of all facets. It has been obvserved that this becomes a performance
problem. This patch mitiages this effect by only dumping a limited number of
facets at a time.
---
The number of maximum number of facets dumped at a time was chosen as 100,000
as it is somewhere well below 1,000,000 which is rougly the point at which I am
unable to add any new flows in my environment without this change.
---
ofproto/ofproto-dpif.c | 27 ++++++++++++++++++++++-----
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b296154..ae426a8 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -247,6 +247,7 @@ struct facet {
struct hmap_node hmap_node; /* In owning ofproto's 'facets' hmap. */
struct list list_node; /* In owning rule's 'facets' list. */
+ struct list may_expire; /* Potential expiry list. */
struct rule_dpif *rule; /* Owning rule. */
struct flow flow; /* Exact-match flow. */
bool installed; /* Installed in datapath? */
@@ -371,6 +372,7 @@ struct ofproto_dpif {
/* Facets. */
struct hmap facets;
+ struct list expirable_facets;
/* Revalidation. */
struct table_dpif tables[N_TABLES];
@@ -517,6 +519,7 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
timer_set_duration(&ofproto->next_expiration, 1000);
hmap_init(&ofproto->facets);
+ list_init(&ofproto->expirable_facets);
for (i = 0; i < N_TABLES; i++) {
struct table_dpif *table = &ofproto->tables[i];
@@ -2427,12 +2430,18 @@ static void
update_stats(struct ofproto_dpif *p)
{
const struct dpif_flow_stats *stats;
- struct dpif_flow_dump dump;
+ static struct dpif_flow_dump dump;
const struct nlattr *key;
size_t key_len;
+ static bool complete = true;
+ int count = 100000;
- dpif_flow_dump_start(&dump, p->dpif);
- while (dpif_flow_dump_next(&dump, &key, &key_len, NULL, NULL, &stats)) {
+ if (complete) {
+ dpif_flow_dump_start(&dump, p->dpif);
+ complete = false;
+ }
+ while (--count &&
+ dpif_flow_dump_next(&dump, &key, &key_len, NULL, NULL, &stats)) {
struct facet *facet;
struct flow flow;
@@ -2467,6 +2476,7 @@ update_stats(struct ofproto_dpif *p)
facet->dp_packet_count = stats->n_packets;
facet->dp_byte_count = stats->n_bytes;
+ list_insert(&p->expirable_facets, &facet->may_expire);
facet_update_time(p, facet, stats->used);
facet_account(p, facet);
facet_push_stats(facet);
@@ -2477,7 +2487,10 @@ update_stats(struct ofproto_dpif *p)
dpif_flow_del(p->dpif, key, key_len, NULL);
}
}
- dpif_flow_dump_done(&dump);
+ if (count) {
+ dpif_flow_dump_done(&dump);
+ complete = true;
+ }
}
/* Calculates and returns the number of milliseconds of idle time after which
@@ -2595,11 +2608,13 @@ expire_facets(struct ofproto_dpif *ofproto, int dp_max_idle)
long long int cutoff = time_msec() - dp_max_idle;
struct facet *facet, *next_facet;
- HMAP_FOR_EACH_SAFE (facet, next_facet, hmap_node, &ofproto->facets) {
+ LIST_FOR_EACH_SAFE (facet, next_facet, may_expire,
+ &ofproto->expirable_facets) {
facet_active_timeout(ofproto, facet);
if (facet->used < cutoff) {
facet_remove(ofproto, facet);
}
+ list_remove(&facet->may_expire);
}
}
@@ -2657,6 +2672,7 @@ facet_create(struct rule_dpif *rule, const struct flow *flow)
facet->used = time_msec();
hmap_insert(&ofproto->facets, &facet->hmap_node, flow_hash(flow, 0));
list_push_back(&rule->facets, &facet->list_node);
+ list_init(&facet->may_expire);
facet->rule = rule;
facet->flow = *flow;
netflow_flow_init(&facet->nf_flow);
@@ -2769,6 +2785,7 @@ facet_remove(struct ofproto_dpif *ofproto, struct facet *facet)
facet_flush_stats(ofproto, facet);
hmap_remove(&ofproto->facets, &facet->hmap_node);
list_remove(&facet->list_node);
+ list_remove(&facet->may_expire);
facet_free(facet);
}
--
1.7.4.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flow.pdf
Type: application/pdf
Size: 47222 bytes
Desc: not available
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20111221/834934f2/attachment-0004.pdf>
More information about the dev
mailing list