From: Ben Pfaff Date: Fri, 26 Jun 2015 15:14:15 +0000 (-0700) Subject: ofp-actions: Support mixing "conjunction" and "note" actions. X-Git-Tag: v2.5.0~992 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=f08e39dd023ff27c32166b76d6906fbf20f78991 ofp-actions: Support mixing "conjunction" and "note" actions. It doesn't make sense to mix "conjunction" actions with most other kinds of actions. That's because flows with "conjunction" actions aren't ever actually executed, so any actions mixed up with them would never do anything useful. "note" actions are a little different because they never do anything useful anyway: they are just there to allow a controller to annotate flows. It makes as much sense to annotate a flow with "conjunction" actions as it does to annotate any other flow, so this commit makes this possible. Requested-by: Soner Sevinc Signed-off-by: Ben Pfaff Acked-by: Andy Zhou --- diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 10e2a92c0..eef3389bb 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -5737,9 +5737,10 @@ ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len, if (a->type == OFPACT_CONJUNCTION) { OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - if (a->type != OFPACT_CONJUNCTION) { - VLOG_WARN("when conjunction action is present, it must be " - "the only kind of action used (saw '%s' action)", + if (a->type != OFPACT_CONJUNCTION && a->type != OFPACT_NOTE) { + VLOG_WARN("\"conjunction\" actions may be used along with " + "\"note\" but not any other kind of action " + "(such as the \"%s\" action used here)", ofpact_name(a->type)); return OFPERR_NXBAC_BAD_CONJUNCTION; } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 523130b40..acf504f5d 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4408,12 +4408,6 @@ evict_rules_from_table(struct oftable *table) return error; } -static bool -is_conjunction(const struct ofpact *ofpacts, size_t ofpacts_len) -{ - return ofpacts_len > 0 && ofpacts->type == OFPACT_CONJUNCTION; -} - static void get_conjunctions(const struct ofputil_flow_mod *fm, struct cls_conjunction **conjsp, size_t *n_conjsp) @@ -4422,23 +4416,29 @@ get_conjunctions(const struct ofputil_flow_mod *fm, struct cls_conjunction *conjs = NULL; int n_conjs = 0; - if (is_conjunction(fm->ofpacts, fm->ofpacts_len)) { - const struct ofpact *ofpact; - int i; - - n_conjs = 0; - OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) { + const struct ofpact *ofpact; + OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) { + if (ofpact->type == OFPACT_CONJUNCTION) { n_conjs++; + } else if (ofpact->type != OFPACT_NOTE) { + /* "conjunction" may appear with "note" actions but not with any + * other type of actions. */ + ovs_assert(!n_conjs); + break; } + } + if (n_conjs) { + int i = 0; conjs = xzalloc(n_conjs * sizeof *conjs); - i = 0; OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) { - struct ofpact_conjunction *oc = ofpact_get_CONJUNCTION(ofpact); - conjs[i].clause = oc->clause; - conjs[i].n_clauses = oc->n_clauses; - conjs[i].id = oc->id; - i++; + if (ofpact->type == OFPACT_CONJUNCTION) { + struct ofpact_conjunction *oc = ofpact_get_CONJUNCTION(ofpact); + conjs[i].clause = oc->clause; + conjs[i].n_clauses = oc->n_clauses; + conjs[i].id = oc->id; + i++; + } } } diff --git a/tests/classifier.at b/tests/classifier.at index 1e75123c5..3520acd07 100644 --- a/tests/classifier.at +++ b/tests/classifier.at @@ -289,3 +289,20 @@ for src in 0 1 2 3; do done OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([conjunctive match and other actions]) +OVS_VSWITCHD_START +# It's OK to use "conjunction" actions with "note" actions. +AT_CHECK([ovs-ofctl add-flow br0 'actions=conjunction(3,1/2),note:41.42.43.44.45.46']) +AT_CHECK([ovs-ofctl add-flow br0 'actions=note:41.42.43.44.45.46,conjunction(3,1/2)']) +# It's not OK to use "conjunction" actions with other types of actions. +AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' add-flow br0 'actions=output:1,conjunction(3,1/2)'], [1], [], [dnl +ofp_actions|WARN|"conjunction" actions may be used along with "note" but not any other kind of action (such as the "output" action used here) +ovs-ofctl: Incorrect instruction ordering +]) +AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' add-flow br0 'actions=conjunction(3,1/2),output:1'], [1], [], [dnl +ofp_actions|WARN|"conjunction" actions may be used along with "note" but not any other kind of action (such as the "output" action used here) +ovs-ofctl: Incorrect instruction ordering +]) +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 0a30d3e6d..395d85114 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -2015,8 +2015,9 @@ example, then the flow that matches \fBip_src=\fIb\fR would have two of \fBconjunction\fR actions within a list of actions is not significant. .IP \(bu -A flow with \fBconjunction\fR actions may not have any other actions. -(It would not be useful.) +A flow with \fBconjunction\fR actions may also include \fBnote\fR +actions for annotations, but not any other kind of actions. (They +would not be useful because they would never be executed.) .IP \(bu All of the flows that constitute a conjunctive flow with a given \fIid\fR must have the same priority. (Flows with the same \fIid\fR