Skip to content

Commit

Permalink
Implement learned flow deletion.
Browse files Browse the repository at this point in the history
When a flow with a "learn" action is deleted, one often wants the flows
that it created (the "learned flows") to be deleted as well.  This commit
makes that possible.

I am aware of a race condition that could lead to a learned flow not being
properly deleted.  Suppose thread A deletes a flow with a "learn" action.
Meanwhile, thread B obtains the actions for this flow and translates and
executes them.  Thread B could obtain the actions for the flow before it is
deleted, but execute them after the "learn" flow and its learned flows are
deleted.  The result is that the flow created by thread B persists despite
its "learn" flow having been deleted.  This race can and should be fixed,
but I think that this commit is worth reviewing without it.

VMware-BZ: #1254021
Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Thomas Graf <[email protected]>
Acked-by: Ethan Jackson <[email protected]>
  • Loading branch information
blp committed Jun 12, 2014
1 parent 9ca4a86 commit 35f48b8
Show file tree
Hide file tree
Showing 8 changed files with 375 additions and 18 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
Post-v2.3.0
---------------------
- The "learn" action supports a new flag "delete_learned" that causes
the learned flows to be deleted when the flow with the "learn" action
is deleted.


v2.3.0 - xx xxx xxxx
Expand Down
36 changes: 34 additions & 2 deletions include/openflow/nicira-ext.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -927,7 +927,7 @@ struct nx_action_learn {
ovs_be16 hard_timeout; /* Max time before discarding (seconds). */
ovs_be16 priority; /* Priority level of flow entry. */
ovs_be64 cookie; /* Cookie for new flow. */
ovs_be16 flags; /* Either 0 or OFPFF_SEND_FLOW_REM. */
ovs_be16 flags; /* NX_LEARN_F_*. */
uint8_t table_id; /* Table to insert flow entry. */
uint8_t pad; /* Must be zero. */
ovs_be16 fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
Expand All @@ -937,6 +937,38 @@ struct nx_action_learn {
};
OFP_ASSERT(sizeof(struct nx_action_learn) == 32);

/* Bits for 'flags' in struct nx_action_learn.
*
* If NX_LEARN_F_SEND_FLOW_REM is set, then the learned flows will have their
* OFPFF_SEND_FLOW_REM flag set.
*
* If NX_LEARN_F_DELETE_LEARNED is set, then removing this action will delete
* all the flows from the learn action's 'table_id' that have the learn
* action's 'cookie'. Important points:
*
* - The deleted flows include those created by this action, those created
* by other learn actions with the same 'table_id' and 'cookie', those
* created by flow_mod requests by a controller in the specified table
* with the specified cookie, and those created through any other
* means.
*
* - If multiple flows specify "learn" actions with
* NX_LEARN_F_DELETE_LEARNED with the same 'table_id' and 'cookie', then
* no deletion occurs until all of those "learn" actions are deleted.
*
* - Deleting a flow that contains a learn action is the most obvious way
* to delete a learn action. Modifying a flow's actions, or replacing it
* by a new flow, can also delete a learn action. Finally, replacing a
* learn action with NX_LEARN_F_DELETE_LEARNED with a learn action
* without that flag also effectively deletes the learn action and can
* trigger flow deletion.
*
* NX_LEARN_F_DELETE_LEARNED was added in Open vSwitch 2.4. */
enum nx_learn_flags {
NX_LEARN_F_SEND_FLOW_REM = 1 << 0,
NX_LEARN_F_DELETE_LEARNED = 1 << 1,
};

#define NX_LEARN_N_BITS_MASK 0x3ff

#define NX_LEARN_SRC_FIELD (0 << 13) /* Copy from field. */
Expand Down
26 changes: 14 additions & 12 deletions lib/learn.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,9 @@ learn_from_openflow(const struct nx_action_learn *nal, struct ofpbuf *ofpacts)
learn->fin_idle_timeout = ntohs(nal->fin_idle_timeout);
learn->fin_hard_timeout = ntohs(nal->fin_hard_timeout);

/* We only support "send-flow-removed" for now. */
switch (ntohs(nal->flags)) {
case 0:
learn->flags = 0;
break;
case OFPFF_SEND_FLOW_REM:
learn->flags = OFPUTIL_FF_SEND_FLOW_REM;
break;
default:
learn->flags = ntohs(nal->flags);
if (learn->flags & ~(NX_LEARN_F_SEND_FLOW_REM |
NX_LEARN_F_DELETE_LEARNED)) {
return OFPERR_OFPBAC_BAD_ARGUMENT;
}

Expand Down Expand Up @@ -321,7 +315,10 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
fm->hard_timeout = learn->hard_timeout;
fm->buffer_id = UINT32_MAX;
fm->out_port = OFPP_NONE;
fm->flags = learn->flags;
fm->flags = 0;
if (learn->flags & NX_LEARN_F_SEND_FLOW_REM) {
fm->flags |= OFPUTIL_FF_SEND_FLOW_REM;
}
fm->ofpacts = NULL;
fm->ofpacts_len = 0;
fm->delete_reason = OFPRR_DELETE;
Expand Down Expand Up @@ -582,7 +579,9 @@ learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
} else if (!strcmp(name, "cookie")) {
learn->cookie = htonll(strtoull(value, NULL, 0));
} else if (!strcmp(name, "send_flow_rem")) {
learn->flags |= OFPFF_SEND_FLOW_REM;
learn->flags |= NX_LEARN_F_SEND_FLOW_REM;
} else if (!strcmp(name, "delete_learned")) {
learn->flags |= NX_LEARN_F_DELETE_LEARNED;
} else {
struct ofpact_learn_spec *spec;
char *error;
Expand Down Expand Up @@ -656,9 +655,12 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)
if (learn->priority != OFP_DEFAULT_PRIORITY) {
ds_put_format(s, ",priority=%"PRIu16, learn->priority);
}
if (learn->flags & OFPFF_SEND_FLOW_REM) {
if (learn->flags & NX_LEARN_F_SEND_FLOW_REM) {
ds_put_cstr(s, ",send_flow_rem");
}
if (learn->flags & NX_LEARN_F_DELETE_LEARNED) {
ds_put_cstr(s, ",delete_learned");
}
if (learn->cookie != 0) {
ds_put_format(s, ",cookie=%#"PRIx64, ntohll(learn->cookie));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ofp-actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ struct ofpact_learn {
uint16_t priority; /* Priority level of flow entry. */
uint8_t table_id; /* Table to insert flow entry. */
ovs_be64 cookie; /* Cookie for new flow. */
enum ofputil_flow_mod_flags flags;
enum nx_learn_flags flags; /* NX_LEARN_F_*. */
uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */

Expand Down
7 changes: 6 additions & 1 deletion ofproto/ofproto-provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ struct ofproto {

/* Rules indexed on their cookie values, in all flow tables. */
struct hindex cookies OVS_GUARDED_BY(ofproto_mutex);
struct hmap learned_cookies OVS_GUARDED_BY(ofproto_mutex);

/* List of expirable flows, in all flow tables. */
struct list expirable OVS_GUARDED_BY(ofproto_mutex);
Expand Down Expand Up @@ -405,8 +406,12 @@ static inline bool rule_is_hidden(const struct rule *);
struct rule_actions {
/* Flags.
*
* 'has_meter' is true if 'ofpacts' contains an OFPACT_METER action. */
* 'has_meter' is true if 'ofpacts' contains an OFPACT_METER action.
*
* 'has_learn_with_delete' is true if 'ofpacts' contains an OFPACT_LEARN
* action whose flags include NX_LEARN_F_DELETE_LEARNED. */
bool has_meter;
bool has_learn_with_delete;

/* Actions. */
uint32_t ofpacts_len; /* Size of 'ofpacts', in bytes. */
Expand Down
Loading

0 comments on commit 35f48b8

Please sign in to comment.