From 7b064a79a8efdcce678c82f21d480c7dc811adfb Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 19 Jan 2011 14:51:26 -0800 Subject: [PATCH] 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 --- include/openflow/nicira-ext.h | 6 +- lib/nx-match.c | 102 ++++++++++++++++++++++++++-------- lib/nx-match.def | 58 +++++++++---------- ofproto/ofproto.c | 33 ++++++++--- 4 files changed, 135 insertions(+), 64 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 0b01aaad..5013a5a0 100644 --- 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 f887cdb5..fc3af68d 100644 --- 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 e045d0f0..9c113eb2 100644 --- 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 3084c09e..2eae86dd 100644 --- 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. */ -- 2.30.2