meta-flow: Accept NXM and OXM field names, support NXM and OXM for output.
authorBen Pfaff <blp@nicira.com>
Tue, 26 Jun 2012 17:52:34 +0000 (10:52 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 27 Jun 2012 05:02:52 +0000 (22:02 -0700)
This commit makes actions that accept NXM header values also accept OXM
header values and accept OXM field names where previously only NXM field
names were accepted.

This makes it possible to add new OXM fields that don't have NXM header
values, e.g. the OXM "metadata" field.

Inspired by Joe Stringer's patch:
http://openvswitch.org/pipermail/dev/2012-June/018344.html

Reported-by: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Ben Pfaff <blp@nicira.com>
lib/meta-flow.c
lib/meta-flow.h
tests/learn.at
tests/ovs-ofctl.at

index 719c0d94f66cd8e765f83598d50e6094d87fca23..e7b860b23bc2bd4876bce9025b2fff8b9606f010 100644 (file)
@@ -54,7 +54,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_NONE,
         true,
         NXM_NX_TUN_ID, "NXM_NX_TUN_ID",
-        0, NULL,
+        NXM_NX_TUN_ID, "NXM_NX_TUN_ID",
     }, {
         MFF_IN_PORT, "in_port", NULL,
         MF_FIELD_SIZES(be16),
@@ -74,9 +74,8 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFS_HEXADECIMAL,                        \
         MFP_NONE,                               \
         true,                                   \
-        NXM_NX_REG(IDX),                        \
-        "NXM_NX_REG" #IDX,                      \
-        0, NULL,                                \
+        NXM_NX_REG(IDX), "NXM_NX_REG" #IDX,     \
+        NXM_NX_REG(IDX), "NXM_NX_REG" #IDX,     \
     }
 #if FLOW_N_REGS > 0
     REGISTER(0),
@@ -147,7 +146,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_NONE,
         true,
         NXM_OF_VLAN_TCI, "NXM_OF_VLAN_TCI",
-        0, NULL,
+        NXM_OF_VLAN_TCI, "NXM_OF_VLAN_TCI",
     }, {
         MFF_VLAN_VID, "dl_vlan", NULL,
         sizeof(ovs_be16), 12,
@@ -155,7 +154,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFS_DECIMAL,
         MFP_NONE,
         true,
-        0, NULL,
+        OXM_OF_VLAN_VID, "OXM_OF_VLAN_VID",
         OXM_OF_VLAN_VID, "OXM_OF_VLAN_VID",
     }, {
         MFF_VLAN_PCP, "dl_vlan_pcp", NULL,
@@ -164,7 +163,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFS_DECIMAL,
         MFP_NONE,
         true,
-        0, NULL,
+        OXM_OF_VLAN_PCP, "OXM_OF_VLAN_PCP",
         OXM_OF_VLAN_PCP, "OXM_OF_VLAN_PCP",
     },
 
@@ -257,7 +256,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_IP_ANY,
         true,
         NXM_NX_IP_TTL, "NXM_NX_IP_TTL",
-        0, NULL,
+        NXM_NX_IP_TTL, "NXM_NX_IP_TTL",
     }, {
         MFF_IP_FRAG, "ip_frag", NULL,
         1, 2,
@@ -266,7 +265,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_IP_ANY,
         false,
         NXM_NX_IP_FRAG, "NXM_NX_IP_FRAG",
-        0, NULL,
+        NXM_NX_IP_FRAG, "NXM_NX_IP_FRAG",
     },
 
     {
@@ -434,19 +433,22 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
     }
 };
 
+/* Maps an NXM or OXM header value to an mf_field. */
 struct nxm_field {
-    struct hmap_node hmap_node;
-    uint32_t nxm_header;
+    struct hmap_node hmap_node; /* In 'all_fields' hmap. */
+    uint32_t header;            /* NXM or OXM header value. */
     const struct mf_field *mf;
 };
 
-static struct hmap all_nxm_fields = HMAP_INITIALIZER(&all_nxm_fields);
-static struct hmap all_oxm_fields = HMAP_INITIALIZER(&all_oxm_fields);
+/* Contains 'struct nxm_field's. */
+static struct hmap all_fields = HMAP_INITIALIZER(&all_fields);
 
 /* Rate limit for parse errors.  These always indicate a bug in an OpenFlow
  * controller and so there's not much point in showing a lot of them. */
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
+const struct mf_field *mf_from_nxm_header__(uint32_t header);
+
 /* Returns the field with the given 'id'. */
 const struct mf_field *
 mf_from_id(enum mf_field_id id)
@@ -477,54 +479,27 @@ mf_from_name(const char *name)
 }
 
 static void
-add_nxm_field(struct hmap *all_fields, uint32_t nxm_header,
-              const struct mf_field *mf)
+add_nxm_field(uint32_t header, const struct mf_field *mf)
 {
     struct nxm_field *f;
 
     f = xmalloc(sizeof *f);
-    hmap_insert(all_fields, &f->hmap_node, hash_int(nxm_header, 0));
-    f->nxm_header = nxm_header;
+    hmap_insert(&all_fields, &f->hmap_node, hash_int(header, 0));
+    f->header = header;
     f->mf = mf;
 }
 
-static struct hmap *
-get_all_fields(uint32_t header)
-{
-        return IS_OXM_HEADER(header) ? &all_oxm_fields : &all_nxm_fields;
-}
-
 static void
 nxm_init_add_field(const struct mf_field *mf, uint32_t header)
 {
-    struct hmap *all_fields = get_all_fields(header);
-
-    if (!header) {
-        return;
-    }
-    add_nxm_field(all_fields, header, mf);
-    if (mf->maskable == MFM_NONE) {
-        return;
-    }
-    add_nxm_field(all_fields, NXM_MAKE_WILD_HEADER(header), mf);
-}
-
-#ifndef NDEBUG
-static void
-nxm_init_verify_field(const struct mf_field *mf, uint32_t header)
-{
-    if (!header) {
-        return;
-    }
-    assert(mf_from_nxm_header(header) == mf);
-    /* Some OXM fields are not maskable while their NXM
-     * counterparts are, just skip this check for now */
-    if (mf->maskable == MFM_NONE || IS_OXM_HEADER(header)) {
-        return;
+    if (header) {
+        assert(!mf_from_nxm_header__(header));
+        add_nxm_field(header, mf);
+        if (mf->maskable != MFM_NONE) {
+            add_nxm_field(NXM_MAKE_WILD_HEADER(header), mf);
+        }
     }
-    assert(mf_from_nxm_header(NXM_MAKE_WILD_HEADER(mf->nxm_header)) == mf);
 }
-#endif
 
 static void
 nxm_init(void)
@@ -533,30 +508,28 @@ nxm_init(void)
 
     for (mf = mf_fields; mf < &mf_fields[MFF_N_IDS]; mf++) {
         nxm_init_add_field(mf, mf->nxm_header);
-        nxm_init_add_field(mf, mf->oxm_header);
-    }
-
-#ifndef NDEBUG
-    /* Verify that the header values are unique. */
-    for (mf = mf_fields; mf < &mf_fields[MFF_N_IDS]; mf++) {
-        nxm_init_verify_field(mf, mf->nxm_header);
-        nxm_init_verify_field(mf, mf->oxm_header);
+        if (mf->oxm_header != mf->nxm_header) {
+            nxm_init_add_field(mf, mf->oxm_header);
+        }
     }
-#endif
 }
 
 const struct mf_field *
 mf_from_nxm_header(uint32_t header)
 {
-    const struct nxm_field *f;
-    struct hmap *all_fields = get_all_fields(header);
-
-    if (hmap_is_empty(all_fields)) {
+    if (hmap_is_empty(&all_fields)) {
         nxm_init();
     }
+    return mf_from_nxm_header__(header);
+}
 
-    HMAP_FOR_EACH_IN_BUCKET (f, hmap_node, hash_int(header, 0), all_fields) {
-        if (f->nxm_header == header) {
+const struct mf_field *
+mf_from_nxm_header__(uint32_t header)
+{
+    const struct nxm_field *f;
+
+    HMAP_FOR_EACH_IN_BUCKET (f, hmap_node, hash_int(header, 0), &all_fields) {
+        if (f->header == header) {
             return f->mf;
         }
     }
@@ -2490,6 +2463,11 @@ mf_parse_subfield_name(const char *name, int name_len, bool *wild)
             && mf->nxm_name[name_len] == '\0') {
             return mf;
         }
+        if (mf->oxm_name
+            && !strncmp(mf->oxm_name, name, name_len)
+            && mf->oxm_name[name_len] == '\0') {
+            return mf;
+        }
     }
 
     return NULL;
index ffde5cc3ae926ce950861b06eafd19268b2fa6e7..82bb13f9fce95cc3909f70a210d58d80d615a6f1 100644 (file)
@@ -188,21 +188,30 @@ struct mf_field {
     enum mf_prereqs prereqs;
     bool writable;              /* May be written by actions? */
 
-    /* NXM properties.
+    /* NXM and OXM properties.
      *
-     * A few "mf_field"s don't correspond to NXM fields.  Those have 0 and
-     * NULL for the following members, respectively. */
-    uint32_t nxm_header;        /* An NXM_* constant (a few fields have 0). */
-    const char *nxm_name;       /* The "NXM_*" constant's name. */
-
-    /* OXM properties */
-    uint32_t oxm_header;        /* Field id in the OXM basic class,
-                                * an OXM_* constant.
-                                * Ignored if oxm_name is NULL */
-    const char *oxm_name;      /* The OXM_* constant's name,
-                                * NULL if the field is not present
-                                * in the OXM basic class */
-
+     * There are the following possibilities for these members for a given
+     * mf_field:
+     *
+     *   - Neither NXM nor OXM defines such a field: these members will all be
+     *     zero or NULL.
+     *
+     *   - NXM and OXM both define such a field: nxm_header and oxm_header will
+     *     both be nonzero and different, similarly for nxm_name and oxm_name.
+     *
+     *   - Only NXM or only OXM defines such a field: nxm_header and oxm_header
+     *     will both have the same value (either an OXM_* or NXM_* value) and
+     *     similarly for nxm_name and oxm_name.
+     *
+     * Thus, 'nxm_header' is the appropriate header to use when outputting an
+     * NXM formatted match, since it will be an NXM_* constant when possible
+     * for compatibility with OpenFlow implementations that expect that, with
+     * OXM_* constants used for fields that OXM adds.  Conversely, 'oxm_header'
+     * is the header to use when outputting an OXM formatted match. */
+    uint32_t nxm_header;        /* An NXM_* (or OXM_*) constant. */
+    const char *nxm_name;       /* The nxm_header constant's name. */
+    uint32_t oxm_header;        /* An OXM_* (or NXM_*) constant. */
+    const char *oxm_name;          /* The oxm_header constant's name */
 };
 
 /* The representation of a field's value. */
index 314baaf74568be0207ebfee6ed2f3cf317bac21c..9304861b7705f32ae0e19ca51c8d3120701d0b43 100644 (file)
@@ -37,7 +37,7 @@ AT_SETUP([learning action - satisfied prerequisites])
 AT_DATA([flows.txt],
 [[actions=learn(eth_type=0x800,load:5->NXM_OF_IP_DST[])
 ip,actions=learn(load:NXM_OF_IP_DST[]->NXM_NX_REG1[])
-ip,actions=learn(eth_type=0x800,NXM_OF_IP_DST[])
+ip,actions=learn(eth_type=0x800,OXM_OF_IPV4_DST[])
 ]])
 AT_CHECK([ovs-ofctl parse-flows flows.txt], [0],
 [[usable protocols: any
index a4fa7e4664d259ee84f799017d9f70b286170485..e45a4039cd5c37252e89797862605c8e24d58765 100644 (file)
@@ -181,6 +181,7 @@ actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note
 tun_id=0x1234,cookie=0x5678,actions=flood
 actions=drop
 reg0=123,actions=move:NXM_NX_REG0[0..5]->NXM_NX_REG1[26..31],load:55->NXM_NX_REG2[0..31],move:NXM_NX_REG0[0..31]->NXM_NX_TUN_ID[0..31],move:NXM_NX_REG0[0..15]->NXM_OF_VLAN_TCI[]
+actions=move:OXM_OF_ETH_DST[]->OXM_OF_ETH_SRC[]
 actions=autopath(5,NXM_NX_REG0[])
 vlan_tci=0x1123/0x1fff,actions=drop
 ]])
@@ -209,6 +210,7 @@ NXT_FLOW_MOD: ADD <any> actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06
 NXT_FLOW_MOD: ADD NXM_NX_TUN_ID(0000000000001234) cookie:0x5678 actions=FLOOD
 NXT_FLOW_MOD: ADD <any> actions=drop
 NXT_FLOW_MOD: ADD NXM_NX_REG0(0000007b) actions=move:NXM_NX_REG0[0..5]->NXM_NX_REG1[26..31],load:0x37->NXM_NX_REG2[],move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],move:NXM_NX_REG0[0..15]->NXM_OF_VLAN_TCI[]
+NXT_FLOW_MOD: ADD <any> actions=move:NXM_OF_ETH_DST[]->NXM_OF_ETH_SRC[]
 NXT_FLOW_MOD: ADD <any> actions=autopath(5,NXM_NX_REG0[])
 NXT_FLOW_MOD: ADD NXM_OF_VLAN_TCI_W(1123/1fff) actions=drop
 ]])