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 3c2856f9e3949e8b5f4085221759d63a68544e1a..d87915e213a29196996d2ae5a63a1e4999f4c5c4 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 a9bc0c2d7a276578a9aacc1ae62c702c5ec19123..6b53f2655179d841d8a0e80c29c5aef2de4a2fdf 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 87ae185e5b207b65a330b9f6b6c1f96beade1878..4572db4440d55589e8fb3dbdd178201076d0c21a 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 4d632efecd47c2691817ff0c81e1d338f542381a..d7bc0ee450d43b4465f40f480a354c917f547de1 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 897429f84cf11a0667d1e230c72ee3b850c1585c..aec0f8fa191f90fabae78f05f7005559775f952c 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 de73e571ebf59c107da60ad0389e754680875522..88d50a507c4335396f3b126b3c223a41f517168c 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 42c8ab5b3e31ba062efd3052a455d880b1416ba5..7ffad2b6b5e31a7227638209c0356af778251ed5 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