[ovs-dev] [bug 7550 8/9] ofproto-dpif: Implement flow table misses as special OpenFlow rules.
Ben Pfaff
blp at nicira.com
Wed Dec 21 21:03:56 UTC 2011
Until now, Open vSwitch has never set up kernel flows for packets that miss
in the OpenFlow flow table. This is more or less acceptable behavior for
packets that should be sent to the controller, since every packet in such
flows must go to userspace anyway, but it is not good for packets that
arrive on OpenFlow ports that have the OFPPC_NO_PACKET_IN flag set, because
such packets will only be discarded by userspace.
This commit changes ofproto-dpif to set up kernel flows for packets that
miss in the OpenFlow flow table. To reduce the number of special cases, it
does this by actually creating OpenFlow flows in a hidden, read-only
OpenFlow table, one for packets that should be sent to the controller, one
for packets to be dropped. This approach has the nice benefit that now
a flow table lookup always succeeds, either with a real flow or one of
these hidden flows.
Another potential advantage of this approach is that users can dump the
hidden table with "ovs-ofctl" if they specify the table number explicitly.
This might be useful for debugging.
Bug #7550.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
NEWS | 4 +-
lib/ofp-util.c | 8 +-
ofproto/ofproto-dpif.c | 194 ++++++++++++++++++++++++++++++++----------------
3 files changed, 137 insertions(+), 69 deletions(-)
diff --git a/NEWS b/NEWS
index 1c67186..376b1a3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,8 +1,8 @@
port-v1.4.0
------------------------
- OpenFlow:
- - New Nicira action NXAST_CONTROLLER that offers additional features over
- output to OFPP_CONTROLLER.
+ - Added an OpenFlow action NXAST_CONTROLLER that offers
+ additional features over output to OFPP_CONTROLLER.
v1.4.0 - xx xxx xxxx
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index a8b29bf..9a6a5af 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2490,11 +2490,13 @@ ofputil_put_action(enum ofputil_action_code code, struct ofpbuf *buf)
bool
action_outputs_to_port(const union ofp_action *action, ovs_be16 port)
{
- switch (ntohs(action->type)) {
- case OFPAT_OUTPUT:
+ switch (ofputil_decode_action(action)) {
+ case OFPUTIL_OFPAT_OUTPUT:
return action->output.port == port;
- case OFPAT_ENQUEUE:
+ case OFPUTIL_OFPAT_ENQUEUE:
return ((const struct ofp_action_enqueue *) action)->port == port;
+ case OFPUTIL_NXAST_CONTROLLER:
+ return port == htons(OFPP_CONTROLLER);
default:
return false;
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 47c7e60..5ea45f3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -54,7 +54,6 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
COVERAGE_DEFINE(ofproto_dpif_ctlr_action);
COVERAGE_DEFINE(ofproto_dpif_expired);
-COVERAGE_DEFINE(ofproto_dpif_no_packet_in);
COVERAGE_DEFINE(ofproto_dpif_xlate);
COVERAGE_DEFINE(facet_changed_rule);
COVERAGE_DEFINE(facet_invalidated);
@@ -67,7 +66,8 @@ COVERAGE_DEFINE(facet_unexpected);
/* Number of implemented OpenFlow tables. */
enum { N_TABLES = 255 };
-BUILD_ASSERT_DECL(N_TABLES >= 1 && N_TABLES <= 255);
+enum { TBL_INTERNAL = N_TABLES - 1 }; /* Used for internal hidden rules. */
+BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
struct ofport_dpif;
struct ofproto_dpif;
@@ -103,8 +103,10 @@ static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
return rule ? CONTAINER_OF(rule, struct rule_dpif, up) : NULL;
}
-static struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *,
- const struct flow *, uint8_t table);
+static struct rule_dpif *rule_lookup(struct ofproto_dpif *,
+ const struct flow *);
+static struct rule_dpif *rule_lookup__(struct ofproto_dpif *,
+ const struct flow *, uint8_t table);
static void flow_push_stats(const struct rule_dpif *, const struct flow *,
uint64_t packets, uint64_t bytes,
@@ -312,7 +314,7 @@ static void facet_free(struct facet *);
static struct facet *facet_find(struct ofproto_dpif *, const struct flow *);
static struct facet *facet_lookup_valid(struct ofproto_dpif *,
const struct flow *);
-static bool facet_revalidate(struct ofproto_dpif *, struct facet *);
+static void facet_revalidate(struct ofproto_dpif *, struct facet *);
static bool execute_controller_action(struct ofproto_dpif *,
const struct flow *,
@@ -477,6 +479,10 @@ struct ofproto_dpif {
struct dpif *dpif;
int max_ports;
+ /* Special OpenFlow rules. */
+ struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
+ struct rule_dpif *no_packet_in_rule; /* Drops flow table misses. */
+
/* Statistics. */
uint64_t n_matches;
@@ -585,6 +591,8 @@ del(const char *type, const char *name)
/* Basic life-cycle. */
+static int add_internal_flows(struct ofproto_dpif *);
+
static struct ofproto *
alloc(void)
{
@@ -665,10 +673,79 @@ construct(struct ofproto *ofproto_)
memset(&ofproto->stats, 0, sizeof ofproto->stats);
ofproto_init_tables(ofproto_, N_TABLES);
+ error = add_internal_flows(ofproto);
+ ofproto->up.tables[TBL_INTERNAL].flags = OFTABLE_HIDDEN | OFTABLE_READONLY;
+
+ return error;
+}
+
+static int
+add_internal_flow(struct ofproto_dpif *ofproto, int id,
+ const struct ofpbuf *actions, struct rule_dpif **rulep)
+{
+ struct ofputil_flow_mod fm;
+ int error;
+
+ cls_rule_init_catchall(&fm.cr, 0);
+ cls_rule_set_reg(&fm.cr, 0, id);
+ fm.cookie = htonll(0);
+ fm.table_id = TBL_INTERNAL;
+ fm.command = OFPFC_ADD;
+ fm.idle_timeout = 0;
+ fm.hard_timeout = 0;
+ fm.buffer_id = 0;
+ fm.out_port = 0;
+ fm.flags = 0;
+ fm.actions = actions->data;
+ fm.n_actions = actions->size / sizeof(union ofp_action);
+
+ error = ofproto_flow_mod(&ofproto->up, &fm);
+ if (error) {
+ char *msg = ofputil_error_to_string(error);
+ VLOG_ERR_RL(&rl, "failed to add internal flow %d (%s)", id, msg);
+ free(msg);
+
+ return error;
+ }
+
+ *rulep = rule_lookup__(ofproto, &fm.cr.flow, TBL_INTERNAL);
+ assert(*rulep != NULL);
return 0;
}
+static int
+add_internal_flows(struct ofproto_dpif *ofproto)
+{
+ struct nx_action_controller *nac;
+ struct ofpbuf actions;
+ int error;
+ int id;
+
+ ofpbuf_init(&actions, 0);
+ id = 1;
+
+ nac = ofputil_put_NXAST_CONTROLLER(&actions);
+ nac->max_len = htons(UINT16_MAX);
+ nac->controllers = htons(0);
+ nac->reason = OFPR_NO_MATCH;
+ error = add_internal_flow(ofproto, id++, &actions, &ofproto->miss_rule);
+ if (error) {
+ goto exit;
+ }
+
+ ofpbuf_clear(&actions);
+ error = add_internal_flow(ofproto, id++, &actions,
+ &ofproto->no_packet_in_rule);
+ if (error) {
+ goto exit;
+ }
+
+exit:
+ ofpbuf_uninit(&actions);
+ return error;
+}
+
static void
destruct(struct ofproto *ofproto_)
{
@@ -776,13 +853,13 @@ run(struct ofproto *ofproto_)
|| !tag_set_is_empty(&ofproto->revalidate_set)) {
struct tag_set revalidate_set = ofproto->revalidate_set;
bool revalidate_all = ofproto->need_revalidate;
- struct facet *facet, *next;
+ struct facet *facet;
/* Clear the revalidation flags. */
tag_set_init(&ofproto->revalidate_set);
ofproto->need_revalidate = false;
- HMAP_FOR_EACH_SAFE (facet, next, hmap_node, &ofproto->facets) {
+ HMAP_FOR_EACH (facet, hmap_node, &ofproto->facets) {
if (revalidate_all
|| tag_set_intersects(&revalidate_set, facet->tags)) {
facet_revalidate(ofproto, facet);
@@ -958,7 +1035,7 @@ port_reconfigured(struct ofport *port_, ovs_be32 old_config)
ovs_be32 changed = old_config ^ port->up.opp.config;
if (changed & htonl(OFPPC_NO_RECV | OFPPC_NO_RECV_STP |
- OFPPC_NO_FWD | OFPPC_NO_FLOOD)) {
+ OFPPC_NO_FWD | OFPPC_NO_FLOOD | OFPPC_NO_PACKET_IN)) {
ofproto->need_revalidate = true;
if (changed & htonl(OFPPC_NO_FLOOD) && port->bundle) {
@@ -2462,33 +2539,7 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
facet = facet_lookup_valid(ofproto, flow);
if (!facet) {
- struct rule_dpif *rule;
-
- rule = rule_dpif_lookup(ofproto, flow, 0);
- if (!rule) {
- /* Don't send a packet-in if OFPPC_NO_PACKET_IN asserted. */
- struct ofport_dpif *port = get_ofp_port(ofproto, flow->in_port);
- if (port) {
- if (port->up.opp.config & htonl(OFPPC_NO_PACKET_IN)) {
- COVERAGE_INC(ofproto_dpif_no_packet_in);
- /* XXX install 'drop' flow entry */
- return;
- }
- } else {
- VLOG_WARN_RL(&rl, "packet-in on unknown port %"PRIu16,
- flow->in_port);
- }
-
- LIST_FOR_EACH_SAFE (packet, next_packet, list_node,
- &miss->packets) {
- list_remove(&packet->list_node);
- send_packet_in_miss(ofproto, packet, flow, false);
- }
-
- return;
- }
-
- facet = facet_create(rule, flow);
+ facet = facet_create(rule_lookup(ofproto, flow), flow);
}
subfacet = subfacet_create(ofproto, facet,
@@ -2499,7 +2550,7 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
struct dpif_flow_stats stats;
list_remove(&packet->list_node);
- ofproto->n_matches++;
+ ofproto->n_matches++; /* XXX */
if (facet->rule->up.cr.priority == FAIL_OPEN_PRIORITY) {
/*
@@ -3282,18 +3333,17 @@ facet_find(struct ofproto_dpif *ofproto, const struct flow *flow)
static struct facet *
facet_lookup_valid(struct ofproto_dpif *ofproto, const struct flow *flow)
{
- struct facet *facet = facet_find(ofproto, flow);
+ struct facet *facet;
- /* The facet we found might not be valid, since we could be in need of
- * revalidation. If it is not valid, don't return it. */
- if (facet
- && (ofproto->need_revalidate
- || tag_set_intersects(&ofproto->revalidate_set, facet->tags))
- && !facet_revalidate(ofproto, facet)) {
- COVERAGE_INC(facet_invalidated);
+ facet = facet_find(ofproto, flow);
+ if (!facet) {
return NULL;
}
+ if (ofproto->need_revalidate
+ || tag_set_intersects(&ofproto->revalidate_set, facet->tags)) {
+ facet_revalidate(ofproto, facet);
+ }
return facet;
}
@@ -3303,12 +3353,8 @@ facet_lookup_valid(struct ofproto_dpif *ofproto, const struct flow *flow)
* 'facet' to the new rule and recompiles its actions.
*
* - If the rule found is the same as 'facet''s current rule, leaves 'facet'
- * where it is and recompiles its actions anyway.
- *
- * - If there is none, destroys 'facet'.
- *
- * Returns true if 'facet' still exists, false if it has been destroyed. */
-static bool
+ * where it is and recompiles its actions anyway. */
+static void
facet_revalidate(struct ofproto_dpif *ofproto, struct facet *facet)
{
struct actions {
@@ -3325,13 +3371,7 @@ facet_revalidate(struct ofproto_dpif *ofproto, struct facet *facet)
COVERAGE_INC(facet_revalidate);
- /* Determine the new rule. */
- new_rule = rule_dpif_lookup(ofproto, &facet->flow, 0);
- if (!new_rule) {
- /* No new rule, so delete the facet. */
- facet_remove(ofproto, facet);
- return false;
- }
+ new_rule = rule_lookup(ofproto, &facet->flow);
/* Calculate new datapath actions.
*
@@ -3412,8 +3452,6 @@ facet_revalidate(struct ofproto_dpif *ofproto, struct facet *facet)
facet->used = new_rule->up.created;
facet->prev_used = facet->used;
}
-
- return true;
}
/* Updates 'facet''s used time. Caller is responsible for calling
@@ -3769,8 +3807,31 @@ subfacet_update_stats(struct ofproto_dpif *ofproto, struct subfacet *subfacet,
/* Rules. */
static struct rule_dpif *
-rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
- uint8_t table_id)
+rule_lookup(struct ofproto_dpif *ofproto, const struct flow *flow)
+{
+ struct ofport_dpif *port;
+ struct rule_dpif *rule;
+
+ rule = rule_lookup__(ofproto, flow, 0);
+ if (rule) {
+ return rule;
+ }
+
+ port = get_ofp_port(ofproto, flow->in_port);
+ if (!port) {
+ VLOG_WARN_RL(&rl, "packet-in on unknown port %"PRIu16, flow->in_port);
+ return ofproto->miss_rule;
+ }
+
+ if (port->up.opp.config & htonl(OFPPC_NO_PACKET_IN)) {
+ return ofproto->no_packet_in_rule;
+ }
+ return ofproto->miss_rule;
+}
+
+static struct rule_dpif *
+rule_lookup__(struct ofproto_dpif *ofproto, const struct flow *flow,
+ uint8_t table_id)
{
struct cls_rule *cls_rule;
struct classifier *cls;
@@ -4156,7 +4217,7 @@ xlate_table_action(struct action_xlate_ctx *ctx,
/* Look up a flow with 'in_port' as the input port. */
old_in_port = ctx->flow.in_port;
ctx->flow.in_port = in_port;
- rule = rule_dpif_lookup(ofproto, &ctx->flow, table_id);
+ rule = rule_lookup__(ofproto, &ctx->flow, table_id);
/* Tag the flow. */
if (table_id > 0 && table_id < N_TABLES) {
@@ -5717,9 +5778,14 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
flow_format(&result, &flow);
ds_put_char(&result, '\n');
- rule = rule_dpif_lookup(ofproto, &flow, 0);
+ rule = rule_lookup(ofproto, &flow);
trace_format_rule(&result, 0, 0, rule);
- if (rule) {
+ if (rule == ofproto->miss_rule) {
+ ds_put_cstr(&result, "\nNo match, flow generates \"packet in\"s.");
+ } else if (rule == ofproto->no_packet_in_rule) {
+ ds_put_cstr(&result, "\nNo match, packets dropped because "
+ "OFPPC_NO_PACKET_IN is set on in_port");
+ } else {
struct ofproto_trace trace;
struct ofpbuf *odp_actions;
--
1.7.2.5
More information about the dev
mailing list