From 96fc46e8fdafd6467906e11e0fb493e2b78f2fb5 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 12 Nov 2010 16:23:26 -0800 Subject: [PATCH] nicira-ext: New Nicira vendor action NXAST_NOTE. Our controller group at Nicira has requested a way to annotate flows with extra information beyond the flow cookie. The new NXAST_NOTE action provides such a way. This new action is somewhat controversial. Some have suggested that it should be added another way (either as part of the Nicira Extended Match or as a new component of the flow_mod and related messages). Others think that it has no place in the OpenFlow protocol at all and that an equivalent should be implemented using the already available features of OVSDB. So it is possible that this extension will be deleted and the feature will be reimplemented some other way (or not at all). CC: Teemu Koponen CC: Jeremy Stribling --- include/openflow/nicira-ext.h | 19 +++++++++++++++++ lib/ofp-parse.c | 40 +++++++++++++++++++++++++++++++++++ lib/ofp-print.c | 20 ++++++++++++++++++ lib/ofp-util.c | 6 ++++++ ofproto/ofproto.c | 3 +++ tests/ovs-ofctl.at | 2 ++ utilities/ovs-ofctl.8.in | 4 ++++ 7 files changed, 94 insertions(+) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 3c2856f9..d87915e2 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -230,6 +230,7 @@ enum nx_action_subtype { NXAST_POP_QUEUE, /* struct nx_action_pop_queue */ NXAST_REG_MOVE, /* struct nx_action_reg_move */ NXAST_REG_LOAD, /* struct nx_action_reg_load */ + NXAST_NOTE /* struct nx_action_note */ }; /* Header for Nicira-defined actions. */ @@ -442,6 +443,24 @@ struct nx_action_reg_load { }; OFP_ASSERT(sizeof(struct nx_action_reg_load) == 24); +/* Action structure for NXAST_NOTE. + * + * This action has no effect. It is variable length. The switch does not + * attempt to interpret the user-defined 'note' data in any way. A controller + * can use this action to attach arbitrary metadata to a flow. + * + * This action might go away in the future. + */ +struct nx_action_note { + uint16_t type; /* OFPAT_VENDOR. */ + uint16_t len; /* A multiple of 8, but at least 16. */ + uint32_t vendor; /* NX_VENDOR_ID. */ + uint16_t subtype; /* NXAST_NOTE. */ + uint8_t note[6]; /* Start of user-defined data. */ + /* Possibly followed by additional user-defined data. */ +}; +OFP_ASSERT(sizeof(struct nx_action_note) == 16); + /* Wildcard for tunnel ID. */ #define NXFW_TUN_ID (1 << 25) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index a9bc0c2d..6b53f265 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -277,6 +277,46 @@ str_to_action(char *str, struct ofpbuf *b) nah = put_action(b, sizeof *nah, OFPAT_VENDOR); nah->vendor = htonl(NX_VENDOR_ID); nah->subtype = htons(NXAST_POP_QUEUE); + } else if (!strcasecmp(act, "note")) { + size_t start_ofs = b->size; + struct nx_action_note *nan; + int remainder; + size_t len; + + nan = put_action(b, sizeof *nan, OFPAT_VENDOR); + nan->vendor = htonl(NX_VENDOR_ID); + nan->subtype = htons(NXAST_NOTE); + + b->size -= sizeof nan->note; + while (arg && *arg != '\0') { + int high, low; + uint8_t byte; + + if (*arg == '.') { + arg++; + } + if (*arg == '\0') { + break; + } + + high = hexit_value(*arg++); + if (high >= 0) { + low = hexit_value(*arg++); + } + if (high < 0 || low < 0) { + ovs_fatal(0, "bad hex digit in `note' argument"); + } + + byte = high * 16 + low; + ofpbuf_put(b, &byte, 1); + } + + len = b->size - start_ofs; + remainder = len % OFP_ACTION_ALIGN; + if (remainder) { + ofpbuf_put_zeros(b, OFP_ACTION_ALIGN - remainder); + } + nan->len = htons(b->size - start_ofs); } else if (!strcasecmp(act, "output")) { put_output_action(b, str_to_u32(arg)); } else if (!strcasecmp(act, "enqueue")) { diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 87ae185e..4572db44 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -184,6 +184,22 @@ static void ofp_print_port_name(struct ds *string, uint16_t port) ds_put_cstr(string, name); } +static void +print_note(struct ds *string, const struct nx_action_note *nan) +{ + size_t len; + size_t i; + + ds_put_cstr(string, "note:"); + len = ntohs(nan->len) - offsetof(struct nx_action_note, note); + for (i = 0; i < len; i++) { + if (i) { + ds_put_char(string, '.'); + } + ds_put_format(string, "%02"PRIx8, nan->note[i]); + } +} + static void ofp_print_nx_action(struct ds *string, const struct nx_action_header *nah) { @@ -217,6 +233,10 @@ ofp_print_nx_action(struct ds *string, const struct nx_action_header *nah) ds_put_cstr(string, "pop_queue"); break; + case NXAST_NOTE: + print_note(string, (const struct nx_action_note *) nah); + break; + default: ds_put_format(string, "***unknown Nicira action:%d***", ntohs(nah->subtype)); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 4d632efe..d7bc0ee4 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -545,6 +545,7 @@ check_nicira_action(const union ofp_action *a, unsigned int len, case NXAST_SET_QUEUE: case NXAST_POP_QUEUE: return check_action_exact_len(a, len, 16); + case NXAST_REG_MOVE: error = check_action_exact_len(a, len, sizeof(struct nx_action_reg_move)); @@ -552,6 +553,7 @@ check_nicira_action(const union ofp_action *a, unsigned int len, return error; } return nxm_check_reg_move((const struct nx_action_reg_move *) a, flow); + case NXAST_REG_LOAD: error = check_action_exact_len(a, len, sizeof(struct nx_action_reg_load)); @@ -559,6 +561,10 @@ check_nicira_action(const union ofp_action *a, unsigned int len, return error; } return nxm_check_reg_load((const struct nx_action_reg_load *) a, flow); + + case NXAST_NOTE: + return 0; + default: return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE); } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 897429f8..aec0f8fa 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2859,6 +2859,9 @@ xlate_nicira_action(struct action_xlate_ctx *ctx, case NXAST_REG_LOAD: nxm_execute_reg_load((const struct nx_action_reg_load *) nah, &ctx->flow); + + case NXAST_NOTE: + /* Nothing to do. */ break; /* If you add a new action here that modifies flow data, don't forget to diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index de73e571..88d50a50 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -10,6 +10,7 @@ udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0 tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:1 udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1 cookie=0x123456789abcdef hard_timeout=10 priority=60000 actions=controller +actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note actions=drop ]) AT_CHECK([ovs-ofctl parse-flows flows.txt], [0], [stdout]) @@ -21,6 +22,7 @@ flow_mod: udp,dl_vlan_pcp=7, ADD: idle:5 actions=strip_vlan,output:0 flow_mod: tcp,nw_src=192.168.0.3,tp_dst=80, ADD: actions=set_queue:37,output:1 flow_mod: udp,nw_src=192.168.0.3,tp_dst=53, ADD: actions=pop_queue,output:1 flow_mod: ADD: cookie:0x123456789abcdef hard:10 pri:60000 actions=CONTROLLER:65535 +flow_mod: ADD: actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.00.00.00.00.00.00,note:00.00.00.00.00.00 flow_mod: ADD: actions=drop ]) AT_CLEANUP diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 42c8ab5b..7ffad2b6 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -479,6 +479,10 @@ OpenFlow implementations do not support queuing at all. Restores the queue to the value it was before any \fBset_queue\fR actions were applied. . +.IP \fBnote:\fR[\fIhh\fR]... +Does nothing at all. Any number of bytes represented as hex digits +\fIhh\fR may be included. Pairs of hex digits may be separated by +periods for readability. .RE . .IP -- 2.30.2