diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 719c0d94f..e7b860b23 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,55 +479,28 @@ 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; + 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); + } } - 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; - } - 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); + if (mf->oxm_header != 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); - } -#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 ffde5cc3a..82bb13f9f 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 314baaf74..9304861b7 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 a4fa7e466..e45a4039c 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 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 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 actions=move:NXM_OF_ETH_DST[]->NXM_OF_ETH_SRC[] NXT_FLOW_MOD: ADD actions=autopath(5,NXM_NX_REG0[]) NXT_FLOW_MOD: ADD NXM_OF_VLAN_TCI_W(1123/1fff) actions=drop ]])