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 <koponen@nicira.com>
CC: Jeremy Stribling <strib@nicira.com>

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 3c2856f..d87915e 100644 (file)
--- 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 a9bc0c2..6b53f26 100644 (file)
--- 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 87ae185..4572db4 100644 (file)
--- 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 4d632ef..d7bc0ee 100644 (file)
--- 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 897429f..aec0f8f 100644 (file)
--- 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 de73e57..88d50a5 100644 (file)
--- 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 42c8ab5..7ffad2b 100644 (file)
--- 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