Skip to content

Commit

Permalink
odp-util: Convert flow serialization parameters to a struct.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Andy Zhou <[email protected]>
  • Loading branch information
@jessegross
jessegross committed Jun 18, 2015
1 parent c4c7e59 commit 5262eea
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 63 deletions.
24 changes: 17 additions & 7 deletions lib/dpif-netdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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_));

Expand Down
53 changes: 19 additions & 34 deletions lib/odp-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -3579,43 +3581,26 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
}

/* 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 */
Expand Down
31 changes: 25 additions & 6 deletions lib/odp-util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
16 changes: 11 additions & 5 deletions lib/tnl-ports.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
17 changes: 11 additions & 6 deletions ofproto/ofproto-dpif-upcall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1363,26 +1363,31 @@ 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);
} else {
/* 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,
Expand Down
16 changes: 13 additions & 3 deletions ofproto/ofproto-dpif.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 7 additions & 2 deletions tests/test-odp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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",
Expand Down

0 comments on commit 5262eea

Please sign in to comment.