Change the value and mask to be added to the end of the set field
action without any extra bytes, exept for the usual ofp-actions
padding to 8 bytes. Together with some structure member packing this
saves on average about to 256 bytes for each set field and load action
(set field internal representation is also used for load actions).
On a specific production data set each flow entry uses on average
about 4.1 load and set field actions. This means that with this patch
an average of more than 1kb can be saved for each flow with such a
flow table.
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
include/openvswitch/ofp-actions.h | 21 ++++-
include/ovn/actions.h | 8 --
lib/learn.c | 9 +-
lib/ofp-actions.c | 192 ++++++++++++++++++++++----------------
ofproto/ofproto-dpif-xlate.c | 15 +--
ovn/controller/lflow.c | 10 +-
ovn/controller/physical.c | 10 +-
ovn/controller/pinctrl.c | 9 +-
ovn/lib/actions.c | 64 ++++++-------
ovn/utilities/ovn-trace.c | 37 +++++---
10 files changed, 206 insertions(+), 169 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index b30b270..9eaa78c 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -480,11 +480,19 @@ struct ofpact_stack {
* Used for NXAST_REG_LOAD and OFPAT12_SET_FIELD. */
struct ofpact_set_field {
struct ofpact ofpact;
- const struct mf_field *field;
bool flow_has_vlan; /* VLAN present at action validation time. */
- union mf_value value;
- union mf_value mask;
+ const struct mf_field *field;
+ union mf_value value[]; /* Significant value bytes followed by
+ * significant mask bytes. */
};
+BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value)
+ % OFPACT_ALIGNTO == 0);
+BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value)
+ == sizeof(struct ofpact_set_field));
+
+/* Use macro to not have to deal with constness. */
+#define ofpact_set_field_mask(SF) \
+ ((union mf_value *)(void *)((uint8_t *)(SF)->value + (SF)->field->n_bytes))
/* OFPACT_PUSH_VLAN/MPLS/PBB
*
@@ -1058,7 +1066,12 @@ OFPACTS
#undef OFPACT
/* Additional functions for composing ofpacts. */
-struct ofpact_set_field *ofpact_put_reg_load(struct ofpbuf *ofpacts);
+struct ofpact_set_field *ofpact_put_set_field(struct ofpbuf *ofpacts,
+ const struct mf_field *);
+struct ofpact_set_field *ofpact_put_reg_load(struct ofpbuf *ofpacts,
+ const struct mf_field *);
+struct ofpact_set_field *ofpact_put_reg_load2(struct ofpbuf *ofpacts,
+ const struct mf_field *);
/* OpenFlow 1.1 instructions.
* The order is sorted in execution order. Not in the value of OFPIT11_xxx.
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index e2a716a..6f6f858 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -27,7 +27,6 @@
#include "util.h"
struct lexer;
-struct ofpact_set_field;
struct ofpbuf;
struct shash;
struct simap;
@@ -151,13 +150,6 @@ struct ovnact_load {
union expr_constant imm;
};
-void ovnact_load_to_ofpact_set_field(const struct ovnact_load *,
- bool (*lookup_port)(const void *aux,
- const char *port_name,
- unsigned int *portp),
- const void *aux,
- struct ofpact_set_field *);
-
/* OVNACT_MOVE, OVNACT_EXCHANGE. */
struct ovnact_move {
struct ovnact ovnact;
diff --git a/lib/learn.c b/lib/learn.c
index 563b034..1fb5a6a 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -137,13 +137,12 @@ learn_execute(const struct ofpact_learn *learn, const
struct flow *flow,
break;
case NX_LEARN_DST_LOAD:
- sf = ofpact_put_reg_load(ofpacts);
- sf->field = spec->dst.field;
+ sf = ofpact_put_reg_load(ofpacts, spec->dst.field);
bitwise_copy(&value, sizeof value, 0,
- &sf->value, spec->dst.field->n_bytes, spec->dst.ofs,
+ sf->value, spec->dst.field->n_bytes, spec->dst.ofs,
spec->n_bits);
- bitwise_one(&sf->mask, spec->dst.field->n_bytes, spec->dst.ofs,
- spec->n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), spec->dst.field->n_bytes,
+ spec->dst.ofs, spec->n_bits);
break;
case NX_LEARN_DST_OUTPUT:
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 5b35edd..7c304d9 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -353,8 +353,8 @@ static enum ofperr ofpacts_verify(const struct ofpact[],
size_t ofpacts_len,
uint32_t allowed_ovsinsts,
enum ofpact_type outer_action);
-static void ofpact_put_set_field(struct ofpbuf *openflow, enum ofp_version,
- enum mf_field_id, uint64_t value);
+static void put_set_field(struct ofpbuf *openflow, enum ofp_version,
+ enum mf_field_id, uint64_t value);
static void put_reg_load(struct ofpbuf *openflow,
const struct mf_subfield *, uint64_t value);
@@ -1435,8 +1435,7 @@ encode_SET_VLAN_VID(const struct ofpact_vlan_vid
*vlan_vid,
} else if (ofp_version == OFP11_VERSION) {
put_OFPAT11_SET_VLAN_VID(out, vid);
} else {
- ofpact_put_set_field(out, ofp_version,
- MFF_VLAN_VID, vid | OFPVID12_PRESENT);
+ put_set_field(out, ofp_version, MFF_VLAN_VID, vid | OFPVID12_PRESENT);
}
}
@@ -1526,7 +1525,7 @@ encode_SET_VLAN_PCP(const struct ofpact_vlan_pcp
*vlan_pcp,
} else if (ofp_version == OFP11_VERSION) {
put_OFPAT11_SET_VLAN_PCP(out, pcp);
} else {
- ofpact_put_set_field(out, ofp_version, MFF_VLAN_PCP, pcp);
+ put_set_field(out, ofp_version, MFF_VLAN_PCP, pcp);
}
}
@@ -1713,8 +1712,7 @@ encode_SET_ETH_addr(const struct ofpact_mac *mac, enum
ofp_version ofp_version,
oada = ofpact_put_raw(out, ofp_version, raw, 0);
oada->dl_addr = mac->mac;
} else {
- ofpact_put_set_field(out, ofp_version, field,
- eth_addr_to_uint64(mac->mac));
+ put_set_field(out, ofp_version, field, eth_addr_to_uint64(mac->mac));
}
}
@@ -1795,7 +1793,7 @@ encode_SET_IPV4_addr(const struct ofpact_ipv4 *ipv4,
if (ofp_version < OFP12_VERSION) {
ofpact_put_raw(out, ofp_version, raw, ntohl(addr));
} else {
- ofpact_put_set_field(out, ofp_version, field, ntohl(addr));
+ put_set_field(out, ofp_version, field, ntohl(addr));
}
}
@@ -1865,8 +1863,7 @@ encode_SET_IP_DSCP(const struct ofpact_dscp *dscp,
if (ofp_version < OFP12_VERSION) {
put_OFPAT_SET_NW_TOS(out, ofp_version, dscp->dscp);
} else {
- ofpact_put_set_field(out, ofp_version,
- MFF_IP_DSCP_SHIFTED, dscp->dscp >> 2);
+ put_set_field(out, ofp_version, MFF_IP_DSCP_SHIFTED, dscp->dscp >> 2);
}
}
@@ -1922,7 +1919,7 @@ encode_SET_IP_ECN(const struct ofpact_ecn *ip_ecn,
} else if (ofp_version == OFP11_VERSION) {
put_OFPAT11_SET_NW_ECN(out, ecn);
} else {
- ofpact_put_set_field(out, ofp_version, MFF_IP_ECN, ecn);
+ put_set_field(out, ofp_version, MFF_IP_ECN, ecn);
}
}
@@ -2026,7 +2023,7 @@ encode_SET_L4_port(const struct ofpact_l4_port *l4_port,
uint16_t port = l4_port->port;
if (ofp_version >= OFP12_VERSION && field != MFF_N_IDS) {
- ofpact_put_set_field(out, ofp_version, field, port);
+ put_set_field(out, ofp_version, field, port);
} else {
ofpact_put_raw(out, ofp_version, raw, port);
}
@@ -2468,16 +2465,17 @@ decode_ofpat_set_field(const struct
ofp12_action_set_field *oasf,
struct ofpbuf b = ofpbuf_const_initializer(oasf, ntohs(oasf->len));
ofpbuf_pull(&b, OBJECT_OFFSETOF(oasf, pad));
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- enum ofperr error = nx_pull_entry(&b, &sf->field, &sf->value,
- may_mask ? &sf->mask : NULL);
+ union mf_value value, mask;
+ const struct mf_field *field;
+ enum ofperr error = nx_pull_entry(&b, &field, &value,
+ may_mask ? &mask : NULL);
if (error) {
return (error == OFPERR_OFPBMC_BAD_MASK
? OFPERR_OFPBAC_BAD_SET_MASK
: error);
}
if (!may_mask) {
- memset(&sf->mask, 0xff, sf->field->n_bytes);
+ memset(&mask, 0xff, field->n_bytes);
}
if (!is_all_zeros(b.data, b.size)) {
@@ -2486,32 +2484,36 @@ decode_ofpat_set_field(const struct
ofp12_action_set_field *oasf,
/* OpenFlow says specifically that one may not set OXM_OF_IN_PORT via
* Set-Field. */
- if (sf->field->id == MFF_IN_PORT_OXM) {
+ if (field->id == MFF_IN_PORT_OXM) {
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
/* oxm_length is now validated to be compatible with mf_value. */
- if (!sf->field->writable) {
+ if (!field->writable) {
VLOG_WARN_RL(&rl, "destination field %s is not writable",
- sf->field->name);
+ field->name);
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
/* The value must be valid for match. OpenFlow 1.5 also says,
* "In an OXM_OF_VLAN_VID set-field action, the OFPVID_PRESENT bit must be
* a 1-bit in oxm_value and in oxm_mask." */
- if (!mf_is_value_valid(sf->field, &sf->value)
- || (sf->field->id == MFF_VLAN_VID
- && (!(sf->mask.be16 & htons(OFPVID12_PRESENT))
- || !(sf->value.be16 & htons(OFPVID12_PRESENT))))) {
+ if (!mf_is_value_valid(field, &value)
+ || (field->id == MFF_VLAN_VID
+ && (!(mask.be16 & htons(OFPVID12_PRESENT))
+ || !(value.be16 & htons(OFPVID12_PRESENT))))) {
struct ds ds = DS_EMPTY_INITIALIZER;
- mf_format(sf->field, &sf->value, NULL, &ds);
+ mf_format(field, &value, NULL, &ds);
VLOG_WARN_RL(&rl, "Invalid value for set field %s: %s",
- sf->field->name, ds_cstr(&ds));
+ field->name, ds_cstr(&ds));
ds_destroy(&ds);
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
+
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, field);
+ memcpy(sf->value, &value, field->n_bytes);
+ memcpy(ofpact_set_field_mask(sf), &mask, field->n_bytes);
return 0;
}
@@ -2536,12 +2538,9 @@ decode_NXAST_RAW_REG_LOAD(const struct
nx_action_reg_load *narl,
enum ofp_version ofp_version OVS_UNUSED,
struct ofpbuf *out)
{
- struct ofpact_set_field *sf = ofpact_put_reg_load(out);
struct mf_subfield dst;
enum ofperr error;
- sf->ofpact.raw = NXAST_RAW_REG_LOAD;
-
dst.field = mf_from_nxm_header(ntohl(narl->dst));
dst.ofs = nxm_decode_ofs(narl->ofs_nbits);
dst.n_bits = nxm_decode_n_bits(narl->ofs_nbits);
@@ -2556,14 +2555,14 @@ decode_NXAST_RAW_REG_LOAD(const struct
nx_action_reg_load *narl,
return OFPERR_OFPBAC_BAD_ARGUMENT;
}
- sf->field = dst.field;
+ struct ofpact_set_field *sf = ofpact_put_reg_load(out, dst.field);
+
bitwise_put(ntohll(narl->value),
- &sf->value, dst.field->n_bytes, dst.ofs,
+ sf->value, dst.field->n_bytes, dst.ofs,
dst.n_bits);
bitwise_put(UINT64_MAX,
- &sf->mask, dst.field->n_bytes, dst.ofs,
+ ofpact_set_field_mask(sf), dst.field->n_bytes, dst.ofs,
dst.n_bits);
-
return 0;
}
@@ -2572,13 +2571,12 @@ decode_NXAST_RAW_REG_LOAD2(const struct
nx_action_reg_load2 *narl,
enum ofp_version ofp_version OVS_UNUSED,
struct ofpbuf *out)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(out);
- sf->ofpact.raw = NXAST_RAW_REG_LOAD2;
-
struct ofpbuf b = ofpbuf_const_initializer(narl, ntohs(narl->len));
ofpbuf_pull(&b, OBJECT_OFFSETOF(narl, pad));
- enum ofperr error = nx_pull_entry(&b, &sf->field, &sf->value, &sf->mask);
+ union mf_value value, mask;
+ const struct mf_field *field;
+ enum ofperr error = nx_pull_entry(&b, &field, &value, &mask);
if (error) {
return error;
}
@@ -2586,17 +2584,21 @@ decode_NXAST_RAW_REG_LOAD2(const struct
nx_action_reg_load2 *narl,
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
- if (!sf->field->writable) {
- VLOG_WARN_RL(&rl, "destination field %s is not writable",
- sf->field->name);
+ if (!field->writable) {
+ VLOG_WARN_RL(&rl, "destination field %s is not writable", field->name);
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
+
+ /* Put value and mask and update length. */
+ struct ofpact_set_field *sf = ofpact_put_reg_load2(out, field);
+ memcpy(sf->value, &value, field->n_bytes);
+ memcpy(ofpact_set_field_mask(sf), &mask, field->n_bytes);
return 0;
}
static void
-ofpact_put_set_field(struct ofpbuf *openflow, enum ofp_version ofp_version,
- enum mf_field_id field, uint64_t value_)
+put_set_field(struct ofpbuf *openflow, enum ofp_version ofp_version,
+ enum mf_field_id field, uint64_t value_)
{
struct ofp12_action_set_field *oasf OVS_UNUSED;
int n_bytes = mf_from_id(field)->n_bytes;
@@ -2633,11 +2635,13 @@ next_load_segment(const struct ofpact_set_field *sf,
if (start < n_bits) {
dst->field = sf->field;
- dst->ofs = bitwise_scan(&sf->mask, n_bytes, 1, start, n_bits);
+ dst->ofs = bitwise_scan(ofpact_set_field_mask(sf), n_bytes, 1, start,
+ n_bits);
if (dst->ofs < n_bits) {
- dst->n_bits = bitwise_scan(&sf->mask, n_bytes, 0, dst->ofs + 1,
+ dst->n_bits = bitwise_scan(ofpact_set_field_mask(sf), n_bytes, 0,
+ dst->ofs + 1,
MIN(dst->ofs + 64, n_bits)) - dst->ofs;
- *value = bitwise_get(&sf->value, n_bytes, dst->ofs, dst->n_bits);
+ *value = bitwise_get(sf->value, n_bytes, dst->ofs, dst->n_bits);
return true;
}
}
@@ -2659,7 +2663,8 @@ set_field_to_nxast(const struct ofpact_set_field *sf,
struct ofpbuf *openflow)
narl = put_NXAST_REG_LOAD2(openflow);
openflow->size = openflow->size - sizeof narl->pad;
- nx_put_entry(openflow, sf->field->id, 0, &sf->value, &sf->mask);
+ nx_put_entry(openflow, sf->field->id, 0, sf->value,
+ ofpact_set_field_mask(sf));
pad_ofpat(openflow, start_ofs);
} else {
struct mf_subfield dst;
@@ -2686,7 +2691,7 @@ set_field_to_legacy_openflow(const struct
ofpact_set_field *sf,
{
switch ((int) sf->field->id) {
case MFF_VLAN_TCI: {
- ovs_be16 tci = sf->value.be16;
+ ovs_be16 tci = sf->value->be16;
bool cfi = (tci & htons(VLAN_CFI)) != 0;
uint16_t vid = vlan_tci_to_vid(tci);
uint8_t pcp = vlan_tci_to_pcp(tci);
@@ -2734,7 +2739,7 @@ set_field_to_legacy_openflow(const struct
ofpact_set_field *sf,
}
case MFF_VLAN_VID: {
- uint16_t vid = ntohs(sf->value.be16) & VLAN_VID_MASK;
+ uint16_t vid = ntohs(sf->value->be16) & VLAN_VID_MASK;
if (ofp_version == OFP10_VERSION) {
put_OFPAT10_SET_VLAN_VID(out, vid);
} else {
@@ -2745,48 +2750,48 @@ set_field_to_legacy_openflow(const struct
ofpact_set_field *sf,
case MFF_VLAN_PCP:
if (ofp_version == OFP10_VERSION) {
- put_OFPAT10_SET_VLAN_PCP(out, sf->value.u8);
+ put_OFPAT10_SET_VLAN_PCP(out, sf->value->u8);
} else {
- put_OFPAT11_SET_VLAN_PCP(out, sf->value.u8);
+ put_OFPAT11_SET_VLAN_PCP(out, sf->value->u8);
}
break;
case MFF_ETH_SRC:
- put_OFPAT_SET_DL_SRC(out, ofp_version)->dl_addr = sf->value.mac;
+ put_OFPAT_SET_DL_SRC(out, ofp_version)->dl_addr = sf->value->mac;
break;
case MFF_ETH_DST:
- put_OFPAT_SET_DL_DST(out, ofp_version)->dl_addr = sf->value.mac;
+ put_OFPAT_SET_DL_DST(out, ofp_version)->dl_addr = sf->value->mac;
break;
case MFF_IPV4_SRC:
- put_OFPAT_SET_NW_SRC(out, ofp_version, sf->value.be32);
+ put_OFPAT_SET_NW_SRC(out, ofp_version, sf->value->be32);
break;
case MFF_IPV4_DST:
- put_OFPAT_SET_NW_DST(out, ofp_version, sf->value.be32);
+ put_OFPAT_SET_NW_DST(out, ofp_version, sf->value->be32);
break;
case MFF_IP_DSCP:
- put_OFPAT_SET_NW_TOS(out, ofp_version, sf->value.u8);
+ put_OFPAT_SET_NW_TOS(out, ofp_version, sf->value->u8);
break;
case MFF_IP_DSCP_SHIFTED:
- put_OFPAT_SET_NW_TOS(out, ofp_version, sf->value.u8 << 2);
+ put_OFPAT_SET_NW_TOS(out, ofp_version, sf->value->u8 << 2);
break;
case MFF_IP_ECN:
- put_OFPAT11_SET_NW_ECN(out, sf->value.u8);
+ put_OFPAT11_SET_NW_ECN(out, sf->value->u8);
break;
case MFF_TCP_SRC:
case MFF_UDP_SRC:
- put_OFPAT_SET_TP_SRC(out, sf->value.be16);
+ put_OFPAT_SET_TP_SRC(out, sf->value->be16);
break;
case MFF_TCP_DST:
case MFF_UDP_DST:
- put_OFPAT_SET_TP_DST(out, sf->value.be16);
+ put_OFPAT_SET_TP_DST(out, sf->value->be16);
break;
default:
@@ -2804,7 +2809,8 @@ set_field_to_set_field(const struct ofpact_set_field *sf,
oasf = put_OFPAT12_SET_FIELD(out);
out->size = out->size - sizeof oasf->pad;
- nx_put_entry(out, sf->field->id, ofp_version, &sf->value, &sf->mask);
+ nx_put_entry(out, sf->field->id, ofp_version, sf->value,
+ ofpact_set_field_mask(sf));
pad_ofpat(out, start_ofs);
}
@@ -2823,7 +2829,7 @@ encode_SET_FIELD(const struct ofpact_set_field *sf,
} else if (ofp_version < OFP12_VERSION) {
/* OpenFlow 1.0 and 1.1 don't have Set-Field. */
set_field_to_legacy_openflow(sf, ofp_version, out);
- } else if (is_all_ones((const uint8_t *) &sf->mask, sf->field->n_bytes)) {
+ } else if (is_all_ones(ofpact_set_field_mask(sf), sf->field->n_bytes)) {
/* We're encoding to OpenFlow 1.2, 1.3, or 1.4. The action sets an
* entire field, so encode it as OFPAT_SET_FIELD. */
set_field_to_set_field(sf, ofp_version, out);
@@ -2877,11 +2883,11 @@ static char * OVS_WARN_UNUSED_RESULT
set_field_parse__(char *arg, struct ofpbuf *ofpacts,
enum ofputil_protocol *usable_protocols)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
char *value;
char *delim;
char *key;
const struct mf_field *mf;
+ union mf_value sf_value, sf_mask;
char *error;
error = set_field_split_str(arg, &key, &value, &delim);
@@ -2896,18 +2902,23 @@ set_field_parse__(char *arg, struct ofpbuf *ofpacts,
if (!mf->writable) {
return xasprintf("%s is read-only", key);
}
- sf->field = mf;
+
delim[0] = '\0';
- error = mf_parse(mf, value, &sf->value, &sf->mask);
+ error = mf_parse(mf, value, &sf_value, &sf_mask);
if (error) {
return error;
}
- if (!mf_is_value_valid(mf, &sf->value)) {
+ if (!mf_is_value_valid(mf, &sf_value)) {
return xasprintf("%s is not a valid value for field %s", value, key);
}
*usable_protocols &= mf->usable_protocols_exact;
+
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, mf);
+ memcpy(sf->value, &sf_value, mf->n_bytes);
+ memcpy(ofpact_set_field_mask(sf), &sf_mask, mf->n_bytes);
+
return NULL;
}
@@ -2929,7 +2940,6 @@ parse_SET_FIELD(const char *arg, struct ofpbuf *ofpacts,
static char * OVS_WARN_UNUSED_RESULT
parse_reg_load(char *arg, struct ofpbuf *ofpacts)
{
- struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts);
struct mf_subfield dst;
char *key, *value_str;
union mf_value value;
@@ -2962,12 +2972,12 @@ parse_reg_load(char *arg, struct ofpbuf *ofpacts)
return error;
}
- sf->field = dst.field;
- memset(&sf->value, 0, sizeof sf->value);
- bitwise_copy(&value, dst.field->n_bytes, 0, &sf->value,
- dst.field->n_bytes, dst.ofs, dst.n_bits);
- bitwise_one(&sf->mask, dst.field->n_bytes, dst.ofs, dst.n_bits);
+ struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts, dst.field);
+ bitwise_copy(&value, dst.field->n_bytes, 0, sf->value,
+ dst.field->n_bytes, dst.ofs, dst.n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), dst.field->n_bytes, dst.ofs,
+ dst.n_bits);
return NULL;
}
@@ -2989,24 +2999,51 @@ format_SET_FIELD(const struct ofpact_set_field *a,
struct ds *s)
ds_chomp(s, ',');
} else {
ds_put_format(s, "%sset_field:%s", colors.special, colors.end);
- mf_format(a->field, &a->value, &a->mask, s);
+ mf_format(a->field, a->value, ofpact_set_field_mask(a), s);
ds_put_format(s, "%s->%s%s",
colors.special, colors.end, a->field->name);
}
}
+/* Appends an OFPACT_SET_FIELD ofpact with enough space for the value and mask
+ * for the 'field' to 'ofpacts' and returns it. */
+struct ofpact_set_field *
+ofpact_put_set_field(struct ofpbuf *ofpacts, const struct mf_field *field)
+{
+ struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+ sf->field = field;
+
+ /* Put value and mask and update length. */
+ ofpbuf_put_zeros(ofpacts, 2 * sf->field->n_bytes);
+ sf = ofpacts->header;
+ ofpact_finish_SET_FIELD(ofpacts, &sf);
+
+ return sf;
+}
+
/* Appends an OFPACT_SET_FIELD ofpact to 'ofpacts' and returns it. The ofpact
* is marked such that, if possible, it will be translated to OpenFlow as
* NXAST_REG_LOAD extension actions rather than OFPAT_SET_FIELD, either because
* that was the way that the action was expressed when it came into OVS or for
* backward compatibility. */
struct ofpact_set_field *
-ofpact_put_reg_load(struct ofpbuf *ofpacts)
+ofpact_put_reg_load(struct ofpbuf *ofpacts, const struct mf_field *field)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, field);
sf->ofpact.raw = NXAST_RAW_REG_LOAD;
+
return sf;
}
+
+struct ofpact_set_field *
+ofpact_put_reg_load2(struct ofpbuf *ofpacts, const struct mf_field *field)
+{
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, field);
+ sf->ofpact.raw = NXAST_RAW_REG_LOAD2;
+
+ return sf;
+}
+
/* Action structure for NXAST_STACK_PUSH and NXAST_STACK_POP.
*
@@ -3308,8 +3345,7 @@ encode_SET_MPLS_LABEL(const struct ofpact_mpls_label
*label,
if (ofp_version < OFP12_VERSION) {
put_OFPAT_SET_MPLS_LABEL(out, ofp_version, label->label);
} else {
- ofpact_put_set_field(out, ofp_version, MFF_MPLS_LABEL,
- ntohl(label->label));
+ put_set_field(out, ofp_version, MFF_MPLS_LABEL, ntohl(label->label));
}
}
@@ -3352,7 +3388,7 @@ encode_SET_MPLS_TC(const struct ofpact_mpls_tc *tc,
if (ofp_version < OFP12_VERSION) {
put_OFPAT_SET_MPLS_TC(out, ofp_version, tc->tc);
} else {
- ofpact_put_set_field(out, ofp_version, MFF_MPLS_TC, tc->tc);
+ put_set_field(out, ofp_version, MFF_MPLS_TC, tc->tc);
}
}
@@ -3576,7 +3612,7 @@ encode_SET_TUNNEL(const struct ofpact_tunnel *tunnel,
put_NXAST_SET_TUNNEL64(out, tun_id);
}
} else {
- ofpact_put_set_field(out, ofp_version, MFF_TUN_ID, tun_id);
+ put_set_field(out, ofp_version, MFF_TUN_ID, tun_id);
}
}
@@ -6916,7 +6952,7 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
struct ofpact *a,
if (mf->id == MFF_VLAN_TCI) {
/* The set field may add or remove the vlan tag,
* Mark the status temporarily. */
- flow->vlan_tci = ofpact_get_SET_FIELD(a)->value.be16;
+ flow->vlan_tci = ofpact_get_SET_FIELD(a)->value->be16;
}
return 0;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 893c033..b475fef 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4924,9 +4924,10 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
/* Set the field only if the packet actually has it. */
if (mf_are_prereqs_ok(mf, flow, wc)) {
- mf_mask_field_masked(mf, &set_field->mask, wc);
- mf_set_flow_value_masked(mf, &set_field->value,
- &set_field->mask, flow);
+ mf_mask_field_masked(mf, ofpact_set_field_mask(set_field), wc);
+ mf_set_flow_value_masked(mf, set_field->value,
+ ofpact_set_field_mask(set_field),
+ flow);
}
break;
@@ -5754,11 +5755,11 @@ xlate_send_packet(const struct ofport_dpif *ofport,
bool oam,
}
if (oam) {
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
+ struct ofpact_set_field *sf
+ = ofpact_put_set_field(&ofpacts, mf_from_id(MFF_TUN_FLAGS));
- sf->field = mf_from_id(MFF_TUN_FLAGS);
- sf->value.be16 = htons(NX_TUN_FLAG_OAM);
- sf->mask.be16 = htons(NX_TUN_FLAG_OAM);
+ sf->value->be16 = htons(NX_TUN_FLAG_OAM);
+ ofpact_set_field_mask(sf)->be16 = htons(NX_TUN_FLAG_OAM);
}
ofpact_put_OUTPUT(&ofpacts)->port = xport->ofp_port;
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 341ca08..fe03375 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -501,12 +501,10 @@ put_load(const uint8_t *data, size_t len,
enum mf_field_id dst, int ofs, int n_bits,
struct ofpbuf *ofpacts)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf_from_id(dst);
- sf->flow_has_vlan = false;
-
- bitwise_copy(data, len, 0, &sf->value, sf->field->n_bytes, ofs, n_bits);
- bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits);
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
+ mf_from_id(dst));
+ bitwise_copy(data, len, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
}
static void
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 23e3e2c..04a08bc 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -104,13 +104,11 @@ static void
put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
struct ofpbuf *ofpacts)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf_from_id(dst);
- sf->flow_has_vlan = false;
-
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
+ mf_from_id(dst));
ovs_be64 n_value = htonll(value);
- bitwise_copy(&n_value, 8, 0, &sf->value, sf->field->n_bytes, ofs, n_bits);
- bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits);
+ bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
}
static void
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 2737467..d3b9fb9 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1342,11 +1342,10 @@ reload_metadata(struct ofpbuf *ofpacts, const struct
match *md)
for (size_t i = 0; i < ARRAY_SIZE(md_fields); i++) {
const struct mf_field *field = mf_from_id(md_fields[i]);
if (!mf_is_all_wild(field, &md->wc)) {
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = field;
- sf->flow_has_vlan = false;
- mf_get_value(field, &md->flow, &sf->value);
- memset(&sf->mask, 0xff, field->n_bytes);
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, field);
+
+ mf_get_value(field, &md->flow, sf->value);
+ memset(ofpact_set_field_mask(sf), 0xff, field->n_bytes);
}
}
}
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 3908e1d..d932845 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -162,13 +162,11 @@ static void
put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
struct ofpbuf *ofpacts)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf_from_id(dst);
- sf->flow_has_vlan = false;
-
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
+ mf_from_id(dst));
ovs_be64 n_value = htonll(value);
bitwise_copy(&n_value, 8, 0, &sf->value, sf->field->n_bytes, ofs, n_bits);
- bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
}
/* Context maintained during ovnacts_parse(). */
@@ -339,52 +337,40 @@ format_LOAD(const struct ovnact_load *load, struct ds *s)
ds_put_char(s, ';');
}
-void
-ovnact_load_to_ofpact_set_field(const struct ovnact_load *load,
- bool (*lookup_port)(const void *aux,
- const char *port_name,
- unsigned int *portp),
- const void *aux,
- struct ofpact_set_field *sf)
+static void
+encode_LOAD(const struct ovnact_load *load,
+ const struct ovnact_encode_params *ep,
+ struct ofpbuf *ofpacts)
{
const union expr_constant *c = &load->imm;
struct mf_subfield dst = expr_resolve_field(&load->dst);
-
- sf->field = dst.field;
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, dst.field);
if (load->dst.symbol->width) {
bitwise_copy(&c->value, sizeof c->value, 0,
- &sf->value, dst.field->n_bytes, dst.ofs,
+ sf->value, dst.field->n_bytes, dst.ofs,
dst.n_bits);
if (c->masked) {
bitwise_copy(&c->mask, sizeof c->mask, 0,
- &sf->mask, dst.field->n_bytes, dst.ofs,
- dst.n_bits);
+ ofpact_set_field_mask(sf), dst.field->n_bytes,
+ dst.ofs, dst.n_bits);
} else {
- bitwise_one(&sf->mask, dst.field->n_bytes,
+ bitwise_one(ofpact_set_field_mask(sf), dst.field->n_bytes,
dst.ofs, dst.n_bits);
}
} else {
uint32_t port;
- if (!lookup_port(aux, load->imm.string, &port)) {
+ if (!ep->lookup_port(ep->aux, load->imm.string, &port)) {
port = 0;
}
- bitwise_put(port, &sf->value,
+ bitwise_put(port, sf->value,
sf->field->n_bytes, 0, sf->field->n_bits);
- bitwise_one(&sf->mask, sf->field->n_bytes, 0, sf->field->n_bits);
+ bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, 0,
+ sf->field->n_bits);
}
}
static void
-encode_LOAD(const struct ovnact_load *load,
- const struct ovnact_encode_params *ep,
- struct ofpbuf *ofpacts)
-{
- ovnact_load_to_ofpact_set_field(load, ep->lookup_port, ep->aux,
- ofpact_put_SET_FIELD(ofpacts));
-}
-
-static void
free_LOAD(struct ovnact_load *load)
{
expr_constant_destroy(&load->imm, load_type(load));
@@ -674,17 +660,19 @@ encode_CT_COMMIT(const struct ovnact_ct_commit *cc,
ofpbuf_pull(ofpacts, set_field_offset);
if (cc->ct_mark_mask) {
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf_from_id(MFF_CT_MARK);
- sf->value.be32 = htonl(cc->ct_mark);
- sf->mask.be32 = htonl(cc->ct_mark_mask);
+ struct ofpact_set_field *sf
+ = ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK));
+
+ sf->value->be32 = htonl(cc->ct_mark);
+ ofpact_set_field_mask(sf)->be32 = htonl(cc->ct_mark_mask);
}
if (!ovs_be128_is_zero(cc->ct_label_mask)) {
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf_from_id(MFF_CT_LABEL);
- sf->value.be128 = cc->ct_label;
- sf->mask.be128 = cc->ct_label_mask;
+ struct ofpact_set_field *sf
+ = ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL));
+
+ sf->value->be128 = cc->ct_label;
+ ofpact_set_field_mask(sf)->be128 = cc->ct_label_mask;
}
ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index df2ff21..60eb182 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -913,23 +913,36 @@ execute_load(const struct ovnact_load *load,
const struct ovntrace_datapath *dp, struct flow *uflow,
struct ovs_list *super OVS_UNUSED)
{
- struct ofpact_set_field sf;
- memset(&sf, 0, sizeof sf);
- ovnact_load_to_ofpact_set_field(load, ovntrace_lookup_port, dp, &sf);
+ const struct ovnact_encode_params ep = {
+ .lookup_port = ovntrace_lookup_port,
+ .aux = dp,
+ };
+ uint64_t stub[512 / 8];
+ struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
- if (!mf_is_register(sf.field->id)) {
- struct ds s = DS_EMPTY_INITIALIZER;
- ovnacts_format(&load->ovnact, OVNACT_LOAD_SIZE, &s);
- ds_chomp(&s, ';');
+ ovnacts_encode(&load->ovnact, sizeof *load, &ep, &ofpacts);
- ovntrace_node_append(super, OVNTRACE_NODE_MODIFY, "%s", ds_cstr(&s));
+ struct ofpact *a;
+ OFPACT_FOR_EACH (a, ofpacts.data, ofpacts.size) {
+ struct ofpact_set_field *sf = ofpact_get_SET_FIELD(a);
- ds_destroy(&s);
- }
+ if (!mf_is_register(sf->field->id)) {
+ struct ds s = DS_EMPTY_INITIALIZER;
+ ovnacts_format(&load->ovnact, OVNACT_LOAD_SIZE, &s);
+ ds_chomp(&s, ';');
- if (mf_are_prereqs_ok(sf.field, uflow, NULL)) {
- mf_set_flow_value_masked(sf.field, &sf.value, &sf.mask, uflow);
+ ovntrace_node_append(super, OVNTRACE_NODE_MODIFY, "%s",
+ ds_cstr(&s));
+
+ ds_destroy(&s);
+ }
+
+ if (mf_are_prereqs_ok(sf->field, uflow, NULL)) {
+ mf_set_flow_value_masked(sf->field, sf->value,
+ ofpact_set_field_mask(sf), uflow);
+ }
}
+ ofpbuf_uninit(&ofpacts);
}
static void
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
