[ovs-discuss] patch
Ben Pfaff
blp at nicira.com
Fri Jan 30 22:29:17 UTC 2015
On Fri, Jan 30, 2015 at 02:22:55PM -0800, Ashok Chippa wrote:
> Would it be possible to get a patch for the bug I reported, where only one
> flow is being shown (with ovs-ofctl dump-flows...) when there are actually
> more than 1 flows?
The patch is on master. I'll append it to this email also.
> Is there a cli command to clear flows in a table? ovs-ofctl del-flow
> <switch> does not work!!
That should work so what do you mean?
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <blp at nicira.com>
Date: Tue, 6 Jan 2015 09:27:32 -0800
Subject: [PATCH] ofproto: Don't count hidden rules in table stats.
The hidden rules created by in-band control and fail-open should not be
included in the table stats reported via OpenFlow. I seem to recall that
this was done correctly in some previous version but it has broken since
then. This commit fixes the problem and adds a test that should make it
harder to break again in the future.
Reported-by: Ashok Chippa <a.n.chippa at gmail.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
ofproto/connmgr.c | 19 ++++++++++++++++++-
ofproto/connmgr.h | 4 +++-
ofproto/fail-open.c | 13 ++++++++++++-
ofproto/fail-open.h | 4 +++-
ofproto/in-band.c | 10 +++++++++-
ofproto/in-band.h | 4 +++-
ofproto/ofproto.c | 6 +++++-
tests/ofproto.at | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 105 insertions(+), 7 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 08abe1e..3d69122 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1980,6 +1980,23 @@ connmgr_flushed(struct connmgr *mgr)
ofpbuf_uninit(&ofpacts);
}
}
+
+/* Returns the number of hidden rules created by the in-band and fail-open
+ * implementations in table 0. (Subtracting this count from the number of
+ * rules in the table 0 classifier, as returned by classifier_count(), yields
+ * the number of flows that OVS should report via OpenFlow for table 0.) */
+int
+connmgr_count_hidden_rules(const struct connmgr *mgr)
+{
+ int n_hidden = 0;
+ if (mgr->in_band) {
+ n_hidden += in_band_count_rules(mgr->in_band);
+ }
+ if (mgr->fail_open) {
+ n_hidden += fail_open_count_rules(mgr->fail_open);
+ }
+ return n_hidden;
+}
/* Creates a new ofservice for 'target' in 'mgr'. Returns 0 if successful,
* otherwise a positive errno value.
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index c0f7c35..dd1a027 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -193,6 +193,8 @@ bool connmgr_has_in_band(struct connmgr *);
/* Fail-open and in-band implementation. */
void connmgr_flushed(struct connmgr *);
+int connmgr_count_hidden_rules(const struct connmgr *);
+
/* A flow monitor managed by NXST_FLOW_MONITOR and related requests. */
struct ofmonitor {
struct ofconn *ofconn; /* Owning 'ofconn'. */
diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
index ecdba44..75c7d1c 100644
--- a/ofproto/fail-open.c
+++ b/ofproto/fail-open.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -76,6 +76,7 @@ struct fail_open {
int last_disconn_secs;
long long int next_bogus_packet_in;
struct rconn_packet_counter *bogus_packet_counter;
+ bool fail_open_active;
};
static void fail_open_recover(struct fail_open *);
@@ -234,6 +235,15 @@ fail_open_flushed(struct fail_open *fo)
ofpbuf_uninit(&ofpacts);
}
+ fo->fail_open_active = open;
+}
+
+/* Returns the number of fail-open rules currently installed in the flow
+ * table. */
+int
+fail_open_count_rules(const struct fail_open *fo)
+{
+ return fo->fail_open_active != 0;
}
/* Creates and returns a new struct fail_open for 'ofproto' and 'mgr'. */
@@ -246,6 +256,7 @@ fail_open_create(struct ofproto *ofproto, struct connmgr *mgr)
fo->last_disconn_secs = 0;
fo->next_bogus_packet_in = LLONG_MAX;
fo->bogus_packet_counter = rconn_packet_counter_create();
+ fo->fail_open_active = false;
return fo;
}
diff --git a/ofproto/fail-open.h b/ofproto/fail-open.h
index 725b82d..4056b3e 100644
--- a/ofproto/fail-open.h
+++ b/ofproto/fail-open.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -47,4 +47,6 @@ void fail_open_run(struct fail_open *);
void fail_open_maybe_recover(struct fail_open *) OVS_EXCLUDED(ofproto_mutex);
void fail_open_flushed(struct fail_open *) OVS_EXCLUDED(ofproto_mutex);
+int fail_open_count_rules(const struct fail_open *);
+
#endif /* fail-open.h */
diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index bde893a..01950fa 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -235,6 +235,14 @@ in_band_must_output_to_local_port(const struct flow *flow)
&& flow->tp_dst == htons(DHCP_CLIENT_PORT));
}
+/* Returns the number of in-band rules currently installed in the flow
+ * table. */
+int
+in_band_count_rules(const struct in_band *ib)
+{
+ return hmap_count(&ib->rules);
+}
+
static void
add_rule(struct in_band *ib, const struct match *match, int priority)
{
diff --git a/ofproto/in-band.h b/ofproto/in-band.h
index ad16dc2..6e0585a 100644
--- a/ofproto/in-band.h
+++ b/ofproto/in-band.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -42,4 +42,6 @@ void in_band_wait(struct in_band *);
bool in_band_must_output_to_local_port(const struct flow *);
+int in_band_count_rules(const struct in_band *);
+
#endif /* in-band.h */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 6d78fe4..34defce 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009-2015 Nicira, Inc.
* Copyright (c) 2010 Jean Tourrilhes - HP-Labs.
*
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -2963,6 +2963,10 @@ query_tables(struct ofproto *ofproto,
s->table_id = i;
s->active_count = classifier_count(cls);
+ if (i == 0) {
+ s->active_count -= connmgr_count_hidden_rules(
+ ofproto->connmgr);
+ }
}
} else {
stats = NULL;
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 8f4e25e..2f5a9dd 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1306,6 +1306,58 @@ AT_CHECK([ovs-ofctl dump-tables br0], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
+dnl In-band and fail-open add "hidden rules" to table 0. These rules shouldn't
+dnl be visible to OpenFlow. This test checks that "dump-flows" and
+dnl "dump-tables" don't make them visible.
+AT_SETUP([ofproto - hidden rules not in table stats])
+# Use an IP address for a controller that won't actually exist: we
+# want to create in-band rules but we do not want to actually connect
+# to a controller (because that could mess about with our test). The
+# Class E range 240.0.0.0 - 255.255.255.255 seems like a good choice.
+OVS_VSWITCHD_START([set-controller br0 tcp:240.0.0.1:6653])
+for i in 1 2 3 4 5; do ovs-appctl time/warp 1000; done
+
+# Check that no hidden flows are visible in OpenFlow.
+AT_CHECK([ovs-ofctl dump-flows br0], [0], [NXST_FLOW reply (xid=0x4):
+])
+
+# Check that some hidden flows related to 240.0.0.1 are actually in table 0.
+#
+# We discard flows that mention table_id because we only want table 0 flows,
+# which in OVS is implied by the absence of a table_id.
+AT_CHECK([ovs-appctl bridge/dump-flows br0], [0], [stdout])
+AT_CHECK([test `grep '240\.0\.0\.1' stdout | grep -v table_id= | wc -l` -gt 0])
+
+# Check that dump-tables doesn't count the hidden flows.
+(printf "OFPST_TABLE reply (xid=0x2):"
+ x=0
+ name=classifier
+ while test $x -lt 254; do
+ printf "
+ table %d (\"%s\"):
+ active=0, lookup=0, matched=0
+ max_entries=1000000
+ matching:
+ in_port: exact match or wildcard
+ eth_src: exact match or wildcard
+ eth_dst: exact match or wildcard
+ eth_type: exact match or wildcard
+ vlan_vid: exact match or wildcard
+ vlan_pcp: exact match or wildcard
+ ip_src: exact match or wildcard
+ ip_dst: exact match or wildcard
+ nw_proto: exact match or wildcard
+ nw_tos: exact match or wildcard
+ tcp_src: exact match or wildcard
+ tcp_dst: exact match or wildcard
+" $x $name
+ x=`expr $x + 1`
+ name=table$x
+ done) > expout
+AT_CHECK([ovs-ofctl dump-tables br0], [0], [expout])
+OVS_VSWITCHD_STOP(["/cannot find route for controller/d"])
+AT_CLEANUP
+
AT_SETUP([ofproto - flow table configuration (OpenFlow 1.2)])
OVS_VSWITCHD_START
# Check the default configuration.
--
2.1.3
More information about the discuss
mailing list