Signed-off-by: Isaku Yamahata <[email protected]>
---
v5
- manual rebase
v4
- squashed goto-table instruction part into single patch
- only introduce goto-table. Other instruction will be addressed
by other patches.
- check zero padding
- man page
- unit test
v3
- introduce OFPACT_{CLEAR_ACTIONS, WRITE_ACTIONS, GOTO_TABLE}
v2
- changed for ofp_instruction
---
lib/ofp-actions.c | 157 ++++++++++++++++++++++++++++++++++++++++-----
lib/ofp-actions.h | 21 ++++++-
lib/ofp-parse.c | 125 ++++++++++++++++++++++++++++++++++---
ofproto/ofproto-dpif.c | 9 +++
tests/ofp-actions.at | 14 +++-
utilities/ovs-ofctl.8.in | 18 +++++
6 files changed, 313 insertions(+), 31 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index baa6fdd..1ff35cc 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -939,9 +939,15 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf
*openflow,
}
if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
+ const struct ofp11_instruction_actions *oia;
const union ofp_action *actions;
size_t n_actions;
+ oia = instruction_get_OFPIT11_APPLY_ACTIONS(
+ insts[OVSINST_OFPIT11_APPLY_ACTIONS]);
+ if (!is_all_zeros(oia->pad, sizeof oia->pad)) {
+ return OFPERR_OFPBAC_BAD_ARGUMENT;
+ }
get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
&actions, &n_actions);
error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
@@ -949,9 +955,23 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf
*openflow,
goto exit;
}
}
+ /* TODO:XXX Clear-Actions */
+ /* TODO:XXX Write-Actions */
+ /* TODO:XXX Write-Metadata */
+ if (insts[OVSINST_OFPIT11_GOTO_TABLE]) {
+ const struct ofp11_instruction_goto_table *oigt;
+ struct ofpact_goto_table *ogt;
+
+ oigt = instruction_get_OFPIT11_GOTO_TABLE(
+ insts[OVSINST_OFPIT11_GOTO_TABLE]);
+ if (!is_all_zeros(oigt->pad, sizeof oigt->pad)) {
+ return OFPERR_OFPBAC_BAD_ARGUMENT;
+ }
+ ogt = ofpact_put_GOTO_TABLE(ofpacts);
+ ogt->table_id = oigt->table_id;
+ }
- if (insts[OVSINST_OFPIT11_GOTO_TABLE] ||
- insts[OVSINST_OFPIT11_WRITE_METADATA] ||
+ if (insts[OVSINST_OFPIT11_WRITE_METADATA] ||
insts[OVSINST_OFPIT11_WRITE_ACTIONS] ||
insts[OVSINST_OFPIT11_CLEAR_ACTIONS]) {
error = OFPERR_OFPBIC_UNSUP_INST;
@@ -1031,6 +1051,9 @@ ofpact_check__(const struct ofpact *a, const struct flow
*flow, int max_ports)
case OFPACT_EXIT:
return 0;
+ case OFPACT_GOTO_TABLE:
+ return 0;
+
default:
NOT_REACHED();
}
@@ -1246,6 +1269,7 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf
*out)
case OFPACT_SET_IPV4_DSCP:
case OFPACT_SET_L4_SRC_PORT:
case OFPACT_SET_L4_DST_PORT:
+ case OFPACT_GOTO_TABLE:
NOT_REACHED();
}
}
@@ -1335,6 +1359,10 @@ ofpact_to_openflow10(const struct ofpact *a, struct
ofpbuf *out)
= htons(ofpact_get_SET_L4_DST_PORT(a)->port);
break;
+ case OFPACT_GOTO_TABLE:
+ /* TODO:XXX */
+ break;
+
case OFPACT_CONTROLLER:
case OFPACT_OUTPUT_REG:
case OFPACT_BUNDLE:
@@ -1443,6 +1471,9 @@ ofpact_to_openflow11(const struct ofpact *a, struct
ofpbuf *out)
= htons(ofpact_get_SET_L4_DST_PORT(a)->port);
break;
+ case OFPACT_GOTO_TABLE:
+ NOT_REACHED();
+
case OFPACT_CONTROLLER:
case OFPACT_OUTPUT_REG:
case OFPACT_BUNDLE:
@@ -1481,18 +1512,10 @@ ofpacts_put_openflow11_actions(const struct ofpact
ofpacts[],
return openflow->size - start_size;
}
-void
-ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[],
- size_t ofpacts_len,
- struct ofpbuf *openflow)
+static void
+ofpacts_update_instruction_actions(struct ofpbuf *openflow, size_t ofs)
{
struct ofp11_instruction_actions *oia;
- size_t ofs;
-
- /* Put an OFPIT11_APPLY_ACTIONS instruction and fill it in. */
- ofs = openflow->size;
- instruction_put_OFPIT11_APPLY_ACTIONS(openflow);
- ofpacts_put_openflow11_actions(ofpacts, ofpacts_len, openflow);
/* Update the instruction's length (or, if it's empty, delete it). */
oia = ofpbuf_at_assert(openflow, ofs, sizeof *oia);
@@ -1502,6 +1525,46 @@ ofpacts_put_openflow11_instructions(const struct ofpact
ofpacts[],
openflow->size = ofs;
}
}
+
+void
+ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[],
+ size_t ofpacts_len,
+ struct ofpbuf *openflow)
+{
+ bool in_instruction;
+ size_t ofs;
+ const struct ofpact *a;
+
+ in_instruction = false;
+ ofs = 0;
+ OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+ if (in_instruction && ofpact_is_instruction(a)) {
+ ofpacts_update_instruction_actions(openflow, ofs);
+ in_instruction = false;
+ ofs = 0;
+ }
+
+ /* TODO:XXX Clear-Actions */
+ /* TODO:XXX Write-Actions */
+ /* TODO:XXX Write-Metadata */
+ if (a->type == OFPACT_GOTO_TABLE) {
+ struct ofp11_instruction_goto_table *oigt;
+ oigt = instruction_put_OFPIT11_GOTO_TABLE(openflow);
+ oigt->table_id = ofpact_get_GOTO_TABLE(a)->table_id;
+ memset(oigt->pad, 0, sizeof oigt->pad);
+ } else {
+ if (!in_instruction) {
+ in_instruction = true;
+ ofs = openflow->size;
+ instruction_put_OFPIT11_APPLY_ACTIONS(openflow);
+ }
+ ofpact_to_openflow11(a, openflow);
+ }
+ }
+ if (in_instruction) {
+ ofpacts_update_instruction_actions(openflow, ofs);
+ }
+}
/* Returns true if 'action' outputs to 'port', false otherwise. */
static bool
@@ -1540,6 +1603,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact,
uint16_t port)
case OFPACT_AUTOPATH:
case OFPACT_NOTE:
case OFPACT_EXIT:
+ case OFPACT_GOTO_TABLE:
default:
return false;
}
@@ -1803,28 +1867,85 @@ ofpact_format(const struct ofpact *a, struct ds *s)
case OFPACT_EXIT:
ds_put_cstr(s, "exit");
break;
+
+ case OFPACT_GOTO_TABLE:
+ NOT_REACHED();
}
}
+static void
+ofpacts_format_close_paren(struct ds *string, int n_actions)
+{
+ if (n_actions == 0) {
+ ds_put_cstr(string, "drop");
+ }
+ ds_put_cstr(string, ")");
+}
+
/* Appends a string representing the 'ofpacts_len' bytes of ofpacts in
* 'ofpacts' to 'string'. */
void
ofpacts_format(const struct ofpact *ofpacts, size_t ofpacts_len,
struct ds *string)
{
- ds_put_cstr(string, "actions=");
+ const struct ofpact *a;
+ const char *prefix;
+ bool use_instruction;
+ bool in_instruction;
+ int n_actions;
+
+ prefix = "actions";
+ use_instruction = false;
+ OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+ if (ofpact_is_instruction(a)) {
+ prefix = "instructions";
+ use_instruction = true;
+ break;
+ }
+ }
+ ds_put_format(string, "%s=", prefix);
if (!ofpacts_len) {
ds_put_cstr(string, "drop");
- } else {
- const struct ofpact *a;
+ return;
+ }
- OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
- if (a != ofpacts) {
- ds_put_cstr(string, ",");
+ in_instruction = false;
+ n_actions = 0;
+ OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+ if (a != ofpacts) {
+ ds_put_cstr(string, ",");
+ }
+
+ if (in_instruction && ofpact_is_instruction(a)) {
+ ofpacts_format_close_paren(string, n_actions);
+ in_instruction = false;
+ n_actions = 0;
+ }
+
+ /* TODO:XXX clear-actions */
+ /* TODO:XXX write-actions */
+ /* TODO:XXX write-metadata */
+ if (a->type == OFPACT_GOTO_TABLE) {
+ ds_put_format(string, "%s:%"PRIu8,
+ ofpact_instruction_name_from_type(
+ OVSINST_OFPIT11_GOTO_TABLE),
+ ofpact_get_GOTO_TABLE(a)->table_id);
+ } else {
+ if (!in_instruction && use_instruction) {
+ ds_put_format(string, "%s(", ofpact_instruction_name_from_type(
+ OVSINST_OFPIT11_APPLY_ACTIONS));
+ in_instruction = true;
+ n_actions = 0;
}
+
ofpact_format(a, string);
+ n_actions++;
}
}
+
+ if (in_instruction) {
+ ofpacts_format_close_paren(string, n_actions);
+ }
}
/* Internal use by helpers. */
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 4fc9094..4995c66 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -87,7 +87,11 @@
\
/* Other. */ \
DEFINE_OFPACT(NOTE, ofpact_note, data) \
- DEFINE_OFPACT(EXIT, ofpact_null, ofpact)
+ DEFINE_OFPACT(EXIT, ofpact_null, ofpact) \
+ \
+ /* Instructions */ \
+ /* TODO:XXX Clear-Actions, Write-Actions, Write-Metadata */ \
+ DEFINE_OFPACT(GOTO_TABLE, ofpact_goto_table, ofpact)
/* enum ofpact_type, with a member OFPACT_<ENUM> for each action. */
enum OVS_PACKED_ENUM ofpact_type {
@@ -389,7 +393,14 @@ struct ofpact_cnt_ids {
/* Controller ids. */
unsigned int n_controllers;
uint16_t cnt_ids[];
+};
+/* OFPACT_GOTO_TABLE
+ *
+ * Used for OFPIT11_GOTO_TABLE */
+struct ofpact_goto_table {
+ struct ofpact ofpact;
+ uint8_t table_id;
};
/* Converting OpenFlow to ofpacts. */
@@ -542,6 +553,14 @@ enum {
#undef DEFINE_INST
};
+
+static inline bool
+ofpact_is_instruction(const struct ofpact *a)
+{
+ /* TODO:XXX Clear-Actions, Write-Actions, Write-Metadata */
+ return a->type == OFPACT_GOTO_TABLE;
+}
+
const char *ofpact_instruction_name_from_type(enum ovs_instruction_type type);
int ofpact_instruction_type_from_name(const char *name);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index e5f5ea0..8248830 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -507,6 +507,90 @@ str_to_ofpacts(const struct flow *flow, char *str, struct
ofpbuf *ofpacts)
ofpact_pad(ofpacts);
}
+static void
+parse_named_instruction(enum ovs_instruction_type type,
+ const struct flow *flow,
+ char *arg, struct ofpbuf *ofpacts)
+{
+ switch (type) {
+ case OVSINST_OFPIT11_APPLY_ACTIONS:
+ str_to_ofpacts(flow, arg, ofpacts);
+ break;
+
+ case OVSINST_OFPIT11_WRITE_ACTIONS:
+ NOT_REACHED(); /* TODO:XXX */
+ break;
+
+ case OVSINST_OFPIT11_CLEAR_ACTIONS:
+ NOT_REACHED(); /* TODO:XXX */
+ break;
+
+ case OVSINST_OFPIT11_WRITE_METADATA:
+ NOT_REACHED(); /* TODO:XXX */
+ break;
+
+ case OVSINST_OFPIT11_GOTO_TABLE: {
+ struct ofpact_goto_table *ogt = ofpact_put_GOTO_TABLE(ofpacts);
+ char *table_s = strsep(&arg, ",");
+ if (!table_s || !table_s[0]) {
+ ovs_fatal(0, "instruction goto-table needs table id");
+ }
+ ogt->table_id = str_to_table_id(table_s);
+ break;
+ }
+ }
+}
+
+static void
+str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
+{
+ char *pos, *inst, *arg;
+ char *inst_args[N_OVS_INSTRUCTIONS];
+ int type;
+ const char *prev_inst = NULL;
+ int prev_type = -1;
+
+ memset(inst_args, 0, sizeof inst_args);
+ pos = str;
+ while (ofputil_parse_key_value(&pos, &inst, &arg)) {
+ type = ofpact_instruction_type_from_name(inst);
+ if (type < 0) {
+ ovs_fatal(0, "Unknown instruction: %s", inst);
+ }
+ if (type == prev_type) {
+ ovs_fatal(0, "instruction can be specified at most once: %s",
+ inst);
+ }
+ if (type <= prev_type) {
+ ovs_fatal(0, "Instruction %s must be specified before %s",
+ inst, prev_inst);
+ }
+ inst_args[type] = arg;
+
+ prev_inst = inst;
+ prev_type = type;
+ }
+
+ /* TODO:XXX */
+ if (inst_args[OVSINST_OFPIT11_CLEAR_ACTIONS]) {
+ ovs_fatal(0, "instruction clear-actions is not supported yet");
+ }
+ if (inst_args[OVSINST_OFPIT11_WRITE_ACTIONS]) {
+ ovs_fatal(0, "instruction write-actions is not supported yet");
+ }
+ if (inst_args[OVSINST_OFPIT11_WRITE_METADATA]) {
+ ovs_fatal(0, "instruction write-metadata is not supported yet");
+ }
+
+ for (type = 0; type < N_OVS_INSTRUCTIONS; type++) {
+ if (inst_args[type]) {
+ parse_named_instruction(type, flow, inst_args[type], ofpacts);
+ }
+ }
+
+ ofpact_pad(ofpacts);
+}
+
struct protocol {
const char *name;
uint16_t dl_type;
@@ -588,6 +672,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
const char *str_,
} fields;
char *string = xstrdup(str_);
char *save_ptr = NULL;
+ char *inst_str = NULL;
char *act_str = NULL;
char *name;
@@ -638,17 +723,35 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
const char *str_,
fm->flags = 0;
if (fields & F_ACTIONS) {
act_str = strstr(string, "action");
- if (!act_str) {
- ofp_fatal(str_, verbose, "must specify an action");
- }
- *act_str = '\0';
+ if (act_str) {
+ *act_str = '\0';
+
+ act_str = strchr(act_str + 1, '=');
+ if (!act_str) {
+ ofp_fatal(str_, verbose, "must specify an action");
+ }
- act_str = strchr(act_str + 1, '=');
- if (!act_str) {
- ofp_fatal(str_, verbose, "must specify an action");
+ act_str++;
}
- act_str++;
+ inst_str = strstr(string, "instruction");
+ if (inst_str) {
+ *inst_str = '\0';
+
+ inst_str = strchr(inst_str + 1, '=');
+ if (!inst_str) {
+ ofp_fatal(str_, verbose, "must specify an instruction");
+ }
+
+ inst_str++;
+ }
+ }
+ if (fields & F_ACTIONS && act_str == NULL && inst_str == NULL) {
+ ofp_fatal(str_, verbose, "must specify an action or an instruction");
+ }
+ if (act_str && inst_str) {
+ ofp_fatal(str_, verbose,
+ "must not specify both action and instruction.");
}
for (name = strtok_r(string, "=, \t\r\n", &save_ptr); name;
name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
@@ -725,7 +828,11 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
const char *str_,
struct ofpbuf ofpacts;
ofpbuf_init(&ofpacts, 32);
- str_to_ofpacts(&fm->cr.flow, act_str, &ofpacts);
+ if (act_str) {
+ str_to_ofpacts(&fm->cr.flow, act_str, &ofpacts);
+ } else {
+ str_to_inst_ofpacts(&fm->cr.flow, inst_str, &ofpacts);
+ }
fm->ofpacts_len = ofpacts.size;
fm->ofpacts = ofpbuf_steal_data(&ofpacts);
} else {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b83a50f..ed0bc08 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5581,6 +5581,15 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
ctx->has_fin_timeout = true;
xlate_fin_timeout(ctx, ofpact_get_FIN_TIMEOUT(a));
break;
+
+ case OFPACT_GOTO_TABLE: {
+ /* TODO:XXX remove recursion */
+ /* It is assumed that goto-table is last action */
+ struct ofpact_goto_table *ogt = ofpact_get_GOTO_TABLE(a);
+ assert(ctx->table_id < ogt->table_id);
+ xlate_table_action(ctx, ctx->flow.in_port, ogt->table_id);
+ break;
+ }
}
}
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index 36c67f1..acc9e13 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -247,6 +247,10 @@ AT_DATA([test-data], [dnl
0004 0018 00000000 dnl
0000 0010 fffffffe 04d2 000000000000
+dnl Apply-Actions non-zero padding
+# bad OF1.1 instructions: OFPBAC_BAD_ARGUMENT
+0004 0008 00000001
+
dnl Check that an empty Apply-Actions instruction gets dropped.
# actions=drop
# 0: 00 -> (none)
@@ -271,8 +275,12 @@ dnl Goto-Table instruction too long.
# bad OF1.1 instructions: OFPBIC_BAD_LEN
0001 0010 01 000000 0000000000000000
-dnl Goto-Table not supported yet.
-# bad OF1.1 instructions: OFPBIC_UNSUP_INST
+dnl Goto-Table instruction non-zero padding
+# bad OF1.1 instructions: OFPBAC_BAD_ARGUMENT
+0001 0008 01 000001
+
+dnl Goto-Table 1
+# instructions=goto_table:1
0001 0008 01 000000
dnl Write-Metadata not supported yet.
@@ -292,7 +300,7 @@ dnl Write-Actions not supported yet.
0003 0008 01 000000
dnl Clear-Actions not supported yet.
-# bad OF1.1 instructions: OFPBIC_UNSUP_INST
+# instructions=clear_actions
0005 0008 01 000000
dnl Experimenter actions not supported yet.
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 386f7c8..8d9bcec 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1122,6 +1122,24 @@ with no match criteria. (This is why the default
\fBtable\fR is 1, to
keep the learned flows separate from the primary flow table 0.)
.RE
.
+.IP \fBinstructions=\fR[\fIinstruction\fR][\fB,\fIinstruction\fR...]\fR
+Specifies a comma-separated list of instructions to take on a packet
+when the flow entry matches. Instructions must be listed in specific order
+and can be specified at most once. Otherwise, it results in an error.
+If no target is specified, then packtes matching the flow are dropped.
+The \fIinstruction\fR is one of the following and must be specified in
+listed order.
+keywords:
+.
+.RS
+.IP \fBapply_actions(\fR[\fIaction\fR][\fB,\fIaction\fR...]\fB)
+Applies the specific action(s) immediately. The syntax of actions are same
+to \fBactions=\fR field.
+.
+.IP \fBgoto_table\fR:\fItable\fR
+Indicates the next table in the process pipeline.
+.RE
+.
.IP "\fBfin_timeout(\fIargument\fR[\fB,\fIargument\fR]\fB)"
This action changes the idle timeout or hard timeout, or both, of this
OpenFlow rule when the rule matches a TCP packet with the FIN or RST
--
1.7.1.1
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
