This helps get rid of one special case in nx_pull_raw() and allows
loading of 32-bit values to OXM_OF_IN_PORT in NXAST_LEARN actions.
Previously the 16-bit limit acted the same on both NXM_OF_IN_PORT and
OXM_OF_IN_PORT, even though OF1.1+ controllers would expect OXM_OF_IN_PORT
to be 32 bits wide.

Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com>
---
I considered simply making the MFF_IN_PORT 32-bit instead, but that could
have resulted in surprises for any existing use expecting it to be 16 bits
(such as register load actions). Maybe there is no such conflict, and indeed
it would be better to just widen the MFF_IN_PORT and add a special case for
NXM_OF_IN_PORT into nx_pull_raw().

 lib/meta-flow.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/meta-flow.h |    2 ++
 lib/nx-match.c  |   13 +----------
 tests/learn.at  |   22 ++++++++++++++++++
 4 files changed, 94 insertions(+), 12 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index e5809d5..f8f02ed 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -116,6 +116,15 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_NONE,
         true,
         NXM_OF_IN_PORT, "NXM_OF_IN_PORT",
+        NXM_OF_IN_PORT, "NXM_OF_IN_PORT",
+    }, {
+        MFF_IN_PORT_OXM, "in_port_oxm", NULL,
+        MF_FIELD_SIZES(be32),
+        MFM_NONE,
+        MFS_OFP_PORT_OXM,
+        MFP_NONE,
+        true,
+        OXM_OF_IN_PORT, "OXM_OF_IN_PORT",
         OXM_OF_IN_PORT, "OXM_OF_IN_PORT",
     }, {
         MFF_SKB_PRIORITY, "skb_priority", NULL,
@@ -686,6 +695,7 @@ mf_is_all_wild(const struct mf_field *mf, const struct 
flow_wildcards *wc)
     case MFF_METADATA:
         return !wc->masks.metadata;
     case MFF_IN_PORT:
+    case MFF_IN_PORT_OXM:
         return !wc->masks.in_port;
     case MFF_SKB_PRIORITY:
         return !wc->masks.skb_priority;
@@ -924,6 +934,11 @@ mf_is_value_valid(const struct mf_field *mf, const union 
mf_value *value)
     case MFF_ND_TLL:
         return true;
 
+    case MFF_IN_PORT_OXM: {
+        uint32_t port = ntohl(value->be32);
+        return port < OFPP_MAX || port >= OFPP11_MAX;
+    }
+
     case MFF_IP_DSCP:
         return !(value->u8 & ~IP_DSCP_MASK);
     case MFF_IP_DSCP_SHIFTED:
@@ -997,6 +1012,10 @@ mf_get_value(const struct mf_field *mf, const struct flow 
*flow,
         value->be16 = htons(flow->in_port);
         break;
 
+    case MFF_IN_PORT_OXM:
+        value->be32 = ofputil_port_to_ofp11(flow->in_port);
+        break;
+
     case MFF_SKB_PRIORITY:
         value->be32 = htonl(flow->skb_priority);
         break;
@@ -1145,6 +1164,16 @@ mf_get_value(const struct mf_field *mf, const struct 
flow *flow,
     }
 }
 
+/* Map a 32 port number to 16-bit range. */
+static inline uint16_t
+port_from_ofp11(ovs_be32 ofp11_port)
+{
+  uint32_t port = ntohl(ofp11_port);
+  return port < OFPP_MAX ? port
+    : port >= OFPP11_MAX ? port - OFPP11_OFFSET
+    : OFPP_NONE;
+}
+
 /* Makes 'match' match field 'mf' exactly, with the value matched taken from
  * 'value'.  The caller is responsible for ensuring that 'match' meets 'mf''s
  * prerequisites. */
@@ -1180,6 +1209,10 @@ mf_set_value(const struct mf_field *mf,
         match_set_in_port(match, ntohs(value->be16));
         break;
 
+    case MFF_IN_PORT_OXM:
+        match_set_in_port(match, port_from_ofp11(value->be32));
+        break;
+
     case MFF_SKB_PRIORITY:
         match_set_skb_priority(match, ntohl(value->be32));
         break;
@@ -1363,6 +1396,10 @@ mf_set_flow_value(const struct mf_field *mf,
         flow->in_port = ntohs(value->be16);
         break;
 
+    case MFF_IN_PORT_OXM:
+        flow->in_port = port_from_ofp11(value->be32);
+        break;
+
     case MFF_SKB_PRIORITY:
         flow->skb_priority = ntohl(value->be32);
         break;
@@ -1559,6 +1596,7 @@ mf_set_wild(const struct mf_field *mf, struct match 
*match)
         break;
 
     case MFF_IN_PORT:
+    case MFF_IN_PORT_OXM:
         match->flow.in_port = 0;
         match->wc.masks.in_port = 0;
         break;
@@ -1740,6 +1778,7 @@ mf_set(const struct mf_field *mf,
 
     switch (mf->id) {
     case MFF_IN_PORT:
+    case MFF_IN_PORT_OXM:
     case MFF_SKB_MARK:
     case MFF_SKB_PRIORITY:
     case MFF_ETH_TYPE:
@@ -1975,6 +2014,12 @@ mf_random_value(const struct mf_field *mf, union 
mf_value *value)
     case MFF_ND_TLL:
         break;
 
+    case MFF_IN_PORT_OXM:
+        value->be32 &= htonl(0x0000ffff);
+       if (ntohl(value->be32) > OFPP_MAX)
+         value->be32 |= htonl(0xffff0000);
+        break;
+
     case MFF_IPV6_LABEL:
         value->be32 &= ~htonl(IPV6_LABEL_MASK);
         break;
@@ -2175,6 +2220,21 @@ mf_from_ofp_port_string(const struct mf_field *mf, const 
char *s,
     return xasprintf("%s: port value out of range for %s", s, mf->name);
 }
 
+static char *
+mf_from_ofp_port_string32(const struct mf_field *mf, const char *s,
+                          ovs_be32 *valuep, ovs_be32 *maskp)
+{
+    uint16_t port;
+
+    ovs_assert(mf->n_bytes == sizeof(ovs_be32));
+    if (ofputil_port_from_string(s, &port)) {
+        *valuep = ofputil_port_to_ofp11(port);
+        *maskp = htonl(UINT32_MAX);
+        return NULL;
+    }
+    return xasprintf("%s: port value out of range for %s", s, mf->name);
+}
+
 struct frag_handling {
     const char *name;
     uint8_t mask;
@@ -2312,6 +2372,9 @@ mf_parse(const struct mf_field *mf, const char *s,
     case MFS_OFP_PORT:
         return mf_from_ofp_port_string(mf, s, &value->be16, &mask->be16);
 
+    case MFS_OFP_PORT_OXM:
+        return mf_from_ofp_port_string32(mf, s, &value->be32, &mask->be32);
+
     case MFS_FRAG:
         return mf_from_frag_string(s, &value->u8, &mask->u8);
 
@@ -2415,6 +2478,12 @@ mf_format(const struct mf_field *mf,
     }
 
     switch (mf->string) {
+    case MFS_OFP_PORT_OXM:
+        if (!mask) {
+            ofputil_format_port(port_from_ofp11(value->be32), s);
+            break;
+        }
+        /* fall through */
     case MFS_OFP_PORT:
         if (!mask) {
             ofputil_format_port(ntohs(value->be16), s);
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 9577a10..54fe4ef 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -39,6 +39,7 @@ enum mf_field_id {
     MFF_TUN_TOS,                /* u8 */
     MFF_METADATA,               /* be64 */
     MFF_IN_PORT,                /* be16 */
+    MFF_IN_PORT_OXM,            /* be32 */
     MFF_SKB_PRIORITY,           /* be32 */
     MFF_SKB_MARK,               /* be32 */
 
@@ -220,6 +221,7 @@ enum mf_string {
     MFS_IPV4,
     MFS_IPV6,
     MFS_OFP_PORT,               /* An OpenFlow port number or name. */
+    MFS_OFP_PORT_OXM,           /* An OpenFlow port number or name (32_bit). */
     MFS_FRAG,                   /* no, yes, first, later, not_later */
     MFS_TNL_FLAGS,              /* FLOW_TNL_F_* flags */
 };
diff --git a/lib/nx-match.c b/lib/nx-match.c
index b328f02..789b12a 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -148,7 +148,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool 
strict,
             error = OFPERR_OFPBMC_BAD_PREREQ;
         } else if (!mf_is_all_wild(mf, &match->wc)) {
             error = OFPERR_OFPBMC_DUP_FIELD;
-        } else if (header != OXM_OF_IN_PORT) {
+        } else {
             unsigned int width = mf->n_bytes;
             union mf_value value;
 
@@ -170,17 +170,6 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool 
strict,
                     mf_set(mf, &value, &mask, match);
                 }
             }
-        } else {
-            /* Special case for 32bit ports when using OXM,
-             * ports are 16 bits wide otherwise. */
-            ovs_be32 port_of11;
-            uint16_t port;
-
-            memcpy(&port_of11, p + 4, sizeof port_of11);
-            error = ofputil_port_from_ofp11(port_of11, &port);
-            if (!error) {
-                match_set_in_port(match, port);
-            }
         }
 
         /* Check if the match is for a cookie rather than a classifier rule. */
diff --git a/tests/learn.at b/tests/learn.at
index 5eb3e5d..964d21d 100644
--- a/tests/learn.at
+++ b/tests/learn.at
@@ -15,6 +15,28 @@ OFPT_FLOW_MOD (xid=0x3): ADD 
actions=learn(table=1,idle_timeout=10,hard_timeout=
 ]])
 AT_CLEANUP
 
+AT_SETUP([learning action - parsing and formatting - illegal in_port_oxm])
+AT_CHECK([[ovs-ofctl parse-flow 'actions=learn(table=1, in_port_oxm=123456)']],
+  [1], [], [stderr])
+AT_CHECK([sed -e 's/.*|ofp_util|WARN|//' < stderr], [0],
+  [[port 123456 is outside the supported range 0 through ffff or 0xffffff00 
through 0xffffffff
+ovs-ofctl: 123456: port value out of range for in_port_oxm
+]], [[]])
+AT_CLEANUP
+
+AT_SETUP([learning action - parsing and formatting - OXM])
+AT_DATA([flows.txt], [[
+actions=learn(output:OXM_OF_IN_PORT[])
+actions=learn(table=1, in_port=1, load:OXM_OF_IN_PORT[]->NXM_NX_REG1[], 
load:0xfffffffe->OXM_OF_IN_PORT[])
+]])
+AT_CHECK([ovs-ofctl parse-flows flows.txt], [0],
+[[usable protocols: any
+chosen protocol: OpenFlow10-table_id
+OFPT_FLOW_MOD (xid=0x1): ADD actions=learn(table=1,output:OXM_OF_IN_PORT[])
+OFPT_FLOW_MOD (xid=0x2): ADD 
actions=learn(table=1,in_port=1,load:OXM_OF_IN_PORT[]->NXM_NX_REG1[],load:0xfffffffe->OXM_OF_IN_PORT[])
+]])
+AT_CLEANUP
+
 AT_SETUP([learning action - examples])
 AT_DATA([flows.txt], [[
 # These are the examples from nicira-ext.h.
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to