nx-match: Allow NXM_NX_TUN_ID and NXM_OF_VLAN_TCI on NXAST_REG_LOAD.

NXM_NX_TUN_ID and NXM_OF_VLAN_TCI were already allowed on NXAST_REG_MOVE,
but not on NXAST_REG_LOAD.  This makes them valid on both.

Requested-by: Pankaj Thakkar <thakkar@nicira.com>

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 0b01aaa..5013a5a 100644 (file)
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -430,9 +430,9 @@ OFP_ASSERT(sizeof(struct nx_action_reg_move) == 24);
  * starts at 0 for the least-significant bit, 1 for the next most significant
  * bit, and so on.
  *
- * 'dst' is an nxm_header with nxm_hasmask=0.  It must be one of the following:
- *
- *   - NXM_NX_REG(idx) for idx in the switch's accepted range.
+ * 'dst' is an nxm_header with nxm_hasmask=0.  See the documentation for
+ * NXAST_REG_MOVE, above, for the permitted fields and for the side effects of
+ * loading them.
  *
  * The 'ofs' and 'n_bits' fields are combined into a single 'ofs_nbits' field
  * to avoid enlarging the structure by another 8 bytes.  To allow 'n_bits' to

diff --git a/lib/nx-match.c b/lib/nx-match.c
index f887cdb..fc3af68 100644 (file)
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -46,7 +46,8 @@ enum {
 /* For each NXM_* field, define NFI_NXM_* as consecutive integers starting from
  * zero. */
 enum nxm_field_index {
-#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO) NFI_NXM_##HEADER,
+#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO, WRITABLE) \
+        NFI_NXM_##HEADER,
 #include "nx-match.def"
     N_NXM_FIELDS
 };
@@ -59,13 +60,14 @@ struct nxm_field {
     ovs_be16 dl_type;           /* dl_type prerequisite, if nonzero. */
     uint8_t nw_proto;           /* nw_proto prerequisite, if nonzero. */
     const char *name;           /* "NXM_*" string. */
+    bool writable;              /* Writable with NXAST_REG_{MOVE,LOAD}? */
 };
 
 /* All the known fields. */
 static struct nxm_field nxm_fields[N_NXM_FIELDS] = {
-#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO) \
+#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO, WRITABLE)     \
     { HMAP_NODE_NULL_INITIALIZER, NFI_NXM_##HEADER, NXM_##HEADER, WILDCARD, \
-      CONSTANT_HTONS(DL_TYPE), NW_PROTO, "NXM_" #HEADER },
+      CONSTANT_HTONS(DL_TYPE), NW_PROTO, "NXM_" #HEADER, WRITABLE },
 #include "nx-match.def"
 };
 
@@ -97,7 +99,7 @@ nxm_init(void)
         /* Verify that the header values are unique (duplicate "case" values
          * cause a compile error). */
         switch (0) {
-#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO) \
+#define DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO, WRITABLE)  \
         case NXM_##HEADER: break;
 #include "nx-match.def"
         }
@@ -1016,9 +1018,7 @@ nxm_check_reg_move(const struct nx_action_reg_move *action,
         return BAD_ARGUMENT;
     }
 
-    if (!NXM_IS_NX_REG(dst->header)
-        && dst->header != NXM_OF_VLAN_TCI
-        && dst->header != NXM_NX_TUN_ID) {
+    if (!dst->writable) {
         return BAD_ARGUMENT;
     }
 
@@ -1045,7 +1045,7 @@ nxm_check_reg_load(const struct nx_action_reg_load *action,
         return BAD_ARGUMENT;
     }
 
-    if (!NXM_IS_NX_REG(dst->header)) {
+    if (!dst->writable) {
         return BAD_ARGUMENT;
     }
 
@@ -1138,6 +1138,68 @@ nxm_read_field(const struct nxm_field *src, const struct flow *flow)
     NOT_REACHED();
 }
 
+static void
+nxm_write_field(const struct nxm_field *dst, struct flow *flow,
+                uint64_t new_value)
+{
+    switch (dst->index) {
+    case NFI_NXM_OF_VLAN_TCI:
+        flow->vlan_tci = htons(new_value);
+        break;
+
+    case NFI_NXM_NX_TUN_ID:
+        flow->tun_id = htonll(new_value);
+        break;
+
+#define NXM_WRITE_REGISTER(IDX)                 \
+    case NFI_NXM_NX_REG##IDX:                   \
+        flow->regs[IDX] = new_value;            \
+        break;                                  \
+    case NFI_NXM_NX_REG##IDX##_W:               \
+        NOT_REACHED();
+
+    NXM_WRITE_REGISTER(0);
+#if FLOW_N_REGS >= 2
+    NXM_WRITE_REGISTER(1);
+#endif
+#if FLOW_N_REGS >= 3
+    NXM_WRITE_REGISTER(2);
+#endif
+#if FLOW_N_REGS >= 4
+    NXM_WRITE_REGISTER(3);
+#endif
+#if FLOW_N_REGS > 4
+#error
+#endif
+
+    case NFI_NXM_OF_IN_PORT:
+    case NFI_NXM_OF_ETH_DST:
+    case NFI_NXM_OF_ETH_SRC:
+    case NFI_NXM_OF_ETH_TYPE:
+    case NFI_NXM_OF_IP_TOS:
+    case NFI_NXM_OF_IP_PROTO:
+    case NFI_NXM_OF_ARP_OP:
+    case NFI_NXM_OF_IP_SRC:
+    case NFI_NXM_OF_ARP_SPA:
+    case NFI_NXM_OF_IP_DST:
+    case NFI_NXM_OF_ARP_TPA:
+    case NFI_NXM_OF_TCP_SRC:
+    case NFI_NXM_OF_UDP_SRC:
+    case NFI_NXM_OF_TCP_DST:
+    case NFI_NXM_OF_UDP_DST:
+    case NFI_NXM_OF_ICMP_TYPE:
+    case NFI_NXM_OF_ICMP_CODE:
+    case NFI_NXM_OF_ETH_DST_W:
+    case NFI_NXM_OF_VLAN_TCI_W:
+    case NFI_NXM_OF_IP_SRC_W:
+    case NFI_NXM_OF_IP_DST_W:
+    case NFI_NXM_OF_ARP_SPA_W:
+    case NFI_NXM_OF_ARP_TPA_W:
+    case N_NXM_FIELDS:
+        NOT_REACHED();
+    }
+}
+
 void
 nxm_execute_reg_move(const struct nx_action_reg_move *action,
                      struct flow *flow)
@@ -1159,16 +1221,7 @@ nxm_execute_reg_move(const struct nx_action_reg_move *action,
     /* Get the final value. */
     uint64_t new_data = dst_data | ((src_data >> src_ofs) << dst_ofs);
 
-    /* Store the result. */
-    if (NXM_IS_NX_REG(dst->header)) {
-        flow->regs[NXM_NX_REG_IDX(dst->header)] = new_data;
-    } else if (dst->header == NXM_OF_VLAN_TCI) {
-        flow->vlan_tci = htons(new_data);
-    } else if (dst->header == NXM_NX_TUN_ID) {
-        flow->tun_id = htonll(new_data);
-    } else {
-        NOT_REACHED();
-    }
+    nxm_write_field(dst, flow, new_data);
 }
 
 void
@@ -1177,15 +1230,18 @@ nxm_execute_reg_load(const struct nx_action_reg_load *action,
 {
     /* Preparation. */
     int n_bits = nxm_decode_n_bits(action->ofs_nbits);
-    uint32_t mask = n_bits == 32 ? UINT32_MAX : (UINT32_C(1) << n_bits) - 1;
-    uint32_t *reg = &flow->regs[NXM_NX_REG_IDX(ntohl(action->dst))];
+    uint64_t mask = n_bits == 64 ? UINT64_MAX : (UINT64_C(1) << n_bits) - 1;
 
     /* Get source data. */
-    uint32_t src_data = ntohll(action->value);
+    uint64_t src_data = ntohll(action->value);
 
     /* Get remaining bits of the destination field. */
+    const struct nxm_field *dst = nxm_field_lookup(ntohl(action->dst));
     int dst_ofs = nxm_decode_ofs(action->ofs_nbits);
-    uint32_t dst_data = *reg & ~(mask << dst_ofs);
+    uint64_t dst_data = nxm_read_field(dst, flow) & ~(mask << dst_ofs);
+
+    /* Get the final value. */
+    uint64_t new_data = dst_data | (src_data << dst_ofs);
 
-    *reg = dst_data | (src_data << dst_ofs);
+    nxm_write_field(dst, flow, new_data);
 }

diff --git a/lib/nx-match.def b/lib/nx-match.def
index e045d0f..9c113eb 100644 (file)
--- a/lib/nx-match.def
+++ b/lib/nx-match.def
@@ -1,5 +1,5 @@
 /*                                                                -*- c -*-
- * Copyright (c) 2008, 2009, 2010 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,41 +14,41 @@
  * limitations under the License.
  */
 
-#define DEFINE_FIELD_M(HEADER, WILDCARD, DL_TYPE, NW_PROTO)             \
-        DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO)               \
-        DEFINE_FIELD(HEADER##_W, WILDCARD, DL_TYPE, NW_PROTO)
+#define DEFINE_FIELD_M(HEADER, WILDCARD, DL_TYPE, NW_PROTO, WRITABLE)   \
+    DEFINE_FIELD(HEADER, WILDCARD, DL_TYPE, NW_PROTO, WRITABLE)         \
+    DEFINE_FIELD(HEADER##_W, WILDCARD, DL_TYPE, NW_PROTO, false)
 
-/*             NXM_ suffix   FWW_* bit     dl_type       nw_proto      */
-/*             ------------  ------------  -----------   ------------- */
-DEFINE_FIELD  (OF_IN_PORT,   FWW_IN_PORT,  0,            0)
-DEFINE_FIELD_M(OF_ETH_DST,   0,            0,            0)
-DEFINE_FIELD  (OF_ETH_SRC,   FWW_DL_SRC,   0,            0)
-DEFINE_FIELD  (OF_ETH_TYPE,  FWW_DL_TYPE,  0,            0)
-DEFINE_FIELD_M(OF_VLAN_TCI,  0,            0,            0)
-DEFINE_FIELD  (OF_IP_TOS,    FWW_NW_TOS,   ETH_TYPE_IP,  0)
-DEFINE_FIELD  (OF_IP_PROTO,  FWW_NW_PROTO, ETH_TYPE_IP,  0)
-DEFINE_FIELD_M(OF_IP_SRC,    0,            ETH_TYPE_IP,  0)
-DEFINE_FIELD_M(OF_IP_DST,    0,            ETH_TYPE_IP,  0)
-DEFINE_FIELD  (OF_TCP_SRC,   FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_TCP)
-DEFINE_FIELD  (OF_TCP_DST,   FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_TCP)
-DEFINE_FIELD  (OF_UDP_SRC,   FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_UDP)
-DEFINE_FIELD  (OF_UDP_DST,   FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_UDP)
-DEFINE_FIELD  (OF_ICMP_TYPE, FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_ICMP)
-DEFINE_FIELD  (OF_ICMP_CODE, FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_ICMP)
-DEFINE_FIELD  (OF_ARP_OP,    FWW_NW_PROTO, ETH_TYPE_ARP, 0)
-DEFINE_FIELD_M(OF_ARP_SPA,   0,            ETH_TYPE_ARP, 0)
-DEFINE_FIELD_M(OF_ARP_TPA,   0,            ETH_TYPE_ARP, 0)
-DEFINE_FIELD  (NX_TUN_ID,    FWW_TUN_ID,   0,            0)
+/*             NXM_ suffix   FWW_* bit     dl_type       nw_proto      rw? */
+/*             ------------  ------------  -----------   ------------- --- */
+DEFINE_FIELD  (OF_IN_PORT,   FWW_IN_PORT,  0,            0,            false)
+DEFINE_FIELD_M(OF_ETH_DST,   0,            0,            0,            false)
+DEFINE_FIELD  (OF_ETH_SRC,   FWW_DL_SRC,   0,            0,            false)
+DEFINE_FIELD  (OF_ETH_TYPE,  FWW_DL_TYPE,  0,            0,            false)
+DEFINE_FIELD_M(OF_VLAN_TCI,  0,            0,            0,            true)
+DEFINE_FIELD  (OF_IP_TOS,    FWW_NW_TOS,   ETH_TYPE_IP,  0,            false)
+DEFINE_FIELD  (OF_IP_PROTO,  FWW_NW_PROTO, ETH_TYPE_IP,  0,            false)
+DEFINE_FIELD_M(OF_IP_SRC,    0,            ETH_TYPE_IP,  0,            false)
+DEFINE_FIELD_M(OF_IP_DST,    0,            ETH_TYPE_IP,  0,            false)
+DEFINE_FIELD  (OF_TCP_SRC,   FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_TCP,  false)
+DEFINE_FIELD  (OF_TCP_DST,   FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_TCP,  false)
+DEFINE_FIELD  (OF_UDP_SRC,   FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_UDP,  false)
+DEFINE_FIELD  (OF_UDP_DST,   FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_UDP,  false)
+DEFINE_FIELD  (OF_ICMP_TYPE, FWW_TP_SRC,   ETH_TYPE_IP,  IP_TYPE_ICMP, false)
+DEFINE_FIELD  (OF_ICMP_CODE, FWW_TP_DST,   ETH_TYPE_IP,  IP_TYPE_ICMP, false)
+DEFINE_FIELD  (OF_ARP_OP,    FWW_NW_PROTO, ETH_TYPE_ARP, 0,            false)
+DEFINE_FIELD_M(OF_ARP_SPA,   0,            ETH_TYPE_ARP, 0,            false)
+DEFINE_FIELD_M(OF_ARP_TPA,   0,            ETH_TYPE_ARP, 0,            false)
+DEFINE_FIELD  (NX_TUN_ID,    FWW_TUN_ID,   0,            0,            true)
 
-DEFINE_FIELD_M(NX_REG0,      0,            0,            0)
+DEFINE_FIELD_M(NX_REG0,      0,            0,            0,            true)
 #if FLOW_N_REGS >= 2
-DEFINE_FIELD_M(NX_REG1,      0,            0,            0)
+DEFINE_FIELD_M(NX_REG1,      0,            0,            0,            true)
 #endif
 #if FLOW_N_REGS >= 3
-DEFINE_FIELD_M(NX_REG2,      0,            0,            0)
+DEFINE_FIELD_M(NX_REG2,      0,            0,            0,            true)
 #endif
 #if FLOW_N_REGS >= 4
-DEFINE_FIELD_M(NX_REG3,      0,            0,            0)
+DEFINE_FIELD_M(NX_REG3,      0,            0,            0,            true)
 #endif
 #if FLOW_N_REGS > 4
 #error

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3084c09..2eae86d 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2858,19 +2858,27 @@ xlate_set_dl_tci(struct action_xlate_ctx *ctx)
     }
 }
 
+struct xlate_reg_state {
+    ovs_be16 vlan_tci;
+    ovs_be64 tun_id;
+};
+
 static void
-xlate_reg_move_action(struct action_xlate_ctx *ctx,
-                      const struct nx_action_reg_move *narm)
+save_reg_state(const struct action_xlate_ctx *ctx,
+               struct xlate_reg_state *state)
 {
-    ovs_be16 old_tci = ctx->flow.vlan_tci;
-    ovs_be64 old_tun = ctx->flow.tun_id;
-
-    nxm_execute_reg_move(narm, &ctx->flow);
+    state->vlan_tci = ctx->flow.vlan_tci;
+    state->tun_id = ctx->flow.tun_id;
+}
 
-    if (ctx->flow.vlan_tci != old_tci) {
+static void
+update_reg_state(struct action_xlate_ctx *ctx,
+                 const struct xlate_reg_state *state)
+{
+    if (ctx->flow.vlan_tci != state->vlan_tci) {
         xlate_set_dl_tci(ctx);
     }
-    if (ctx->flow.tun_id != old_tun) {
+    if (ctx->flow.tun_id != state->tun_id) {
         nl_msg_put_be64(ctx->odp_actions, ODPAT_SET_TUNNEL, ctx->flow.tun_id);
     }
 }
@@ -2884,6 +2892,7 @@ xlate_nicira_action(struct action_xlate_ctx *ctx,
     const struct nx_action_set_queue *nasq;
     const struct nx_action_multipath *nam;
     enum nx_action_subtype subtype = ntohs(nah->subtype);
+    struct xlate_reg_state state;
     ovs_be64 tun_id;
 
     assert(nah->vendor == htonl(NX_VENDOR_ID));
@@ -2916,12 +2925,18 @@ xlate_nicira_action(struct action_xlate_ctx *ctx,
         break;
 
     case NXAST_REG_MOVE:
-        xlate_reg_move_action(ctx, (const struct nx_action_reg_move *) nah);
+        save_reg_state(ctx, &state);
+        nxm_execute_reg_move((const struct nx_action_reg_move *) nah,
+                             &ctx->flow);
+        update_reg_state(ctx, &state);
         break;
 
     case NXAST_REG_LOAD:
+        save_reg_state(ctx, &state);
         nxm_execute_reg_load((const struct nx_action_reg_load *) nah,
                              &ctx->flow);
+        update_reg_state(ctx, &state);
+        break;
 
     case NXAST_NOTE:
         /* Nothing to do. */