From 735d7efbaf03fd769d4716b7785fd81cc70f50d9 Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Tue, 12 Mar 2013 14:19:18 -0700 Subject: [PATCH] ovs-appctl: dpif/show display per bridge stats This is to fix the fallout of single datapath change. ovs-appctl dpif/show displays per bridge miss, hit and flow counts on the screen, but the backend is obtaining those information from the datapath. With a single datapath, all bridges of the same datapath would all display the same (global) counters maintained by the datapath, obviously not correct. This patch fixes the bug by maintaining per ofproto_dpif miss and hit counts, which are used for display output. The number of flows count is obtained by counting the number facets per ofproto. ovs-dpctl show still displays the counters maintain by the datapath, as before. Bug #15369 Signed-off-by: Andy Zhou Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif.c | 38 +++++++++++++++++++++++++++++++------- tests/ofproto-dpif.at | 14 +++++++++----- tests/tunnel.at | 26 +++++++++++++------------- 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 74000020a..421e9d4b2 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -665,6 +665,9 @@ static void drop_key_clear(struct dpif_backer *); static struct ofport_dpif * odp_port_to_ofport(const struct dpif_backer *, uint32_t odp_port); +static void dpif_stats_update_hit_count(struct ofproto_dpif *ofproto, + uint64_t delta); + struct ofproto_dpif { struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */ struct ofproto up; @@ -714,6 +717,10 @@ struct ofproto_dpif { struct sset ghost_ports; /* Ports with no datapath port. */ struct sset port_poll_set; /* Queued names for port_poll() reply. */ int port_poll_errno; /* Last errno for port_poll() reply. */ + + /* Per ofproto's dpif stats. */ + uint64_t n_hit; + uint64_t n_missed; }; /* Defer flow mod completion until "ovs-appctl ofproto/unclog"? (Useful only @@ -1291,6 +1298,9 @@ construct(struct ofproto *ofproto_) error = add_internal_flows(ofproto); ofproto->up.tables[TBL_INTERNAL].flags = OFTABLE_HIDDEN | OFTABLE_READONLY; + ofproto->n_hit = 0; + ofproto->n_missed = 0; + return error; } @@ -3820,6 +3830,8 @@ handle_miss_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls, if (error) { continue; } + + ofproto->n_missed++; flow_extract(upcall->packet, flow.skb_priority, flow.skb_mark, &flow.tunnel, flow.in_port, &miss->flow); @@ -4107,6 +4119,11 @@ delete_unexpected_flow(struct ofproto_dpif *ofproto, * avoided by calling update_stats() whenever rules are created or * deleted. However, the performance impact of making so many calls to the * datapath do not justify the benefit of having perfectly accurate statistics. + * + * In addition, this function maintains per ofproto flow hit counts. The patch + * port is not treated specially. e.g. A packet ingress from br0 patched into + * br1 will increase the hit count of br0 by 1, however, does not affect + * the hit or miss counts of br1. */ static void update_stats(struct dpif_backer *backer) @@ -4138,6 +4155,12 @@ update_stats(struct dpif_backer *backer) subfacet = subfacet_find(ofproto, key, key_len, key_hash); switch (subfacet ? subfacet->path : SF_NOT_INSTALLED) { case SF_FAST_PATH: + /* Update ofproto_dpif's hit count. */ + if (stats->n_packets > subfacet->dp_packet_count) { + uint64_t delta = stats->n_packets - subfacet->dp_packet_count; + dpif_stats_update_hit_count(ofproto, delta); + } + update_subfacet_stats(subfacet, stats); break; @@ -8040,19 +8063,14 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn *conn, int argc OVS_UNUSED, static void show_dp_format(const struct ofproto_dpif *ofproto, struct ds *ds) { - struct dpif_dp_stats s; const struct shash_node **ports; int i; - dpif_get_dp_stats(ofproto->backer->dpif, &s); - ds_put_format(ds, "%s (%s):\n", ofproto->up.name, dpif_name(ofproto->backer->dpif)); - /* xxx It would be better to show bridge-specific stats instead - * xxx of dp ones. */ ds_put_format(ds, - "\tlookups: hit:%"PRIu64" missed:%"PRIu64" lost:%"PRIu64"\n", - s.n_hit, s.n_missed, s.n_lost); + "\tlookups: hit:%"PRIu64" missed:%"PRIu64"\n", + ofproto->n_hit, ofproto->n_missed); ds_put_format(ds, "\tflows: %zu\n", hmap_count(&ofproto->subfacets)); @@ -8472,6 +8490,12 @@ odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, uint32_t odp_port) } } +static void +dpif_stats_update_hit_count(struct ofproto_dpif *ofproto, uint64_t delta) +{ + ofproto->n_hit += delta; +} + const struct ofproto_class ofproto_dpif_class = { init, enumerate_types, diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index a66018326..0a2a900a3 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1472,13 +1472,13 @@ ADD_OF_PORTS([br1], [3]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (dummy) p2 2/2: (dummy) br1 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br1 65534/101: (dummy) p3 3/3: (dummy) @@ -1486,7 +1486,7 @@ br1 (dummy@ovs-dummy): AT_CHECK([ovs-appctl dpif/show br0], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (dummy) @@ -1573,15 +1573,19 @@ for i in $(seq 1 5); do ovs-appctl netdev-dummy/receive br1 'in_port(101),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=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' done +AT_CHECK([ovs-appctl time/warp 1000 && ovs-appctl time/warp 1000], [0], [warped +warped +]) + AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:13 missed:2 lost:0 + lookups: hit:9 missed:1 flows: 1 br0 65534/100: (dummy) p2 2/2: (dummy) pbr0 1/none: (patch: peer=pbr1) br1 (dummy@ovs-dummy): - lookups: hit:13 missed:2 lost:0 + lookups: hit:4 missed:1 flows: 1 br1 65534/101: (dummy) p3 3/3: (dummy) diff --git a/tests/tunnel.at b/tests/tunnel.at index 24af06136..aee543eb7 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -16,7 +16,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: remote_ip=1.1.1.1) @@ -42,7 +42,7 @@ AT_CHECK([ovs-vsctl set Interface p2 type=gre options:local_ip=2.2.2.3 \ -- set Interface p3 type=gre64]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: remote_ip=1.1.1.1) @@ -80,7 +80,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: remote_ip=1.1.1.1) @@ -127,7 +127,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: key=5, local_ip=2.2.2.2, remote_ip=1.1.1.1) @@ -162,7 +162,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: remote_ip=1.1.1.1, tos=inherit, ttl=inherit) @@ -207,7 +207,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: key=flow, remote_ip=1.1.1.1) @@ -242,7 +242,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: key=1, remote_ip=1.1.1.1) @@ -297,7 +297,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (gre: key=flow, remote_ip=1.1.1.1) @@ -336,7 +336,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \ AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (vxlan: remote_ip=1.1.1.1) @@ -351,7 +351,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=lisp \ AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (lisp: remote_ip=1.1.1.1) @@ -366,7 +366,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \ AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (vxlan: dst_port=4341, remote_ip=1.1.1.1) @@ -378,7 +378,7 @@ AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=5000]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/2: (vxlan: dst_port=5000, remote_ip=1.1.1.1) @@ -390,7 +390,7 @@ AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=8472]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl br0 (dummy@ovs-dummy): - lookups: hit:0 missed:0 lost:0 + lookups: hit:0 missed:0 flows: 0 br0 65534/100: (dummy) p1 1/1: (vxlan: remote_ip=1.1.1.1) -- 2.43.0