return 0;
}
-/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the
- * front of 'openflow' into ofpacts. On success, replaces any existing content
- * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'.
- * Returns 0 if successful, otherwise an OpenFlow error.
- *
- * Actions are processed according to their OpenFlow version which
- * is provided in the 'version' parameter.
- *
- * In most places in OpenFlow 1.1 and 1.2, actions appear encapsulated in
- * instructions, so you should call ofpacts_pull_openflow_instructions()
- * instead of this function.
- *
- * The parsed actions are valid generically, but they may not be valid in a
- * specific context. For example, port numbers up to OFPP_MAX are valid
- * generically, but specific datapaths may only support port numbers in a
- * smaller range. Use ofpacts_check() to additional check whether actions are
- * valid in a specific context. */
-enum ofperr
-ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
- unsigned int actions_len,
- enum ofp_version version,
- struct ofpbuf *ofpacts) {
+static enum ofperr
+ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
+ unsigned int actions_len,
+ enum ofp_version version,
+ uint32_t allowed_ovsinsts,
+ struct ofpbuf *ofpacts)
+{
const union ofp_action *actions;
enum ofperr error;
return error;
}
- error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts));
+ error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts),
+ allowed_ovsinsts);
if (error) {
ofpbuf_clear(ofpacts);
}
return error;
}
+/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the
+ * front of 'openflow' into ofpacts. On success, replaces any existing content
+ * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'.
+ * Returns 0 if successful, otherwise an OpenFlow error.
+ *
+ * Actions are processed according to their OpenFlow version which
+ * is provided in the 'version' parameter.
+ *
+ * In most places in OpenFlow, actions appear encapsulated in instructions, so
+ * you should call ofpacts_pull_openflow_instructions() instead of this
+ * function.
+ *
+ * The parsed actions are valid generically, but they may not be valid in a
+ * specific context. For example, port numbers up to OFPP_MAX are valid
+ * generically, but specific datapaths may only support port numbers in a
+ * smaller range. Use ofpacts_check() to additional check whether actions are
+ * valid in a specific context. */
+enum ofperr
+ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
+ unsigned int actions_len,
+ enum ofp_version version,
+ struct ofpbuf *ofpacts)
+{
+ return ofpacts_pull_openflow_actions__(openflow, actions_len, version,
+ 1u << OVSINST_OFPIT11_APPLY_ACTIONS,
+ ofpacts);
+}
\f
/* OpenFlow 1.1 actions. */
const struct ofp11_instruction *insts[N_OVS_INSTRUCTIONS];
enum ofperr error;
+ if (version == OFP10_VERSION) {
+ return ofpacts_pull_openflow_actions__(openflow, instructions_len,
+ version,
+ (1u << N_OVS_INSTRUCTIONS) - 1,
+ ofpacts);
+ }
+
ofpbuf_clear(ofpacts);
if (instructions_len % OFP11_INSTRUCTION_ALIGN != 0) {
ogt->table_id = oigt->table_id;
}
- error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts));
+ error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts),
+ (1u << N_OVS_INSTRUCTIONS) - 1);
exit:
if (error) {
ofpbuf_clear(ofpacts);
/* Verifies that the 'ofpacts_len' bytes of actions in 'ofpacts' are
* in the appropriate order as defined by the OpenFlow spec. */
enum ofperr
-ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len)
+ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len,
+ uint32_t allowed_ovsinsts)
{
const struct ofpact *a;
enum ovs_instruction_type inst;
}
return OFPERR_OFPBAC_UNSUPPORTED_ORDER;
}
+ if (!((1u << next) & allowed_ovsinsts)) {
+ const char *name = ovs_instruction_name_from_type(next);
+
+ VLOG_WARN("%s instruction not allowed here", name);
+ return OFPERR_OFPBIC_UNSUP_INST;
+ }
inst = next;
}
{
const struct ofpact *a;
- ovs_assert(ofp_version >= OFP11_VERSION);
+ if (ofp_version == OFP10_VERSION) {
+ ofpacts_put_openflow_actions(ofpacts, ofpacts_len, openflow,
+ ofp_version);
+ return;
+ }
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
switch (ovs_instruction_type_from_ofpact_type(a->type)) {
struct flow *, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables,
enum ofputil_protocol usable_protocols);
-enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len);
+enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len,
+ uint32_t allowed_ovsinsts);
enum ofperr ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports);
/* Converting ofpacts to OpenFlow. */
/* OF1.1+(3,0). Unknown instruction. */
OFPERR_OFPBIC_UNKNOWN_INST,
- /* OF1.1+(3,1). Switch or table does not support the instruction. */
+ /* NX1.0(2,257), OF1.1+(3,1). Switch or table does not support the
+ * instruction. */
OFPERR_OFPBIC_UNSUP_INST,
/* OF1.1+(3,2). Invalid Table-ID specified. */
return error_s;
}
- error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts));
+ error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts),
+ (1u << OVSINST_OFPIT11_APPLY_ACTIONS));
if (error) {
ofpbuf_set_size(ofpacts, orig_size);
return xstrdup("Incorrect action ordering");
/* If write_metadata is specified as an action AND an instruction, ofpacts
could be invalid. */
- error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts));
+ error = ofpacts_verify(ofpbuf_data(ofpacts), ofpbuf_size(ofpacts),
+ (1u << N_OVS_INSTRUCTIONS) - 1);
if (error) {
return xstrdup("Incorrect instruction ordering");
}
return error;
}
- error = ofpacts_pull_openflow_instructions(&b, ofpbuf_size(&b), oh->version,
- ofpacts);
- if (error) {
- return error;
- }
-
/* Translate the message. */
fm->priority = ntohs(ofm->priority);
if (ofm->command == OFPFC_ADD
ofputil_match_from_ofp10_match(&ofm->match, &fm->match);
ofputil_normalize_match(&fm->match);
- /* Now get the actions. */
- error = ofpacts_pull_openflow_actions(&b, ofpbuf_size(&b), oh->version,
- ofpacts);
- if (error) {
- return error;
- }
-
/* OpenFlow 1.0 says that exact-match rules have to have the
* highest possible priority. */
fm->priority = (ofm->match.wildcards & htonl(OFPFW10_ALL)
if (error) {
return error;
}
- error = ofpacts_pull_openflow_actions(&b, ofpbuf_size(&b), oh->version,
- ofpacts);
- if (error) {
- return error;
- }
/* Translate the message. */
command = ntohs(nfm->command);
}
}
+ error = ofpacts_pull_openflow_instructions(&b, ofpbuf_size(&b),
+ oh->version, ofpacts);
+ if (error) {
+ return error;
+ }
fm->ofpacts = ofpbuf_data(ofpacts);
fm->ofpacts_len = ofpbuf_size(ofpacts);
struct ofpbuf *ofpacts)
{
const struct ofp_header *oh;
+ size_t instructions_len;
enum ofperr error;
enum ofpraw raw;
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad match");
return EINVAL;
}
-
- if (ofpacts_pull_openflow_instructions(msg, length - sizeof *ofs -
- padded_match_len, oh->version,
- ofpacts)) {
- VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
- return EINVAL;
- }
+ instructions_len = length - sizeof *ofs - padded_match_len;
fs->priority = ntohs(ofs->priority);
fs->table_id = ofs->table_id;
"length %"PRIuSIZE, length);
return EINVAL;
}
-
- if (ofpacts_pull_openflow_actions(msg, length - sizeof *ofs,
- oh->version, ofpacts)) {
- return EINVAL;
- }
+ instructions_len = length - sizeof *ofs;
fs->cookie = get_32aligned_be64(&ofs->cookie);
ofputil_match_from_ofp10_match(&ofs->match, &fs->match);
fs->flags = 0;
} else if (raw == OFPRAW_NXST_FLOW_REPLY) {
const struct nx_flow_stats *nfs;
- size_t match_len, actions_len, length;
+ size_t match_len, length;
nfs = ofpbuf_try_pull(msg, sizeof *nfs);
if (!nfs) {
if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL)) {
return EINVAL;
}
-
- actions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
- if (ofpacts_pull_openflow_actions(msg, actions_len, oh->version,
- ofpacts)) {
- return EINVAL;
- }
+ instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
fs->cookie = nfs->cookie;
fs->table_id = nfs->table_id;
OVS_NOT_REACHED();
}
+ if (ofpacts_pull_openflow_instructions(msg, instructions_len, oh->version,
+ ofpacts)) {
+ VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
+ return EINVAL;
+ }
fs->ofpacts = ofpbuf_data(ofpacts);
fs->ofpacts_len = ofpbuf_size(ofpacts);
# actions=set_tunnel64:0x885f3298
ffff 0018 00002320 0009 000000000000 00000000885f3298
-# actions=write_metadata:0xfedcba9876543210
+# bad OF1.0 actions: OFPBIC_UNSUP_INST
+& ofp_actions|WARN|write_metadata instruction not allowed here
ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff
-# actions=write_metadata:0xfedcba9876543210/0xffff0000ffff0000
+# bad OF1.0 actions: OFPBIC_UNSUP_INST
+& ofp_actions|WARN|write_metadata instruction not allowed here
ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffff0000ffff0000
# actions=multipath(eth_src,50,modulo_n,1,0,NXM_NX_REG0[])
[0], [expout], [experr])
AT_CLEANUP
+AT_SETUP([OpenFlow 1.0 "instruction" translations])
+AT_KEYWORDS([ofp-actions OF1.0 instruction])
+AT_DATA([test-data], [dnl
+dnl Try a couple of ordinary actions to make sure they're accepted,
+dnl but there's no point in retrying all the actions from the previous test.
+# actions=LOCAL
+0000 0008 fffe 04d2
+
+# actions=mod_dl_src:00:11:22:33:44:55
+0004 0010 001122334455 000000000000
+
+dnl Now check that write_metadata is accepted.
+# actions=write_metadata:0xfedcba9876543210
+ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff
+
+# actions=write_metadata:0xfedcba9876543210/0xffff0000ffff0000
+ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffff0000ffff0000
+
+])
+sed '/^[[#&]]/d' < test-data > input.txt
+sed -n 's/^# //p; /^$/p' < test-data > expout
+sed -n 's/^& //p' < test-data > experr
+AT_CAPTURE_FILE([input.txt])
+AT_CAPTURE_FILE([expout])
+AT_CAPTURE_FILE([experr])
+AT_CHECK(
+ [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-ofp10-instructions < input.txt],
+ [0], [expout], [experr])
+AT_CLEANUP
+
AT_SETUP([OpenFlow 1.1 action translation])
AT_KEYWORDS([ofp-actions OF1.1])
AT_DATA([test-data], [dnl
# actions=set_tunnel64:0x885f3298
ffff 0018 00002320 0009 000000000000 00000000885f3298
-dnl OpenFlow 1.1 uses OFPIT_WRITE_METADATA to express the NXAST_WRITE_METADATA
-dnl action instead, so parse-ofp11-actions will recognise and drop this action.
-# actions=write_metadata:0xfedcba9876543210
-# 0: ff -> (none)
-# 1: ff -> (none)
-# 2: 00 -> (none)
-# 3: 20 -> (none)
-# 4: 00 -> (none)
-# 5: 00 -> (none)
-# 6: 23 -> (none)
-# 7: 20 -> (none)
-# 8: 00 -> (none)
-# 9: 16 -> (none)
-# 10: 00 -> (none)
-# 11: 00 -> (none)
-# 12: 00 -> (none)
-# 13: 00 -> (none)
-# 14: 00 -> (none)
-# 15: 00 -> (none)
-# 16: fe -> (none)
-# 17: dc -> (none)
-# 18: ba -> (none)
-# 19: 98 -> (none)
-# 20: 76 -> (none)
-# 21: 54 -> (none)
-# 22: 32 -> (none)
-# 23: 10 -> (none)
-# 24: ff -> (none)
-# 25: ff -> (none)
-# 26: ff -> (none)
-# 27: ff -> (none)
-# 28: ff -> (none)
-# 29: ff -> (none)
-# 30: ff -> (none)
-# 31: ff -> (none)
+dnl Write-Metadata is only allowed in contexts that allow instructions.
+& ofp_actions|WARN|write_metadata instruction not allowed here
+# bad OF1.1 actions: OFPBIC_UNSUP_INST
ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff
-dnl Write-Metadata duplicated.
-& ofp_actions|WARN|duplicate write_metadata instruction not allowed, for OpenFlow 1.1+ compatibility
-# bad OF1.1 actions: OFPBAC_UNSUPPORTED_ORDER
-ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff
-
-dnl Write-Metadata in wrong position.
-& ofp_actions|WARN|invalid instruction ordering: apply_actions must appear before write_metadata, for OpenFlow 1.1+ compatibility
-# bad OF1.1 actions: OFPBAC_UNSUPPORTED_ORDER
-ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff ffff 0010 00002320 0002 0000 12345678
-
# actions=multipath(eth_src,50,modulo_n,1,0,NXM_NX_REG0[])
ffff 0020 00002320 000a 0000 0032 0000 0000 0000 0000 0000 0000 001f 00010004
# actions=write_metadata:0xfedcba9876543210
0002 0018 00000000 fedcba9876543210 ffffffffffffffff
+dnl Write-Metadata as Nicira extension action is transformed into instruction.
+# actions=write_metadata:0xfedcba9876543210
+# 1: 04 -> 02
+# 3: 28 -> 18
+# 8: ff -> fe
+# 9: ff -> dc
+# 10: 00 -> ba
+# 11: 20 -> 98
+# 12: 00 -> 76
+# 13: 00 -> 54
+# 14: 23 -> 32
+# 15: 20 -> 10
+# 16: 00 -> ff
+# 17: 16 -> ff
+# 18: 00 -> ff
+# 19: 00 -> ff
+# 20: 00 -> ff
+# 21: 00 -> ff
+# 22: 00 -> ff
+# 23: 00 -> ff
+# 24: fe -> (none)
+# 25: dc -> (none)
+# 26: ba -> (none)
+# 27: 98 -> (none)
+# 28: 76 -> (none)
+# 29: 54 -> (none)
+# 30: 32 -> (none)
+# 31: 10 -> (none)
+# 32: ff -> (none)
+# 33: ff -> (none)
+# 34: ff -> (none)
+# 35: ff -> (none)
+# 36: ff -> (none)
+# 37: ff -> (none)
+# 38: ff -> (none)
+# 39: ff -> (none)
+0004 0028 00000000 ffff 0020 00002320 0016 000000000000 fedcba9876543210 ffffffffffffffff
+
dnl Write-Metadata with mask.
# actions=write_metadata:0xfedcba9876543210/0xff00ff00ff00ff00
0002 0018 00000000 fedcba9876543210 ff00ff00ff00ff00
}
}
-/* "parse-ofp10-actions": reads a series of OpenFlow 1.0 action specifications
- * as hex bytes from stdin, converts them to ofpacts, prints them as strings
- * on stdout, and then converts them back to hex bytes and prints any
- * differences from the input. */
static void
-ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+ofctl_parse_ofp10_actions__(bool instructions)
{
struct ds in;
/* Convert to ofpacts. */
ofpbuf_init(&ofpacts, 0);
size = ofpbuf_size(&of10_in);
- error = ofpacts_pull_openflow_actions(&of10_in, ofpbuf_size(&of10_in),
- OFP10_VERSION, &ofpacts);
+ error = (instructions
+ ? ofpacts_pull_openflow_instructions
+ : ofpacts_pull_openflow_actions)(
+ &of10_in, ofpbuf_size(&of10_in), OFP10_VERSION, &ofpacts);
if (error) {
printf("bad OF1.0 actions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);
ds_destroy(&in);
}
+/* "parse-ofp10-actions": reads a series of OpenFlow 1.0 action specifications
+ * as hex bytes from stdin, converts them to ofpacts, prints them as strings
+ * on stdout, and then converts them back to hex bytes and prints any
+ * differences from the input. */
+static void
+ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+{
+ ofctl_parse_ofp10_actions__(false);
+}
+
+/* "parse-ofp10-instructions": reads a series of OpenFlow 1.0 action
+ * specifications as hex bytes from stdin, converts them to ofpacts, prints
+ * them as strings on stdout, and then converts them back to hex bytes and
+ * prints any differences from the input. */
+static void
+ofctl_parse_ofp10_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+{
+ ofctl_parse_ofp10_actions__(true);
+}
+
/* "parse-ofp10-match": reads a series of ofp10_match specifications as hex
* bytes from stdin, converts them to cls_rules, prints them as strings on
* stdout, and then converts them back to hex bytes and prints any differences
{ "parse-nxm", 0, 0, ofctl_parse_nxm },
{ "parse-oxm", 1, 1, ofctl_parse_oxm },
{ "parse-ofp10-actions", 0, 0, ofctl_parse_ofp10_actions },
+ { "parse-ofp10-instructions", 0, 0, ofctl_parse_ofp10_instructions },
{ "parse-ofp10-match", 0, 0, ofctl_parse_ofp10_match },
{ "parse-ofp11-match", 0, 0, ofctl_parse_ofp11_match },
{ "parse-ofp11-actions", 0, 0, ofctl_parse_ofp11_actions },