[ovs-dev] [PATCH] odp-util: Convert flow serialization parameters to a struct.
Jesse Gross
jesse at nicira.com
Wed Jun 17 17:41:51 UTC 2015
Serializing between userspace flows and netlink attributes currently
requires several additional parameters besides the flows themselves.
This will continue to grow in the future as well. This converts
the function arguments to a parameters struct, which makes the code
easier to read and allowing irrelevant arguments to be omitted.
Signed-off-by: Jesse Gross <jesse at nicira.com>
---
lib/dpif-netdev.c | 24 ++++++++++++++------
lib/odp-util.c | 53 ++++++++++++++++---------------------------
lib/odp-util.h | 31 ++++++++++++++++++++-----
lib/tnl-ports.c | 16 +++++++++----
ofproto/ofproto-dpif-upcall.c | 17 +++++++++-----
ofproto/ofproto-dpif.c | 16 ++++++++++---
tests/test-odp.c | 9 ++++++--
7 files changed, 103 insertions(+), 63 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index f13169c..e8ffcd7 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1815,22 +1815,27 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
struct flow_wildcards wc;
struct dp_netdev_actions *actions;
size_t offset;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &netdev_flow->flow,
+ .mask = &wc.masks,
+ .recirc = true,
+ .max_mpls_depth = SIZE_MAX,
+ };
miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
/* Key */
offset = key_buf->size;
flow->key = ofpbuf_tail(key_buf);
- odp_flow_key_from_flow(key_buf, &netdev_flow->flow, &wc.masks,
- netdev_flow->flow.in_port.odp_port, true);
+ odp_parms.odp_in_port = netdev_flow->flow.in_port.odp_port;
+ odp_flow_key_from_flow(&odp_parms, key_buf);
flow->key_len = key_buf->size - offset;
/* Mask */
offset = mask_buf->size;
flow->mask = ofpbuf_tail(mask_buf);
- odp_flow_key_from_mask(mask_buf, &wc.masks, &netdev_flow->flow,
- odp_to_u32(wc.masks.in_port.odp_port),
- SIZE_MAX, true);
+ odp_parms.odp_in_port = wc.masks.in_port.odp_port;
+ odp_flow_key_from_mask(&odp_parms, mask_buf);
flow->mask_len = mask_buf->size - offset;
/* Actions */
@@ -3008,10 +3013,15 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, struct dp_packet *packet_,
struct ds ds = DS_EMPTY_INITIALIZER;
char *packet_str;
struct ofpbuf key;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = flow,
+ .mask = &wc->masks,
+ .odp_in_port = flow->in_port.odp_port,
+ .recirc = true,
+ };
ofpbuf_init(&key, 0);
- odp_flow_key_from_flow(&key, flow, &wc->masks, flow->in_port.odp_port,
- true);
+ odp_flow_key_from_flow(&odp_parms, &key);
packet_str = ofp_packet_to_string(dp_packet_data(packet_),
dp_packet_size(packet_));
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 76dc44b..56979ac 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3417,13 +3417,13 @@ static void get_tp_key(const struct flow *, union ovs_key_tp *);
static void put_tp_key(const union ovs_key_tp *, struct flow *);
static void
-odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
- const struct flow *mask, odp_port_t odp_in_port,
- size_t max_mpls_depth, bool recirc, bool export_mask)
+odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms,
+ bool export_mask, struct ofpbuf *buf)
{
struct ovs_key_ethernet *eth_key;
size_t encap;
- const struct flow *data = export_mask ? mask : flow;
+ const struct flow *flow = parms->flow;
+ const struct flow *data = export_mask ? parms->mask : parms->flow;
nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, data->skb_priority);
@@ -3433,15 +3433,15 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
- if (recirc) {
+ if (parms->recirc) {
nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID, data->recirc_id);
nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, data->dp_hash);
}
/* Add an ingress port attribute if this is a mask or 'odp_in_port'
* is not the magical value "ODPP_NONE". */
- if (export_mask || odp_in_port != ODPP_NONE) {
- nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, odp_in_port);
+ if (export_mask || parms->odp_in_port != ODPP_NONE) {
+ nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, parms->odp_in_port);
}
eth_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_ETHERNET,
@@ -3507,7 +3507,9 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
int i, n;
n = flow_count_mpls_labels(flow, NULL);
- n = MIN(n, max_mpls_depth);
+ if (export_mask) {
+ n = MIN(n, parms->max_mpls_depth);
+ }
mpls_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_MPLS,
n * sizeof *mpls_key);
for (i = 0; i < n; i++) {
@@ -3579,43 +3581,26 @@ unencap:
}
/* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'.
- * 'flow->in_port' is ignored (since it is likely to be an OpenFlow port
- * number rather than a datapath port number). Instead, if 'odp_in_port'
- * is anything other than ODPP_NONE, it is included in 'buf' as the input
- * port.
*
* 'buf' must have at least ODPUTIL_FLOW_KEY_BYTES bytes of space, or be
- * capable of being expanded to allow for that much space.
- *
- * 'recirc' indicates support for recirculation fields. If this is true, then
- * these fields will always be serialised. */
+ * capable of being expanded to allow for that much space. */
void
-odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow,
- const struct flow *mask, odp_port_t odp_in_port,
- bool recirc)
+odp_flow_key_from_flow(const struct odp_flow_key_parms *parms,
+ struct ofpbuf *buf)
{
- odp_flow_key_from_flow__(buf, flow, mask, odp_in_port, SIZE_MAX, recirc,
- false);
+ odp_flow_key_from_flow__(parms, false, buf);
}
/* Appends a representation of 'mask' as OVS_KEY_ATTR_* attributes to
- * 'buf'. 'flow' is used as a template to determine how to interpret
- * 'mask'. For example, the 'dl_type' of 'mask' describes the mask, but
- * it doesn't indicate whether the other fields should be interpreted as
- * ARP, IPv4, IPv6, etc.
+ * 'buf'.
*
* 'buf' must have at least ODPUTIL_FLOW_KEY_BYTES bytes of space, or be
- * capable of being expanded to allow for that much space.
- *
- * 'recirc' indicates support for recirculation fields. If this is true, then
- * these fields will always be serialised. */
+ * capable of being expanded to allow for that much space. */
void
-odp_flow_key_from_mask(struct ofpbuf *buf, const struct flow *mask,
- const struct flow *flow, uint32_t odp_in_port_mask,
- size_t max_mpls_depth, bool recirc)
+odp_flow_key_from_mask(const struct odp_flow_key_parms *parms,
+ struct ofpbuf *buf)
{
- odp_flow_key_from_flow__(buf, flow, mask, u32_to_odp(odp_in_port_mask),
- max_mpls_depth, recirc, true);
+ odp_flow_key_from_flow__(parms, true, buf);
}
/* Generate ODP flow key from the given packet metadata */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 4f0e794..59d29f3 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -158,12 +158,31 @@ int odp_flow_from_string(const char *s,
const struct simap *port_names,
struct ofpbuf *, struct ofpbuf *);
-void odp_flow_key_from_flow(struct ofpbuf *, const struct flow * flow,
- const struct flow *mask, odp_port_t odp_in_port,
- bool recirc);
-void odp_flow_key_from_mask(struct ofpbuf *, const struct flow *mask,
- const struct flow *flow, uint32_t odp_in_port,
- size_t max_mpls_depth, bool recirc);
+struct odp_flow_key_parms {
+ /* The flow and mask to be serialized. In the case of masks, 'flow'
+ * is used as a template to determine how to interpret 'mask'. For
+ * example, the 'dl_type' of 'mask' describes the mask, but it doesn't
+ * indicate whether the other fields should be interpreted as ARP, IPv4,
+ * IPv6, etc. */
+ const struct flow *flow;
+ const struct flow *mask;
+
+ /* 'flow->in_port' is ignored (since it is likely to be an OpenFlow port
+ * number rather than a datapath port number). Instead, if 'odp_in_port'
+ * is anything other than ODPP_NONE, it is included in 'buf' as the input
+ * port. */
+ odp_port_t odp_in_port;
+
+ /* Indicates support for recirculation fields. If this is true, then
+ * these fields will always be serialised. */
+ bool recirc;
+
+ /* Only used for mask translation: */
+ size_t max_mpls_depth;
+};
+
+void odp_flow_key_from_flow(const struct odp_flow_key_parms *, struct ofpbuf *);
+void odp_flow_key_from_mask(const struct odp_flow_key_parms *, struct ofpbuf *);
uint32_t odp_flow_key_hash(const struct nlattr *, size_t);
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 2602db5..a0a73c8 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -161,22 +161,28 @@ tnl_port_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
size_t key_len, mask_len;
struct flow_wildcards wc;
struct ofpbuf buf;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &flow,
+ .mask = &wc.masks,
+ };
ds_put_format(&ds, "%s (%"PRIu32") : ", p->dev_name, p->portno);
minimask_expand(&p->cr.match.mask, &wc);
miniflow_expand(&p->cr.match.flow, &flow);
/* Key. */
+ odp_parms.odp_in_port = flow.in_port.odp_port;
+ odp_parms.recirc = true;
ofpbuf_use_stack(&buf, &keybuf, sizeof keybuf);
- odp_flow_key_from_flow(&buf, &flow, &wc.masks,
- flow.in_port.odp_port, true);
+ odp_flow_key_from_flow(&odp_parms, &buf);
key = buf.data;
key_len = buf.size;
+
/* mask*/
+ odp_parms.odp_in_port = wc.masks.in_port.odp_port;
+ odp_parms.recirc = false;
ofpbuf_use_stack(&buf, &maskbuf, sizeof maskbuf);
- odp_flow_key_from_mask(&buf, &wc.masks, &flow,
- odp_to_u32(wc.masks.in_port.odp_port),
- SIZE_MAX, false);
+ odp_flow_key_from_mask(&odp_parms, &buf);
mask = buf.data;
mask_len = buf.size;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index c39b571..f75bc69 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1363,6 +1363,10 @@ ukey_create_from_upcall(struct upcall *upcall)
struct odputil_keybuf keystub, maskstub;
struct ofpbuf keybuf, maskbuf;
bool recirc, megaflow;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = upcall->flow,
+ .mask = &upcall->xout.wc.masks,
+ };
if (upcall->key_len) {
ofpbuf_use_const(&keybuf, upcall->key, upcall->key_len);
@@ -1370,19 +1374,20 @@ ukey_create_from_upcall(struct upcall *upcall)
/* dpif-netdev doesn't provide a netlink-formatted flow key in the
* upcall, so convert the upcall's flow here. */
ofpbuf_use_stack(&keybuf, &keystub, sizeof keystub);
- odp_flow_key_from_flow(&keybuf, upcall->flow, &upcall->xout.wc.masks,
- upcall->flow->in_port.odp_port, true);
+ odp_parms.odp_in_port = upcall->flow->in_port.odp_port;
+ odp_parms.recirc = true;
+ odp_flow_key_from_flow(&odp_parms, &keybuf);
}
atomic_read_relaxed(&enable_megaflows, &megaflow);
recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
ofpbuf_use_stack(&maskbuf, &maskstub, sizeof maskstub);
if (megaflow) {
- size_t max_mpls;
+ odp_parms.odp_in_port = ODPP_NONE;
+ odp_parms.max_mpls_depth = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
+ odp_parms.recirc = recirc;
- max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
- odp_flow_key_from_mask(&maskbuf, &upcall->xout.wc.masks, upcall->flow,
- UINT32_MAX, max_mpls, recirc);
+ odp_flow_key_from_mask(&odp_parms, &maskbuf);
}
return ukey_create__(keybuf.data, keybuf.size, maskbuf.data, maskbuf.size,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 369e0b9..0de8686 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1003,13 +1003,17 @@ check_recirc(struct dpif_backer *backer)
struct odputil_keybuf keybuf;
struct ofpbuf key;
bool enable_recirc;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &flow,
+ .recirc = true,
+ };
memset(&flow, 0, sizeof flow);
flow.recirc_id = 1;
flow.dp_hash = 1;
ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
- odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+ odp_flow_key_from_flow(&odp_parms, &key);
enable_recirc = dpif_probe_feature(backer->dpif, "recirculation", &key,
NULL);
@@ -1037,12 +1041,15 @@ check_ufid(struct dpif_backer *backer)
struct ofpbuf key;
ovs_u128 ufid;
bool enable_ufid;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &flow,
+ };
memset(&flow, 0, sizeof flow);
flow.dl_type = htons(0x1234);
ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
- odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+ odp_flow_key_from_flow(&odp_parms, &key);
dpif_flow_hash(backer->dpif, key.data, key.size, &ufid);
enable_ufid = dpif_probe_feature(backer->dpif, "UFID", &key, &ufid);
@@ -1144,13 +1151,16 @@ check_max_mpls_depth(struct dpif_backer *backer)
for (n = 0; n < FLOW_MAX_MPLS_LABELS; n++) {
struct odputil_keybuf keybuf;
struct ofpbuf key;
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &flow,
+ };
memset(&flow, 0, sizeof flow);
flow.dl_type = htons(ETH_TYPE_MPLS);
flow_set_mpls_bos(&flow, n, 1);
ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
- odp_flow_key_from_flow(&key, &flow, NULL, 0, false);
+ odp_flow_key_from_flow(&odp_parms, &key);
if (!dpif_probe_feature(backer->dpif, "MPLS", &key, NULL)) {
break;
}
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 4daf472..faa60d4 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -54,6 +54,11 @@ parse_keys(bool wc_keys)
}
if (!wc_keys) {
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &flow,
+ .recirc = true,
+ };
+
/* Convert odp_key to flow. */
fitness = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
switch (fitness) {
@@ -75,8 +80,8 @@ parse_keys(bool wc_keys)
/* Convert cls_rule back to odp_key. */
ofpbuf_uninit(&odp_key);
ofpbuf_init(&odp_key, 0);
- odp_flow_key_from_flow(&odp_key, &flow, NULL,
- flow.in_port.odp_port, true);
+ odp_parms.odp_in_port = flow.in_port.odp_port;
+ odp_flow_key_from_flow(&odp_parms, &odp_key);
if (odp_key.size > ODPUTIL_FLOW_KEY_BYTES) {
printf ("too long: %"PRIu32" > %d\n",
--
2.1.0
More information about the dev
mailing list