[ovs-dev] [PATCH] meta-flow: Accept NXM and OXM field names, support NXM and OXM for output.

Ben Pfaff blp at nicira.com
Tue Jun 26 19:24:06 UTC 2012


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 at wand.net.nz>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/meta-flow.c    |  106 ++++++++++++++++++++-------------------------------
 lib/meta-flow.h    |   37 +++++++++++-------
 tests/learn.at     |    2 +-
 tests/ovs-ofctl.at |    2 +
 4 files changed, 68 insertions(+), 79 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index b3a4bff..db690cf 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -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;
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index ffde5cc..82bb13f 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -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. */
diff --git a/tests/learn.at b/tests/learn.at
index 314baaf..9304861 100644
--- a/tests/learn.at
+++ b/tests/learn.at
@@ -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
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index a4fa7e4..e45a403 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -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
 ]])
-- 
1.7.2.5




More information about the dev mailing list