Add per-table counters. This resolves some short-comings
in the data provided in a table stats reply message.
* Lookups and matches are calculated based on table
accesses rather than datapath flow hits and misses.
* Lookups and matches are credited to the table where they
occurred rather than all being credited to table 0.
These problems were observed when running make check-ryu
and this patch allows many of its tester.py match checks
to pass.
Signed-off-by: Simon Horman <ho...@verge.net.au>
Reviewed-by: YAMAMOTO Takashi <yamam...@valinux.co.jp>
--
v3
* Add: Reviewed-by: YAMAMOTO Takashi <yamam...@valinux.co.jp>
* Rebase
v2
* Use NULL instead of 0 as NULL pointer argument
* Rebase
---
ofproto/ofproto-dpif-xlate.c | 8 +-
ofproto/ofproto-dpif.c | 43 +++++-----
ofproto/ofproto-dpif.h | 5 +-
ofproto/ofproto-provider.h | 3 +
ofproto/ofproto.c | 2 +
tests/ofproto-dpif.at | 195 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 233 insertions(+), 23 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 248382f..b5fa165 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2057,7 +2057,9 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t
in_port, uint8_t table_id,
!skip_wildcards
? &ctx->xout->wc : NULL,
honor_table_miss,
- &ctx->table_id, &rule);
+ &ctx->table_id,
+ ctx->xin->resubmit_stats,
+ &rule);
ctx->xin->flow.in_port.ofp_port = old_in_port;
if (ctx->xin->resubmit_hook) {
@@ -2654,7 +2656,7 @@ xlate_learn_action(struct xlate_ctx *ctx,
entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
entry->u.learn.ofproto = ctx->xin->ofproto;
rule_dpif_lookup(ctx->xbridge->ofproto, &ctx->xin->flow, NULL,
- &entry->u.learn.rule);
+ NULL, &entry->u.learn.rule);
}
}
@@ -3263,7 +3265,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out
*xout)
if (!xin->ofpacts && !ctx.rule) {
ctx.table_id = rule_dpif_lookup(ctx.xbridge->ofproto, flow,
!xin->skip_wildcards ? wc : NULL,
- &rule);
+ ctx.xin->resubmit_stats, &rule);
if (ctx.xin->resubmit_stats) {
rule_dpif_credit_stats(rule, ctx.xin->resubmit_stats);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6a725e4..6200f57 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1515,25 +1515,20 @@ get_features(struct ofproto *ofproto_ OVS_UNUSED,
}
static void
-get_tables(struct ofproto *ofproto_, struct ofp12_table_stats *ots)
+get_tables(struct ofproto *ofproto, struct ofp12_table_stats *ots)
{
- struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
- struct dpif_dp_stats s;
- uint64_t n_miss, n_no_pkt_in, n_bytes, n_dropped_frags;
- uint64_t n_lookup;
- long long int used;
+ int i;
strcpy(ots->name, "classifier");
- dpif_get_dp_stats(ofproto->backer->dpif, &s);
- rule_get_stats(&ofproto->miss_rule->up, &n_miss, &n_bytes, &used);
- rule_get_stats(&ofproto->no_packet_in_rule->up, &n_no_pkt_in, &n_bytes,
- &used);
- rule_get_stats(&ofproto->drop_frags_rule->up, &n_dropped_frags, &n_bytes,
- &used);
- n_lookup = s.n_hit + s.n_missed - n_dropped_frags;
- ots->lookup_count = htonll(n_lookup);
- ots->matched_count = htonll(n_lookup - n_miss - n_no_pkt_in);
+ for (i = 0; i < ofproto->n_tables; i++) {
+ uint64_t missed, matched;
+
+ atomic_read(&ofproto->tables[i].n_matched, &matched);
+ ots[i].matched_count = htonll(matched);
+ atomic_read(&ofproto->tables[i].n_missed, &missed);
+ ots[i].lookup_count = htonll(matched + missed);
+ }
}
static struct ofport *
@@ -3192,7 +3187,9 @@ rule_dpif_get_actions(const struct rule_dpif *rule)
* where misses occur. */
uint8_t
rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
- struct flow_wildcards *wc, struct rule_dpif **rule)
+ struct flow_wildcards *wc,
+ const struct dpif_flow_stats *stats,
+ struct rule_dpif **rule)
{
enum rule_dpif_lookup_verdict verdict;
enum ofputil_port_config config = 0;
@@ -3212,7 +3209,7 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct
flow *flow,
}
verdict = rule_dpif_lookup_from_table(ofproto, flow, wc, true,
- &table_id, rule);
+ &table_id, stats, rule);
switch (verdict) {
case RULE_DPIF_LOOKUP_VERDICT_MATCH:
@@ -3322,7 +3319,9 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
const struct flow *flow,
struct flow_wildcards *wc,
bool honor_table_miss,
- uint8_t *table_id, struct rule_dpif **rule)
+ uint8_t *table_id,
+ const struct dpif_flow_stats *stats,
+ struct rule_dpif **rule)
{
uint8_t next_id;
@@ -3332,6 +3331,12 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
{
*table_id = next_id;
*rule = rule_dpif_lookup_in_table(ofproto, *table_id, flow, wc);
+ if (stats) {
+ struct oftable *tbl = &ofproto->up.tables[next_id];
+ atomic_uint64_t *stat = *rule ? &tbl->n_matched : &tbl->n_missed;
+ uint64_t orig;
+ atomic_add(stat, stats->n_packets, &orig);
+ }
if (*rule) {
return RULE_DPIF_LOOKUP_VERDICT_MATCH;
} else if (!honor_table_miss) {
@@ -4190,7 +4195,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow
*flow,
if (ofpacts) {
rule = NULL;
} else {
- rule_dpif_lookup(ofproto, flow, &trace.wc, &rule);
+ rule_dpif_lookup(ofproto, flow, &trace.wc, NULL, &rule);
trace_format_rule(ds, 0, rule);
if (rule == ofproto->miss_rule) {
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index eb4787c..8bffe83 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -87,13 +87,16 @@ size_t ofproto_dpif_get_max_mpls_depth(const struct
ofproto_dpif *);
bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *);
uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
- struct flow_wildcards *, struct rule_dpif **rule);
+ struct flow_wildcards *,
+ const struct dpif_flow_stats *,
+ struct rule_dpif **rule);
enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif
*,
const struct flow *,
struct
flow_wildcards *,
bool
force_controller_on_miss,
uint8_t *table_id,
+ const struct
dpif_flow_stats *,
struct rule_dpif
**rule);
void rule_dpif_ref(struct rule_dpif *);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 0148fe6..f7c7b72 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -266,6 +266,9 @@ struct oftable {
/* Table config: contains enum ofproto_table_config; accessed atomically.
*/
atomic_uint config;
+
+ atomic_uint64_t n_matched;
+ atomic_uint64_t n_missed;
};
/* Assigns TABLE to each oftable, in turn, in OFPROTO.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f16005c..803f4ac 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6764,6 +6764,8 @@ oftable_init(struct oftable *table)
classifier_init(&table->cls, flow_segment_u32s);
table->max_flows = UINT_MAX;
atomic_init(&table->config, (unsigned int)OFPROTO_TABLE_MISS_DEFAULT);
+ atomic_init(&table->n_matched, 0);
+ atomic_init(&table->n_missed, 0);
}
/* Destroys 'table', including its classifier and eviction groups.
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c5a01a1..a6bfa1a 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4619,3 +4619,198 @@ AT_CHECK([grep -c 'resubmits yielded over 64 kB of
stack' ovs-vswitchd.log], [0]
])
OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of stack/d"])
AT_CLEANUP
+
+
+AT_SETUP([ofproto-if packet-out controller])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], 1, 2)
+
+AT_CHECK([ovs-ofctl add-flow br0 'dl_dst=50:54:00:00:00:0a
actions=controller'])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir
--pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 CONTROLLER controller
'50540000000a5054000000091234'])
+done
+
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): total_len=14 in_port=CONTROLLER (via action)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): total_len=14 in_port=CONTROLLER (via action)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): total_len=14 in_port=CONTROLLER (via action)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ dl_dst=50:54:00:00:00:0a actions=CONTROLLER:65535
+NXST_FLOW reply:
+])
+
+(echo "OFPST_TABLE reply (OF1.3) (xid=0x2): 254 tables"
+ echo " 0: active=1, lookup=0, matched=0"
+ x=1
+ while test $x -lt 254; do
+ echo " $x: active=0, lookup=0, matched=0"
+ x=`expr $x + 1`
+ done) > expout
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto-if packet-out controller (patch port)])
+OVS_VSWITCHD_START(
+ [-- \
+ add-port br0 p1 -- \
+ set interface p1 type=patch options:peer=p2 -- \
+ add-br br1 -- \
+ set bridge br1 datapath-type=dummy -- \
+ set bridge br1 fail-mode=secure -- \
+ set bridge br1 protocols='[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13]' --
\
+ add-port br1 p2 -- \
+ set interface p2 type=patch options:peer=p1 --])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_CHECK([ovs-ofctl monitor br1 65534 invalid_ttl --detach --no-chdir
--pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 CONTROLLER output:1
'50540000000a5054000000091234'])
+done
+
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via no_match)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via no_match)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via no_match)
data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+])
+
+(echo "OFPST_TABLE reply (OF1.3) (xid=0x2): 254 tables"
+ x=0
+ while test $x -lt 254; do
+ echo " $x: active=0, lookup=0, matched=0"
+ x=`expr $x + 1`
+ done) > expout
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+(echo "OFPST_TABLE reply (OF1.3) (xid=0x2): 254 tables"
+ echo " 0: active=0, lookup=3, matched=0"
+ x=1
+ while test $x -lt 254; do
+ echo " $x: active=0, lookup=0, matched=0"
+ x=`expr $x + 1`
+ done) > expout
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br1 ], [0], [expout])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto-if packet-out goto_table])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], 1, 2)
+
+AT_DATA([flows.txt], [dnl
+table=0 dl_dst=50:54:00:00:00:0a actions=goto_table(1)
+table=1 dl_dst=50:54:00:00:00:0a actions=controller
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows.txt])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir
--pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)'
+done
+
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip | sort], [0],
[dnl
+ n_packets=3, n_bytes=180, dl_dst=50:54:00:00:00:0a actions=goto_table:1
+ table=1, n_packets=3, n_bytes=180, dl_dst=50:54:00:00:00:0a
actions=CONTROLLER:65535
+OFPST_FLOW reply (OF1.3):
+])
+
+(echo "OFPST_TABLE reply (OF1.3) (xid=0x2): 254 tables"
+ echo " 0: active=1, lookup=3, matched=3"
+ echo " 1: active=1, lookup=3, matched=3"
+ x=2
+ while test $x -lt 254; do
+ echo " $x: active=0, lookup=0, matched=0"
+ x=`expr $x + 1`
+ done) > expout
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto-if packet-out table-miss (continue)])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], 1, 2)
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'table=1
dl_dst=50:54:00:00:00:0a actions=controller'])
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 all continue])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir
--pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)'
+done
+
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+dnl
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=60 in_port=1 (via
action) data_len=60 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip | sort], [0],
[dnl
+ table=1, n_packets=3, n_bytes=180, dl_dst=50:54:00:00:00:0a
actions=CONTROLLER:65535
+OFPST_FLOW reply (OF1.3):
+])
+
+(echo "OFPST_TABLE reply (OF1.3) (xid=0x2): 254 tables"
+ echo " 0: active=0, lookup=3, matched=0"
+ echo " 1: active=1, lookup=3, matched=3"
+ x=2
+ while test $x -lt 254; do
+ echo " $x: active=0, lookup=0, matched=0"
+ x=`expr $x + 1`
+ done) > expout
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
--
1.8.5.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
