From 0ab6decf2ce8bae6290967b6f0a3252dc86c4c55 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Thu, 19 May 2011 16:18:57 -0700
Subject: [PATCH] ofproto: Properly initialize table_id when creating rules.
Nothing was initializing table_id, and there was no "hook" to choose an
appropriate table. This fixes both problems.
Found by inspection.
---
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto.c | 25 +++++++++++++++++++++----
ofproto/private.h | 19 +++++++++++++++++++
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 63bc3799..fd3f2127 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3926,6 +3926,7 @@ const struct ofproto_class ofproto_dpif_class = {
port_poll,
port_poll_wait,
port_is_lacp_current,
+ NULL, /* rule_choose_table */
rule_alloc,
rule_construct,
rule_destruct,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ac646e50..4c370d3b 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -64,7 +64,8 @@ COVERAGE_DEFINE(ofproto_update_port);
static void ofport_destroy__(struct ofport *);
static void ofport_destroy(struct ofport *);
-static int rule_create(struct ofproto *, const struct cls_rule *,
+static int rule_create(struct ofproto *,
+ const struct cls_rule *, uint8_t table_id,
const union ofp_action *, size_t n_actions,
uint16_t idle_timeout, uint16_t hard_timeout,
ovs_be64 flow_cookie, bool send_flow_removed,
@@ -871,7 +872,7 @@ ofproto_add_flow(struct ofproto *p, const struct cls_rule *cls_rule,
const union ofp_action *actions, size_t n_actions)
{
struct rule *rule;
- rule_create(p, cls_rule, actions, n_actions, 0, 0, 0, false, &rule);
+ rule_create(p, cls_rule, 0, actions, n_actions, 0, 0, 0, false, &rule);
}
/* Searches for a rule with matching criteria exactly equal to 'target' in
@@ -1222,7 +1223,8 @@ init_ports(struct ofproto *p)
* flow table, and stores the new rule into '*rulep'. Returns 0 on success,
* otherwise a positive errno value or OpenFlow error code. */
static int
-rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
+rule_create(struct ofproto *ofproto,
+ const struct cls_rule *cls_rule, uint8_t table_id,
const union ofp_action *actions, size_t n_actions,
uint16_t idle_timeout, uint16_t hard_timeout,
ovs_be64 flow_cookie, bool send_flow_removed,
@@ -1231,6 +1233,20 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
struct rule *rule;
int error;
+ if (table_id == 0xff) {
+ if (ofproto->n_tables > 1) {
+ error = ofproto->ofproto_class->rule_choose_table(ofproto,
+ cls_rule,
+ &table_id);
+ if (error) {
+ return error;
+ }
+ assert(table_id < ofproto->n_tables);
+ } else {
+ table_id = 0;
+ }
+ }
+
rule = ofproto->ofproto_class->rule_alloc();
if (!rule) {
error = ENOMEM;
@@ -1239,6 +1255,7 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
rule->ofproto = ofproto;
rule->cr = *cls_rule;
+ rule->table_id = table_id;
rule->flow_cookie = flow_cookie;
rule->created = time_msec();
rule->idle_timeout = idle_timeout;
@@ -2209,7 +2226,7 @@ add_flow(struct ofconn *ofconn, struct flow_mod *fm)
}
buf_err = ofconn_pktbuf_retrieve(ofconn, fm->buffer_id, &packet, &in_port);
- error = rule_create(p, &fm->cr, fm->actions, fm->n_actions,
+ error = rule_create(p, &fm->cr, fm->table_id, fm->actions, fm->n_actions,
fm->idle_timeout, fm->hard_timeout, fm->cookie,
fm->flags & OFPFF_SEND_FLOW_REM, &rule);
if (error) {
diff --git a/ofproto/private.h b/ofproto/private.h
index db0edef1..7a41a10a 100644
--- a/ofproto/private.h
+++ b/ofproto/private.h
@@ -506,6 +506,25 @@ struct ofproto_class {
/* ## OpenFlow Rule Functions ## */
/* ## ----------------------- ## */
+ /* Chooses an appropriate table for 'cls_rule' within 'ofproto'. On
+ * success, stores the table ID into '*table_idp' and returns 0. On
+ * failure, returns an OpenFlow error code (as returned by ofp_mkerr()).
+ *
+ * The choice of table should be a function of 'cls_rule' and 'ofproto''s
+ * datapath capabilities. It should not depend on the flows already in
+ * 'ofproto''s flow tables. Failure implies that an OpenFlow rule with
+ * 'cls_rule' as its matching condition can never be inserted into
+ * 'ofproto', even starting from an empty flow table.
+ *
+ * If multiple tables are candidates for inserting the flow, the function
+ * should choose one arbitrarily (but deterministically).
+ *
+ * This function will never be called for an ofproto that has only one
+ * table, so it may be NULL in that case. */
+ int (*rule_choose_table)(const struct ofproto *ofproto,
+ const struct cls_rule *cls_rule,
+ uint8_t *table_idp);
+
/* Life-cycle functions for a "struct rule" (see "Life Cycle" above).
*
* ->rule_construct() should first check whether the rule is acceptable:
--
2.30.2