Previously we stored the netlink-formatted version of a flow key and
mask in 'struct udpif_key'. This patch stores the original key,mask
in the 'struct match' format, which reduces the size of a ukey from 1216
bytes to 560. Furthermore, this reduces netlink conversions required and
simplifies mask comparison in revalidate_ukey().
Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
For reference, prior to this patchset the size of a udpif_key is 656, but the
actions are not cached on master. This patch allows the entire patchset to have
a negligible impact on the memory footprint while boosting performance. Some
cases may even see an improvement in memory usage.
v6: First post.
---
ofproto/netflow.c | 3 +-
ofproto/netflow.h | 2 +-
ofproto/ofproto-dpif-upcall.c | 144 ++++++++++++++++-------------------------
ofproto/ofproto-dpif.c | 6 ++
ofproto/ofproto-dpif.h | 1 +
5 files changed, 65 insertions(+), 91 deletions(-)
diff --git a/ofproto/netflow.c b/ofproto/netflow.c
index 5ab522f..640846b 100644
--- a/ofproto/netflow.c
+++ b/ofproto/netflow.c
@@ -276,7 +276,8 @@ netflow_expire__(struct netflow *nf, struct netflow_flow
*nf_flow)
}
void
-netflow_flow_clear(struct netflow *nf, struct flow *flow) OVS_EXCLUDED(mutex)
+netflow_flow_clear(struct netflow *nf, const struct flow *flow)
+ OVS_EXCLUDED(mutex)
{
struct netflow_flow *nf_flow;
diff --git a/ofproto/netflow.h b/ofproto/netflow.h
index 94dd3ff..048eb20 100644
--- a/ofproto/netflow.h
+++ b/ofproto/netflow.h
@@ -52,7 +52,7 @@ void netflow_wait(struct netflow *);
void netflow_mask_wc(struct flow *, struct flow_wildcards *);
-void netflow_flow_clear(struct netflow *netflow, struct flow *flow);
+void netflow_flow_clear(struct netflow *netflow, const struct flow *flow);
void netflow_flow_update(struct netflow *nf, const struct flow *flow,
ofp_port_t output_iface,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 079e0e4..bab0f67 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -200,10 +200,7 @@ struct udpif_key {
/* These elements are read only once created, and therefore aren't
* protected by a mutex. */
- const struct nlattr *key; /* Datapath flow key. */
- size_t key_len; /* Length of 'key'. */
- const struct nlattr *mask; /* Datapath flow mask. */
- size_t mask_len; /* Length of 'mask'. */
+ struct match match; /* Flow key/mask. */
struct ofpbuf *actions; /* Datapath flow actions as nlattrs. */
ovs_u128 uid; /* Unique flow identifier. */
uint32_t hash; /* Pre-computed hash for 'key'. */
@@ -219,18 +216,16 @@ struct udpif_key {
struct xlate_cache *xcache OVS_GUARDED; /* Cache for xlate entries that
* are affected by this ukey.
* Used for stats and learning.*/
- union {
- struct odputil_keybuf buf;
- struct nlattr nla;
- } keybuf, maskbuf;
};
/* Datapath operation with optional ukey attached. */
struct ukey_op {
struct udpif_key *ukey;
struct dpif_flow_stats stats; /* Stats for 'op'. */
- struct odputil_uidbuf uid; /* Storage for Netlink-formatted uid. */
struct dpif_op dop; /* Flow operation. */
+ struct odputil_uidbuf uid; /* Storage for Netlink-formatted uid. */
+ struct odputil_keybuf key; /* Storage for Netlink-formatted key. */
+ struct odputil_keybuf mask; /* Storage for Netlink-formatted mask. */
};
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -1159,17 +1154,30 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
* already. */
if (may_put && upcall->type == DPIF_UC_MISS) {
struct udpif_key *ukey = upcall->ukey;
+ struct ofpbuf mask;
upcall->ukey = NULL;
op = &ops[n_ops++];
+ ofpbuf_use_stack(&mask, &op->mask, sizeof op->mask);
+ if (megaflow) {
+ size_t max_mpls;
+ bool recirc;
+
+ recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
+ max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
+ odp_flow_key_from_mask(&mask, &upcall->xout.wc.masks,
+ upcall->flow, UINT32_MAX, max_mpls,
+ recirc);
+ }
+
op->ukey = ukey;
op->dop.type = DPIF_OP_FLOW_PUT;
op->dop.u.flow_put.flags = DPIF_FP_CREATE;
- op->dop.u.flow_put.key = ukey->key;
- op->dop.u.flow_put.key_len = ukey->key_len;
- op->dop.u.flow_put.mask = ukey->mask;
- op->dop.u.flow_put.mask_len = ukey->mask_len;
+ op->dop.u.flow_put.key = upcall->key;
+ op->dop.u.flow_put.key_len = upcall->key_len;
+ op->dop.u.flow_put.mask = ofpbuf_data(&mask);
+ op->dop.u.flow_put.mask_len = ofpbuf_size(&mask);
op->dop.u.flow_put.uid = upcall->uid;
op->dop.u.flow_put.stats = NULL;
op->dop.u.flow_put.actions = ofpbuf_data(ukey->actions);
@@ -1243,34 +1251,10 @@ ukey_create(struct upcall *upcall)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
struct udpif_key *ukey = xmalloc(sizeof *ukey);
- struct ofpbuf key, mask;
- bool recirc, megaflow;
- ofpbuf_use_stack(&key, &ukey->keybuf, sizeof ukey->keybuf);
- if (upcall->key_len) {
- ofpbuf_put(&key, upcall->key, upcall->key_len);
- } else {
- /* dpif-netdev doesn't provide a netlink-formatted flow key in the
- * upcall, so convert the upcall's flow here. */
- odp_flow_key_from_flow(&key, upcall->flow, &upcall->xout.wc.masks,
- upcall->flow->in_port.odp_port, true);
- }
- ukey->key = ofpbuf_data(&key);
- ukey->key_len = ofpbuf_size(&key);
-
- atomic_read_relaxed(&enable_megaflows, &megaflow);
- recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
- ofpbuf_use_stack(&mask, &ukey->maskbuf, sizeof ukey->maskbuf);
- if (megaflow) {
- size_t max_mpls;
-
- max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
- odp_flow_key_from_mask(&mask, &upcall->xout.wc.masks,
- upcall->flow, UINT32_MAX, max_mpls,
- recirc);
- }
- ukey->mask = ofpbuf_data(&mask);
- ukey->mask_len = ofpbuf_size(&mask);
+ memcpy(&ukey->match.flow, upcall->flow, sizeof ukey->match.flow);
+ memcpy(&ukey->match.wc.masks, &upcall->xout.wc.masks,
+ sizeof ukey->match.wc.masks);
memcpy(&ukey->uid, upcall->uid, sizeof ukey->uid);
ukey->hash = get_uid_hash(&ukey->uid);
ukey->actions = ofpbuf_clone(&upcall->put_actions);
@@ -1305,19 +1289,18 @@ ukey_install_start(struct udpif *udpif, struct
udpif_key *new_ukey)
old_ukey = ukey_lookup(udpif, &new_ukey->uid);
if (old_ukey) {
/* Uncommon case: A ukey is already installed with the same UID. */
- if (old_ukey->key_len == new_ukey->key_len
- && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) {
+ if (flow_equal(&old_ukey->match.flow, &new_ukey->match.flow)) {
COVERAGE_INC(handler_duplicate_upcall);
} else {
struct ds ds = DS_EMPTY_INITIALIZER;
odp_format_uid(&old_ukey->uid, &ds);
ds_put_cstr(&ds, " ");
- odp_flow_key_format(old_ukey->key, old_ukey->key_len, &ds);
+ flow_format(&ds, &old_ukey->match.flow);
ds_put_cstr(&ds, "\n");
odp_format_uid(&new_ukey->uid, &ds);
ds_put_cstr(&ds, " ");
- odp_flow_key_format(new_ukey->key, new_ukey->key_len, &ds);
+ flow_format(&ds, &new_ukey->match.flow);
VLOG_WARN("Conflicting ukey for flows:\n%s", ds_cstr(&ds));
ds_destroy(&ds);
@@ -1448,13 +1431,10 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
struct ofproto_dpif *ofproto;
struct dpif_flow_stats push;
struct ofpbuf xout_actions;
- struct flow flow, dp_mask;
- uint32_t *dp32, *xout32;
ofp_port_t ofp_in_port;
struct xlate_in xin;
long long int last_used;
int error;
- size_t i;
bool ok;
bool need_revalidate;
@@ -1492,13 +1472,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
goto exit;
}
- if (odp_flow_key_to_flow(ukey->key, ukey->key_len, &flow)
- == ODP_FIT_ERROR) {
- goto exit;
- }
-
- error = xlate_lookup(udpif->backer, &flow, &ofproto, NULL, NULL, &netflow,
- &ofp_in_port);
+ error = xlate_lookup(udpif->backer, &ukey->match.flow, &ofproto, NULL,
+ NULL, &netflow, &ofp_in_port);
if (error) {
goto exit;
}
@@ -1510,8 +1485,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
ukey->xcache = xlate_cache_new();
}
- xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL, push.tcp_flags,
- NULL);
+ xlate_in_init(&xin, ofproto, &ukey->match.flow, ofp_in_port, NULL,
+ push.tcp_flags, NULL);
if (push.n_packets) {
xin.resubmit_stats = &push;
xin.may_learn = true;
@@ -1531,32 +1506,19 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
ofpbuf_size(xout.odp_actions));
} else {
ofpbuf_use_stack(&xout_actions, slow_path_buf, sizeof slow_path_buf);
- compose_slow_path(udpif, &xout, &flow, flow.in_port.odp_port,
- &xout_actions);
+ compose_slow_path(udpif, &xout, &ukey->match.flow,
+ ukey->match.flow.in_port.odp_port, &xout_actions);
}
if (!ofpbuf_equal(&xout_actions, ukey->actions)) {
goto exit;
}
- if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &flow)
- == ODP_FIT_ERROR) {
+ if (memcmp(&xout.wc.masks, &ukey->match.wc.masks,
+ sizeof ukey->match.wc.masks)) {
goto exit;
}
- /* Since the kernel is free to ignore wildcarded bits in the mask, we can't
- * directly check that the masks are the same. Instead we check that the
- * mask in the kernel is more specific i.e. less wildcarded, than what
- * we've calculated here. This guarantees we don't catch any packets we
- * shouldn't with the megaflow. */
- dp32 = (uint32_t *) &dp_mask;
- xout32 = (uint32_t *) &xout.wc.masks;
- for (i = 0; i < FLOW_U32S; i++) {
- if ((dp32[i] | xout32[i]) != dp32[i]) {
- goto exit;
- }
- }
-
ok = true;
exit:
@@ -1564,19 +1526,28 @@ exit:
ukey->reval_seq = reval_seq;
}
if (netflow && !ok) {
- netflow_flow_clear(netflow, &flow);
+ netflow_flow_clear(netflow, &ukey->match.flow);
}
xlate_out_uninit(xoutp);
return ok;
}
static void
-delete_op_init(struct ukey_op *op, struct udpif_key *ukey, bool terse_dump)
+delete_op_init(struct udpif *udpif, struct ukey_op *op, struct udpif_key *ukey,
+ bool terse_dump)
{
+ struct ofpbuf key;
+ bool recirc;
+
+ recirc = dpif_backer_get_enable_recirc(udpif->backer);
+ ofpbuf_use_stack(&key, &op->key, sizeof op->key);
+ odp_flow_key_from_flow(&key, &ukey->match.flow, &ukey->match.wc.masks,
+ ukey->match.flow.in_port.odp_port, recirc);
+
op->ukey = ukey;
op->dop.type = DPIF_OP_FLOW_DEL;
- op->dop.u.flow_del.key = ukey->key;
- op->dop.u.flow_del.key_len = ukey->key_len;
+ op->dop.u.flow_del.key = ofpbuf_data(&key);
+ op->dop.u.flow_del.key_len = ofpbuf_size(&key);
op->dop.u.flow_del.uid = &ukey->uid;
op->dop.u.flow_del.terse = terse_dump;
op->dop.u.flow_del.stats = &op->stats;
@@ -1610,10 +1581,10 @@ push_ukey_ops__(struct udpif *udpif, struct ukey_op
*ops, size_t n_ops)
ovs_mutex_unlock(&op->ukey->mutex);
if (push->n_packets || netflow_exists()) {
+ const struct flow *flow = &op->ukey->match.flow;
struct ofproto_dpif *ofproto;
struct netflow *netflow;
ofp_port_t ofp_in_port;
- struct flow flow;
int error;
ovs_mutex_lock(&op->ukey->mutex);
@@ -1624,17 +1595,12 @@ push_ukey_ops__(struct udpif *udpif, struct ukey_op
*ops, size_t n_ops)
}
ovs_mutex_unlock(&op->ukey->mutex);
- if (odp_flow_key_to_flow(op->ukey->key, op->ukey->key_len, &flow)
- == ODP_FIT_ERROR) {
- continue;
- }
-
- error = xlate_lookup(udpif->backer, &flow, &ofproto,
- NULL, NULL, &netflow, &ofp_in_port);
+ error = xlate_lookup(udpif->backer, flow, &ofproto, NULL, NULL,
+ &netflow, &ofp_in_port);
if (!error) {
struct xlate_in xin;
- xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL,
+ xlate_in_init(&xin, ofproto, flow, ofp_in_port, NULL,
push->tcp_flags, NULL);
xin.resubmit_stats = push->n_packets ? push : NULL;
xin.may_learn = push->n_packets > 0;
@@ -1642,7 +1608,7 @@ push_ukey_ops__(struct udpif *udpif, struct ukey_op *ops,
size_t n_ops)
xlate_actions_for_side_effects(&xin);
if (netflow) {
- netflow_flow_clear(netflow, &flow);
+ netflow_flow_clear(netflow, flow);
}
}
}
@@ -1769,7 +1735,7 @@ revalidate(struct revalidator *revalidator)
ukey->flow_exists = keep;
if (!keep) {
- delete_op_init(&ops[n_ops++], ukey, terse_dump);
+ delete_op_init(udpif, &ops[n_ops++], ukey, terse_dump);
}
ovs_mutex_unlock(&ukey->mutex);
}
@@ -1840,7 +1806,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
ukey)))) {
struct ukey_op *op = &ops[n_ops++];
- delete_op_init(op, ukey, terse_dump);
+ delete_op_init(udpif, op, ukey, terse_dump);
if (n_ops == REVALIDATE_MAX_BATCH) {
push_ukey_ops(udpif, umap, ops, n_ops);
n_ops = 0;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6749d9d..c08bbd7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -366,6 +366,12 @@ ofproto_dpif_get_enable_recirc(const struct ofproto_dpif
*ofproto)
}
bool
+dpif_backer_get_enable_recirc(const struct dpif_backer *backer)
+{
+ return backer->enable_recirc;
+}
+
+bool
ofproto_dpif_get_enable_uid(const struct ofproto_dpif *ofproto)
{
return ofproto->backer->enable_uid;
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 7db18d4..1752dda 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -88,6 +88,7 @@ enum rule_dpif_lookup_verdict {
size_t ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *);
bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *);
bool ofproto_dpif_get_enable_uid(const struct ofproto_dpif *);
+bool dpif_backer_get_enable_recirc(const struct dpif_backer *);
bool dpif_backer_get_enable_uid(const struct dpif_backer *);
uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
