It's desirable to keep the resubmit recursion limit as small as
possible to reduce the performance impact of buggy flow tables.
However, some controllers may need a relatively large value. This
patch allows each controller to set the resubmit recursion limit
which is appropriate for its use case.
Signed-off-by: Ethan Jackson <et...@nicira.com>
---
NEWS | 1 +
ofproto/ofproto-dpif.c | 36 +++++++++++++++++++++++++++---------
ofproto/ofproto-provider.h | 8 ++++++++
ofproto/ofproto.c | 16 ++++++++++++++++
ofproto/ofproto.h | 1 +
vswitchd/bridge.c | 8 ++++++++
vswitchd/vswitch.xml | 6 ++++++
7 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 54a7114..db7970c 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,7 @@ post-v1.8.0
are true, but because we do not know of any users for this
feature it seems better on balance to remove it. (The ovs-pki-cgi
program was not included in distribution packaging.)
+ - The resubmit recursion limit is now configurable.
v1.8.0 - xx xxx xxxx
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cf34e92..c3bfdf4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -64,9 +64,9 @@ COVERAGE_DEFINE(facet_revalidate);
COVERAGE_DEFINE(facet_unexpected);
COVERAGE_DEFINE(facet_suppress);
-/* Maximum depth of flow table recursion (due to resubmit actions) in a
+/* Default maximum depth of flow table recursion (due to resubmit actions) in a
* flow translation. */
-#define MAX_RESUBMIT_RECURSION 32
+#define DEFAULT_MAX_RESUBMIT 32
/* Number of implemented OpenFlow tables. */
enum { N_TABLES = 255 };
@@ -593,6 +593,7 @@ COVERAGE_DEFINE(rev_inconsistency);
struct ofproto_dpif {
struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
+ size_t max_resubmit;
struct ofproto up;
struct dpif *dpif;
int max_ports;
@@ -751,6 +752,7 @@ construct(struct ofproto *ofproto_)
}
ofproto->max_ports = dpif_get_max_ports(ofproto->dpif);
+ ofproto->max_resubmit = DEFAULT_MAX_RESUBMIT;
ofproto->n_matches = 0;
dpif_flow_flush(ofproto->dpif);
@@ -4980,8 +4982,9 @@ static void
xlate_table_action(struct action_xlate_ctx *ctx,
uint16_t in_port, uint8_t table_id)
{
- if (ctx->recurse < MAX_RESUBMIT_RECURSION) {
- struct ofproto_dpif *ofproto = ctx->ofproto;
+ struct ofproto_dpif *ofproto = ctx->ofproto;
+
+ if (ctx->recurse < ofproto->max_resubmit) {
struct rule_dpif *rule;
uint16_t old_in_port;
uint8_t old_table_id;
@@ -5032,8 +5035,8 @@ xlate_table_action(struct action_xlate_ctx *ctx,
} else {
static struct vlog_rate_limit recurse_rl = VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_ERR_RL(&recurse_rl, "resubmit actions recursed over %d times",
- MAX_RESUBMIT_RECURSION);
+ VLOG_ERR_RL(&recurse_rl, "resubmit actions recursed over %zu times",
+ ofproto->max_resubmit);
ctx->max_resubmit_trigger = true;
}
}
@@ -5601,9 +5604,9 @@ xlate_actions(struct action_xlate_ctx *ctx,
const struct ofpact *ofpacts, size_t ofpacts_len,
struct ofpbuf *odp_actions)
{
- /* Normally false. Set to true if we ever hit MAX_RESUBMIT_RECURSION, so
- * that in the future we always keep a copy of the original flow for
- * tracing purposes. */
+ /* Normally false. Set to true if we ever hit the resubmit recursion
+ * limit, so that in the future we always keep a copy of the original flow
+ * for tracing purposes. */
static bool hit_resubmit_limit;
enum slow_path_reason special;
@@ -6379,6 +6382,20 @@ rule_invalidate(const struct rule_dpif *rule)
}
}
+static int
+set_max_resubmit(struct ofproto *ofproto, size_t max_resubmit)
+{
+ /* Arbitrarily chosen ridiculously high limit. This should prevent a buggy
+ * controller from essentially setting max_resubmit to infinity and
+ * shooting itself in the foot. */
+ if (max_resubmit > 1024) {
+ return EINVAL;
+ }
+
+ ofproto_dpif_cast(ofproto)->max_resubmit = max_resubmit;
+ return 0;
+}
+
static bool
set_frag_handling(struct ofproto *ofproto_,
enum ofp_config_flags frag_handling)
@@ -7174,6 +7191,7 @@ const struct ofproto_class ofproto_dpif_class = {
rule_get_stats,
rule_execute,
rule_modify_actions,
+ set_max_resubmit,
set_frag_handling,
packet_out,
set_netflow,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 15dc347..eb78663 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -899,6 +899,14 @@ struct ofproto_class {
* rule. */
void (*rule_modify_actions)(struct rule *rule);
+ /* Sets the resubmit recursion limit of 'ofproto' to 'max_resubmit'. Flows
+ * which resubmit deeper than the recursion limit are dropped.
+ *
+ * Returns 0 if successful, or an error. EOPNOTSUPP as a return value
+ * indicates that 'ofproto' does not support changing the resubmit
+ * recursion limit, as does a null pointer. */
+ int (*set_max_resubmit)(struct ofproto *ofproto, size_t max_resubmit);
+
/* Changes the OpenFlow IP fragment handling policy to 'frag_handling',
* which takes one of the following values, with the corresponding
* meanings:
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5c9ab9d..79e9b11 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -455,6 +455,22 @@ ofproto_set_datapath_id(struct ofproto *p, uint64_t
datapath_id)
}
}
+/* Sets the resubmit recursion limit of 'ofproto' to 'max_resubmit'. Flows
+ * which resubmit deeper than the recursion limit are dropped. */
+void
+ofproto_set_max_resubmit(struct ofproto *ofproto, size_t max_resubmit)
+{
+ int error;
+
+ error = (ofproto->ofproto_class->set_max_resubmit
+ ? ofproto->ofproto_class->set_max_resubmit(ofproto, max_resubmit)
+ : EOPNOTSUPP);
+ if (error) {
+ VLOG_WARN("%s: failed to set maximum resubmit limit (%s)",
+ ofproto->name, strerror(error));
+ }
+}
+
void
ofproto_set_controllers(struct ofproto *p,
const struct ofproto_controller *controllers,
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 32a00f2..7911f68 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -203,6 +203,7 @@ int ofproto_port_query_by_name(const struct ofproto *,
const char *devname,
/* Top-level configuration. */
uint64_t ofproto_get_datapath_id(const struct ofproto *);
void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
+void ofproto_set_max_resubmit(struct ofproto *, size_t max_resubmit);
void ofproto_set_controllers(struct ofproto *,
const struct ofproto_controller *, size_t n);
void ofproto_set_fail_mode(struct ofproto *, enum ofproto_fail_mode fail_mode);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9bafa66..5b0a960 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -528,6 +528,7 @@ bridge_reconfigure_continue(const struct
ovsrec_open_vswitch *ovs_cfg)
collect_in_band_managers(ovs_cfg, &managers, &n_managers);
HMAP_FOR_EACH (br, node, &all_bridges) {
struct port *port;
+ int max_resubmit;
/* We need the datapath ID early to allow LACP ports to use it as the
* default system ID. */
@@ -544,6 +545,13 @@ bridge_reconfigure_continue(const struct
ovsrec_open_vswitch *ovs_cfg)
iface_set_mac(iface);
}
}
+
+ max_resubmit = smap_get_int(&br->cfg->other_config,
+ "max-resubmit-recursion", -1);
+ if (max_resubmit >= 0) {
+ ofproto_set_max_resubmit(br->ofproto, max_resubmit);
+ }
+
bridge_configure_mirrors(br);
bridge_configure_flow_eviction_threshold(br);
bridge_configure_forward_bpdu(br);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 7b30ce8..d075d84 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -453,6 +453,12 @@
QoS configured, or if the port does not have a queue with the specified
ID, the default queue is used instead.
</column>
+
+ <column name="other_config" key="max-resubmit-recursion"
+ type='{"type": "integer", "minInteger": 0, "maxInteger": 1024}'>
+ Configures how deeply a flow can resubmit before being dropped. If
+ unset, OVS will choose a reasonable default.
+ </column>
</group>
<group title="Spanning Tree Configuration">
--
1.7.11.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
