[ovs-dev] [PATCH v4 3/4] lib/odp: Use masked set actions.
Ben Pfaff
blp at nicira.com
Tue Aug 12 22:45:16 UTC 2014
On Mon, Aug 11, 2014 at 09:15:00AM -0700, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
The amount of redundancy in the code at this point is starting to give
me a headache. How about a pattern like this (provided as an
incremental)?
diff --git a/lib/odp-util.c b/lib/odp-util.c
index b9b6e2a..b4ea27f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3649,47 +3649,62 @@ commit_odp_tunnel_action(const struct flow *flow, struct flow *base,
}
}
-static void
-commit_set_ether_addr_action(const struct flow *flow, struct flow *base,
- struct ofpbuf *odp_actions,
- struct flow_wildcards *wc,
- bool use_masked)
+static bool
+commit(enum ovs_key_attr attr, bool use_masked_set,
+ const void *key, void *base, void *mask, size_t size,
+ struct ofpbuf *odp_actions)
{
- struct ovs_key_ethernet key, mask;
- bool fully_masked;
-
- /* Mask bits are set when we have either read or set the corresponding
- * values. Masked bits will be exact-matched, no need to set them
- * if the value did not actually change. */
- if (eth_addr_equals(flow->dl_src, base->dl_src) &&
- eth_addr_equals(flow->dl_dst, base->dl_dst)) {
- return;
+ if (memcmp(key, base, size)) {
+ bool fully_masked = is_all_ones(mask, size);
+ if (use_masked_set && !fully_masked) {
+ commit_masked_set_action(odp_actions, attr, key, mask, size);
+ } else {
+ if (!fully_masked) {
+ memset(mask, 0xff, size);
+ }
+ commit_set_action(odp_actions, attr, key, size);
+ }
+ memcpy(base, key, size);
+ return true;
+ } else {
+ /* Mask bits are set when we have either read or set the corresponding
+ * values. Masked bits will be exact-matched, no need to set them
+ * if the value did not actually change. */
+ return false;
}
+}
- memcpy(key.eth_src, flow->dl_src, ETH_ADDR_LEN);
- memcpy(key.eth_dst, flow->dl_dst, ETH_ADDR_LEN);
+static void
+get_ethernet_key(const struct flow *flow, struct ovs_key_ethernet *eth)
+{
+ memcpy(eth->eth_src, flow->dl_src, ETH_ADDR_LEN);
+ memcpy(eth->eth_dst, flow->dl_dst, ETH_ADDR_LEN);
+}
- fully_masked = eth_mask_is_exact(wc->masks.dl_src)
- && eth_mask_is_exact(wc->masks.dl_dst);
+static void
+put_ethernet_key(const struct ovs_key_ethernet *eth, struct flow *flow)
+{
+ memcpy(flow->dl_src, eth->eth_src, ETH_ADDR_LEN);
+ memcpy(flow->dl_dst, eth->eth_dst, ETH_ADDR_LEN);
+}
- if (use_masked && !fully_masked) {
- memcpy(mask.eth_src, wc->masks.dl_src, ETH_ADDR_LEN);
- memcpy(mask.eth_dst, wc->masks.dl_dst, ETH_ADDR_LEN);
+static void
+commit_set_ether_addr_action(const struct flow *flow, struct flow *base_flow,
+ struct ofpbuf *odp_actions,
+ struct flow_wildcards *wc,
+ bool use_masked)
+{
+ struct ovs_key_ethernet key, base, mask;
- commit_masked_set_action(odp_actions, OVS_KEY_ATTR_ETHERNET, &key,
- &mask, sizeof key);
- } else {
- if (!fully_masked) {
- memset(&wc->masks.dl_src, 0xff, sizeof wc->masks.dl_src);
- memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
- }
+ get_ethernet_key(flow, &key);
+ get_ethernet_key(base_flow, &base);
+ get_ethernet_key(&wc->masks, &mask);
- commit_set_action(odp_actions, OVS_KEY_ATTR_ETHERNET, &key,
- sizeof key);
+ if (commit(OVS_KEY_ATTR_ETHERNET, use_masked,
+ &key, &base, &mask, sizeof key, odp_actions)) {
+ put_ethernet_key(&base, base_flow);
+ put_ethernet_key(&mask, &wc->masks);
}
-
- memcpy(base->dl_src, flow->dl_src, ETH_ADDR_LEN);
- memcpy(base->dl_dst, flow->dl_dst, ETH_ADDR_LEN);
}
static void
@@ -3791,61 +3806,52 @@ commit_mpls_action(const struct flow *flow, struct flow *base,
}
static void
-commit_set_ipv4_action(const struct flow *flow, struct flow *base,
- struct ofpbuf *odp_actions, struct flow_wildcards *wc,
- bool use_masked)
+get_ipv4_key(const struct flow *flow, struct ovs_key_ipv4 *ipv4)
{
- struct ovs_key_ipv4 key, mask;
-
- /* Check that non-masked bits are intact, and that nw_proto and nw_frag
- * remain unchanged. */
- ovs_assert(!((flow->nw_src ^ base->nw_src) & ~wc->masks.nw_src)
- && !((flow->nw_dst ^ base->nw_dst) & ~wc->masks.nw_dst)
- && !((flow->nw_tos ^ base->nw_tos) & ~wc->masks.nw_tos)
- && !((flow->nw_ttl ^ base->nw_ttl) & ~wc->masks.nw_ttl)
- && flow->nw_proto == base->nw_proto
- && flow->nw_frag == base->nw_frag);
+ ipv4->ipv4_src = flow->nw_src;
+ ipv4->ipv4_dst = flow->nw_dst;
+ ipv4->ipv4_proto = flow->nw_proto;
+ ipv4->ipv4_tos = flow->nw_tos;
+ ipv4->ipv4_ttl = flow->nw_ttl;
+ ipv4->ipv4_frag = ovs_to_odp_frag(flow->nw_frag);
+}
- /* Mask bits are set when we have either read or set the corresponding
- * values. Masked bits will be exact-matched, no need to set them
- * if the value did not actually change. */
- if ((flow->nw_src == base->nw_src) && (flow->nw_dst == base->nw_dst) &&
- (flow->nw_tos == base->nw_tos) && (flow->nw_ttl == base->nw_ttl)) {
- return;
+static void
+put_ipv4_key(const struct ovs_key_ipv4 *ipv4, struct flow *flow)
+{
+ flow->nw_src = ipv4->ipv4_src;
+ flow->nw_dst = ipv4->ipv4_dst;
+ flow->nw_proto = ipv4->ipv4_proto;
+ flow->nw_tos = ipv4->ipv4_tos;
+ flow->nw_ttl = ipv4->ipv4_ttl;
+ if (ipv4->ipv4_frag == 0xff) {
+ flow->nw_frag = 0xff;
+ } else {
+ odp_to_ovs_frag(ipv4->ipv4_frag, flow);
}
+}
+
+static void
+commit_set_ipv4_action(const struct flow *flow, struct flow *base_flow,
+ struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+ bool use_masked)
+{
+ struct ovs_key_ipv4 key, mask, base;
- key.ipv4_src = flow->nw_src;
- key.ipv4_dst = flow->nw_dst;
- key.ipv4_tos = flow->nw_tos;
- key.ipv4_ttl = flow->nw_ttl;
- key.ipv4_proto = base->nw_proto;
- key.ipv4_frag = ovs_to_odp_frag(base->nw_frag);
+ /* Check that nw_proto and nw_frag remain unchanged. */
+ ovs_assert(flow->nw_proto == base_flow->nw_proto &&
+ flow->nw_frag == base_flow->nw_frag);
- if (use_masked) {
- mask.ipv4_src = wc->masks.nw_src;
- mask.ipv4_dst = wc->masks.nw_dst;
- mask.ipv4_tos = wc->masks.nw_tos;
- mask.ipv4_ttl = wc->masks.nw_ttl;
- mask.ipv4_proto = 0; /* Not writeable. */
- mask.ipv4_frag = 0; /* Not writeable. */
-
- commit_masked_set_action(odp_actions, OVS_KEY_ATTR_IPV4, &key, &mask,
- sizeof key);
- } else {
- memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
- memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
- memset(&wc->masks.nw_tos, 0xff, sizeof wc->masks.nw_tos);
- memset(&wc->masks.nw_ttl, 0xff, sizeof wc->masks.nw_ttl);
- memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
- memset(&wc->masks.nw_frag, 0xff, sizeof wc->masks.nw_frag);
+ get_ipv4_key(flow, &key);
+ get_ipv4_key(base_flow, &base);
+ get_ipv4_key(&wc->masks, &mask);
+ mask.ipv4_frag = 0; /* Not writable. */
- commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4, &key, sizeof key);
+ if (commit(OVS_KEY_ATTR_IPV4, use_masked, &key, &base, &mask, sizeof key,
+ odp_actions)) {
+ put_ipv4_key(&base, base_flow);
+ put_ipv4_key(&mask, &wc->masks);
}
-
- base->nw_src = flow->nw_src;
- base->nw_dst = flow->nw_dst;
- base->nw_tos = flow->nw_tos;
- base->nw_ttl = flow->nw_ttl;
}
static void
More information about the dev
mailing list