Until now, the optional OVS_USERSPACE_ATTR_USERDATA attribute had to be
exactly 64 bits long, if it was present. However, 64 bits is not enough
space to associate as much information with a flow as would be convenient
for some userspace features now under development. This commit generalizes
the attribute, allowing it to be any length.
This generalization is backward-compatible: if userspace only uses 64-bit
attributes, then it will not see any change in behavior.
CC: Romain Lenglet <rleng...@vmware.com>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
This passes all of the unit tests. I only build-tested the kernel
module.
datapath/datapath.c | 9 ++---
datapath/datapath.h | 2 +-
include/linux/openvswitch.h | 11 +++---
lib/dpif-linux.c | 6 +--
lib/dpif-netdev.c | 74 +++++++++++++++++++++++++------------------
lib/dpif.h | 10 +++---
lib/odp-util.c | 61 ++++++++++++++++++++++++++---------
lib/odp-util.h | 2 +-
ofproto/ofproto-dpif.c | 20 +++++++++---
tests/odp.at | 1 +
10 files changed, 123 insertions(+), 73 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 04a5e7f..2a542da 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -374,8 +374,8 @@ static int queue_userspace_packet(struct net *net, int
dp_ifindex,
len = sizeof(struct ovs_header);
len += nla_total_size(skb->len);
len += nla_total_size(FLOW_BUFSIZE);
- if (upcall_info->cmd == OVS_PACKET_CMD_ACTION)
- len += nla_total_size(8);
+ if (upcall_info->userdata)
+ len += NLA_ALIGN(upcall_info->userdata->nla_len);
user_skb = genlmsg_new(len, GFP_ATOMIC);
if (!user_skb) {
@@ -392,8 +392,8 @@ static int queue_userspace_packet(struct net *net, int
dp_ifindex,
nla_nest_end(user_skb, nla);
if (upcall_info->userdata)
- nla_put_u64(user_skb, OVS_PACKET_ATTR_USERDATA,
- nla_get_u64(upcall_info->userdata));
+ nla_append(user_skb, NLA_ALIGN(upcall_info->userdata->nla_len),
+ upcall_info->userdata);
nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
@@ -678,7 +678,6 @@ static int validate_userspace(const struct nlattr *attr)
{
static const struct nla_policy userspace_policy[OVS_USERSPACE_ATTR_MAX
+ 1] = {
[OVS_USERSPACE_ATTR_PID] = {.type = NLA_U32 },
- [OVS_USERSPACE_ATTR_USERDATA] = {.type = NLA_U64 },
};
struct nlattr *a[OVS_USERSPACE_ATTR_MAX + 1];
int error;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 2b93348..9bc98fb 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -120,7 +120,7 @@ struct ovs_skb_cb {
* struct dp_upcall - metadata to include with a packet to send to userspace
* @cmd: One of %OVS_PACKET_CMD_*.
* @key: Becomes %OVS_PACKET_ATTR_KEY. Must be nonnull.
- * @userdata: If nonnull, its u64 value is extracted and passed to userspace as
+ * @userdata: If nonnull, its variable-length value is passed to userspace as
* %OVS_PACKET_ATTR_USERDATA.
* @portid: Netlink PID to which packet should be sent. If @portid is 0 then
no
* packet is sent and the packet is accounted in the datapath's @n_lost
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index b12bf0c..b46015d 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -148,7 +148,8 @@ enum ovs_packet_cmd {
* for %OVS_PACKET_CMD_EXECUTE. It has nested %OVS_ACTION_ATTR_* attributes.
* @OVS_PACKET_ATTR_USERDATA: Present for an %OVS_PACKET_CMD_ACTION
* notification if the %OVS_ACTION_ATTR_USERSPACE action specified an
- * %OVS_USERSPACE_ATTR_USERDATA attribute.
+ * %OVS_USERSPACE_ATTR_USERDATA attribute, with the same length and content
+ * specified there.
*
* These attributes follow the &struct ovs_header within the Generic Netlink
* payload for %OVS_PACKET_* commands.
@@ -158,7 +159,7 @@ enum ovs_packet_attr {
OVS_PACKET_ATTR_PACKET, /* Packet data. */
OVS_PACKET_ATTR_KEY, /* Nested OVS_KEY_ATTR_* attributes. */
OVS_PACKET_ATTR_ACTIONS, /* Nested OVS_ACTION_ATTR_* attributes. */
- OVS_PACKET_ATTR_USERDATA, /* u64 OVS_ACTION_ATTR_USERSPACE arg. */
+ OVS_PACKET_ATTR_USERDATA, /* OVS_ACTION_ATTR_USERSPACE arg. */
__OVS_PACKET_ATTR_MAX
};
@@ -442,13 +443,13 @@ enum ovs_sample_attr {
* enum ovs_userspace_attr - Attributes for %OVS_ACTION_ATTR_USERSPACE action.
* @OVS_USERSPACE_ATTR_PID: u32 Netlink PID to which the %OVS_PACKET_CMD_ACTION
* message should be sent. Required.
- * @OVS_USERSPACE_ATTR_USERDATA: If present, its u64 argument is copied to the
- * %OVS_PACKET_CMD_ACTION message as %OVS_PACKET_ATTR_USERDATA,
+ * @OVS_USERSPACE_ATTR_USERDATA: If present, its variable-length argument is
+ * copied to the %OVS_PACKET_CMD_ACTION message as %OVS_PACKET_ATTR_USERDATA.
*/
enum ovs_userspace_attr {
OVS_USERSPACE_ATTR_UNSPEC,
OVS_USERSPACE_ATTR_PID, /* u32 Netlink PID to receive upcalls. */
- OVS_USERSPACE_ATTR_USERDATA, /* u64 optional user-specified cookie. */
+ OVS_USERSPACE_ATTR_USERDATA, /* Optional user-specified cookie. */
__OVS_USERSPACE_ATTR_MAX
};
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index b6eba39..2659ea6 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -1225,7 +1225,7 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall
*upcall,
[OVS_PACKET_ATTR_KEY] = { .type = NL_A_NESTED },
/* OVS_PACKET_CMD_ACTION only. */
- [OVS_PACKET_ATTR_USERDATA] = { .type = NL_A_U64, .optional = true },
+ [OVS_PACKET_ATTR_USERDATA] = { .type = NL_A_UNSPEC, .optional = true },
};
struct ovs_header *ovs_header;
@@ -1263,9 +1263,7 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall
*upcall,
upcall->key = CONST_CAST(struct nlattr *,
nl_attr_get(a[OVS_PACKET_ATTR_KEY]));
upcall->key_len = nl_attr_get_size(a[OVS_PACKET_ATTR_KEY]);
- upcall->userdata = (a[OVS_PACKET_ATTR_USERDATA]
- ? nl_attr_get_u64(a[OVS_PACKET_ATTR_USERDATA])
- : 0);
+ upcall->userdata = a[OVS_PACKET_ATTR_USERDATA];
*dp_ifindex = ovs_header->dp_ifindex;
return 0;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 038dfdc..30fcb03 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -151,7 +151,7 @@ static int dpif_netdev_open(const struct dpif_class *,
const char *name,
bool create, struct dpif **);
static int dp_netdev_output_userspace(struct dp_netdev *, const struct ofpbuf
*,
int queue_no, const struct flow *,
- uint64_t arg);
+ const struct nlattr *userdata);
static void dp_netdev_execute_actions(struct dp_netdev *,
struct ofpbuf *, struct flow *,
const struct nlattr *actions,
@@ -1043,7 +1043,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct
dp_netdev_port *port,
dp->n_hit++;
} else {
dp->n_missed++;
- dp_netdev_output_userspace(dp, packet, DPIF_UC_MISS, &key, 0);
+ dp_netdev_output_userspace(dp, packet, DPIF_UC_MISS, &key, NULL);
}
}
@@ -1108,37 +1108,51 @@ dp_netdev_output_port(struct dp_netdev *dp, struct
ofpbuf *packet,
static int
dp_netdev_output_userspace(struct dp_netdev *dp, const struct ofpbuf *packet,
- int queue_no, const struct flow *flow, uint64_t arg)
+ int queue_no, const struct flow *flow,
+ const struct nlattr *userdata)
{
struct dp_netdev_queue *q = &dp->queues[queue_no];
- struct dp_netdev_upcall *u;
- struct dpif_upcall *upcall;
- struct ofpbuf *buf;
- size_t key_len;
-
- if (q->head - q->tail >= MAX_QUEUE_LEN) {
- dp->n_lost++;
- return ENOBUFS;
- }
+ if (q->head - q->tail < MAX_QUEUE_LEN) {
+ struct dp_netdev_upcall *u = &q->upcalls[q->head++ & QUEUE_MASK];
+ struct dpif_upcall *upcall = &u->upcall;
+ struct ofpbuf *buf = &u->buf;
+ size_t buf_size;
+
+ upcall->type = queue_no;
+
+ /* Allocate buffer big enough for everything. */
+ buf_size = ODPUTIL_FLOW_KEY_BYTES + 2 + packet->size;
+ if (userdata) {
+ buf_size += NLA_ALIGN(userdata->nla_len);
+ }
+ ofpbuf_init(buf, buf_size);
- u = &q->upcalls[q->head++ & QUEUE_MASK];
+ /* Put ODP flow. */
+ odp_flow_key_from_flow(buf, flow, flow->in_port);
+ upcall->key = buf->data;
+ upcall->key_len = buf->size;
- buf = &u->buf;
- ofpbuf_init(buf, ODPUTIL_FLOW_KEY_BYTES + 2 + packet->size);
- odp_flow_key_from_flow(buf, flow, flow->in_port);
- key_len = buf->size;
- ofpbuf_pull(buf, key_len);
- ofpbuf_reserve(buf, 2);
- ofpbuf_put(buf, packet->data, packet->size);
+ /* Put userdata. */
+ if (userdata) {
+ upcall->userdata = ofpbuf_put(buf, userdata,
+ NLA_ALIGN(userdata->nla_len));
+ }
- upcall = &u->upcall;
- upcall->type = queue_no;
- upcall->packet = buf;
- upcall->key = buf->base;
- upcall->key_len = key_len;
- upcall->userdata = arg;
+ /* Put packet.
+ *
+ * We adjust 'data' and 'size' in 'buf' so that only the packet itself
+ * is visible in 'upcall->packet'. The ODP flow and (if present)
+ * userdata become part of the headroom. */
+ ofpbuf_put_zeros(buf, 2);
+ buf->data = ofpbuf_put(buf, packet->data, packet->size);
+ buf->size = packet->size;
+ upcall->packet = buf;
- return 0;
+ return 0;
+ } else {
+ dp->n_lost++;
+ return ENOBUFS;
+ }
}
static void
@@ -1180,11 +1194,9 @@ dp_netdev_action_userspace(struct dp_netdev *dp,
struct ofpbuf *packet, struct flow *key,
const struct nlattr *a)
{
- const struct nlattr *userdata_attr;
- uint64_t userdata;
+ const struct nlattr *userdata;
- userdata_attr = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
- userdata = userdata_attr ? nl_attr_get_u64(userdata_attr) : 0;
+ userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
dp_netdev_output_userspace(dp, packet, DPIF_UC_ACTION, key, userdata);
}
diff --git a/lib/dpif.h b/lib/dpif.h
index c5e3fc8..fd05b2f 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -540,10 +540,10 @@ const char *dpif_upcall_type_to_string(enum
dpif_upcall_type);
/* A packet passed up from the datapath to userspace.
*
- * If 'key' or 'actions' is nonnull, then it points into data owned by
- * 'packet', so their memory cannot be freed separately. (This is hardly a
- * great way to do things but it works out OK for the dpif providers and
- * clients that exist so far.)
+ * If 'key', 'actions', or 'userdata' is nonnull, then it points into data
+ * owned by 'packet', so their memory cannot be freed separately. (This is
+ * hardly a great way to do things but it works out OK for the dpif providers
+ * and clients that exist so far.)
*/
struct dpif_upcall {
/* All types. */
@@ -553,7 +553,7 @@ struct dpif_upcall {
size_t key_len; /* Length of 'key' in bytes. */
/* DPIF_UC_ACTION only. */
- uint64_t userdata; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
+ struct nlattr *userdata; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
};
int dpif_recv_set(struct dpif *, bool enable);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 7e48981..cdbfe4e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -245,9 +245,11 @@ format_odp_userspace_action(struct ds *ds, const struct
nlattr *attr)
{
static const struct nl_policy ovs_userspace_policy[] = {
[OVS_USERSPACE_ATTR_PID] = { .type = NL_A_U32 },
- [OVS_USERSPACE_ATTR_USERDATA] = { .type = NL_A_U64, .optional = true },
+ [OVS_USERSPACE_ATTR_USERDATA] = { .type = NL_A_UNSPEC,
+ .optional = true },
};
struct nlattr *a[ARRAY_SIZE(ovs_userspace_policy)];
+ const struct nlattr *userdata_attr;
if (!nl_parse_nested(attr, ovs_userspace_policy, a, ARRAY_SIZE(a))) {
ds_put_cstr(ds, "userspace(error)");
@@ -257,7 +259,8 @@ format_odp_userspace_action(struct ds *ds, const struct
nlattr *attr)
ds_put_format(ds, "userspace(pid=%"PRIu32,
nl_attr_get_u32(a[OVS_USERSPACE_ATTR_PID]));
- if (a[OVS_USERSPACE_ATTR_USERDATA]) {
+ userdata_attr = a[OVS_USERSPACE_ATTR_USERDATA];
+ if (userdata_attr && nl_attr_get_size(userdata_attr) == sizeof(uint64_t)) {
uint64_t userdata = nl_attr_get_u64(a[OVS_USERSPACE_ATTR_USERDATA]);
union user_action_cookie cookie;
@@ -284,6 +287,16 @@ format_odp_userspace_action(struct ds *ds, const struct
nlattr *attr)
ds_put_format(ds, ",userdata=0x%"PRIx64, userdata);
break;
}
+ } else if (userdata_attr) {
+ const uint8_t *userdata = nl_attr_get(userdata_attr);
+ size_t len = nl_attr_get_size(userdata_attr);
+ size_t i;
+
+ ds_put_format(ds, ",userdata(");
+ for (i = 0; i < len; i++) {
+ ds_put_format(ds, "%02x", userdata[i]);
+ }
+ ds_put_char(ds, ')');
}
ds_put_char(ds, ')');
@@ -424,7 +437,7 @@ parse_odp_action(const char *s, const struct simap
*port_names,
int n = -1;
if (sscanf(s, "userspace(pid=%lli)%n", &pid, &n) > 0 && n > 0) {
- odp_put_userspace_action(pid, NULL, actions);
+ odp_put_userspace_action(pid, NULL, 0, actions);
return n;
} else if (sscanf(s, "userspace(pid=%lli,sFlow(vid=%i,"
"pcp=%i,output=%lli))%n",
@@ -440,7 +453,7 @@ parse_odp_action(const char *s, const struct simap
*port_names,
cookie.type = USER_ACTION_COOKIE_SFLOW;
cookie.sflow.vlan_tci = htons(tci);
cookie.sflow.output = output;
- odp_put_userspace_action(pid, &cookie, actions);
+ odp_put_userspace_action(pid, &cookie, sizeof cookie, actions);
return n;
} else if (sscanf(s, "userspace(pid=%lli,slow_path%n", &pid, &n) > 0
&& n > 0) {
@@ -462,18 +475,29 @@ parse_odp_action(const char *s, const struct simap
*port_names,
}
n++;
- odp_put_userspace_action(pid, &cookie, actions);
+ odp_put_userspace_action(pid, &cookie, sizeof cookie, actions);
return n;
} else if (sscanf(s, "userspace(pid=%lli,userdata="
"%31[x0123456789abcdefABCDEF])%n", &pid, userdata_s,
&n) > 0 && n > 0) {
- union user_action_cookie cookie;
uint64_t userdata;
userdata = strtoull(userdata_s, NULL, 0);
- memcpy(&cookie, &userdata, sizeof cookie);
- odp_put_userspace_action(pid, &cookie, actions);
+ odp_put_userspace_action(pid, &userdata, sizeof(userdata),
+ actions);
return n;
+ } else if (sscanf(s, "userspace(pid=%lli,userdata(%n", &pid, &n) > 0
+ && n > 0) {
+ struct ofpbuf buf;
+ char *end;
+
+ ofpbuf_init(&buf, 16);
+ end = ofpbuf_put_hex(&buf, &s[n], NULL);
+ if (end[0] == ')' && end[1] == ')') {
+ odp_put_userspace_action(pid, buf.data, buf.size, actions);
+ ofpbuf_uninit(&buf);
+ return (end + 2) - s;
+ }
}
}
@@ -2020,25 +2044,30 @@ odp_key_fitness_to_string(enum odp_key_fitness fitness)
}
/* Appends an OVS_ACTION_ATTR_USERSPACE action to 'odp_actions' that specifies
- * Netlink PID 'pid'. If 'cookie' is nonnull, adds a userdata attribute whose
- * contents contains 'cookie' and returns the offset within 'odp_actions' of
- * the start of the cookie. (If 'cookie' is null, then the return value is not
- * meaningful.) */
+ * Netlink PID 'pid'. If 'userdata' is nonnull, adds a userdata attribute
+ * whose contents are the 'userdata_size' bytes at 'userdata' and returns the
+ * offset within 'odp_actions' of the start of the cookie. (If 'userdata' is
+ * null, then the return value is not meaningful.) */
size_t
-odp_put_userspace_action(uint32_t pid, const union user_action_cookie *cookie,
+odp_put_userspace_action(uint32_t pid,
+ const void *userdata, size_t userdata_size,
struct ofpbuf *odp_actions)
{
+ size_t userdata_ofs;
size_t offset;
offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_USERSPACE);
nl_msg_put_u32(odp_actions, OVS_USERSPACE_ATTR_PID, pid);
- if (cookie) {
+ if (userdata) {
+ userdata_ofs = odp_actions->size + NLA_HDRLEN;
nl_msg_put_unspec(odp_actions, OVS_USERSPACE_ATTR_USERDATA,
- cookie, sizeof *cookie);
+ userdata, userdata_size);
+ } else {
+ userdata_ofs = 0;
}
nl_msg_end_nested(odp_actions, offset);
- return cookie ? odp_actions->size - NLA_ALIGN(sizeof *cookie) : 0;
+ return userdata_ofs;
}
void
diff --git a/lib/odp-util.h b/lib/odp-util.h
index ccf6c2a..62401fc 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -152,7 +152,7 @@ union user_action_cookie {
BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 8);
size_t odp_put_userspace_action(uint32_t pid,
- const union user_action_cookie *,
+ const void *userdata, size_t userdata_size,
struct ofpbuf *odp_actions);
void odp_put_tunnel_action(const struct flow_tnl *tunnel,
struct ofpbuf *odp_actions);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b1ec3fb..baca083 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3894,7 +3894,16 @@ classify_upcall(const struct dpif_upcall *upcall)
}
/* "action" upcalls need a closer look. */
- memcpy(&cookie, &upcall->userdata, sizeof(cookie));
+ if (!upcall->userdata) {
+ VLOG_WARN_RL(&rl, "action upcall missing cookie");
+ return BAD_UPCALL;
+ }
+ if (nl_attr_get_size(upcall->userdata) != sizeof(cookie)) {
+ VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %zu",
+ nl_attr_get_size(upcall->userdata));
+ return BAD_UPCALL;
+ }
+ memcpy(&cookie, nl_attr_get(upcall->userdata), sizeof(cookie));
switch (cookie.type) {
case USER_ACTION_COOKIE_SFLOW:
return SFLOW_UPCALL;
@@ -3904,7 +3913,8 @@ classify_upcall(const struct dpif_upcall *upcall)
case USER_ACTION_COOKIE_UNSPEC:
default:
- VLOG_WARN_RL(&rl, "invalid user cookie : 0x%"PRIx64, upcall->userdata);
+ VLOG_WARN_RL(&rl, "invalid user cookie : 0x%"PRIx64,
+ nl_attr_get_u64(upcall->userdata));
return BAD_UPCALL;
}
}
@@ -3924,7 +3934,7 @@ handle_sflow_upcall(struct dpif_backer *backer,
return;
}
- memcpy(&cookie, &upcall->userdata, sizeof(cookie));
+ memcpy(&cookie, nl_attr_get(upcall->userdata), sizeof(cookie));
dpif_sflow_received(ofproto->sflow, upcall->packet, &flow,
odp_in_port, &cookie);
}
@@ -5583,7 +5593,7 @@ compose_slow_path(const struct ofproto_dpif *ofproto,
const struct flow *flow,
ofpbuf_use_stack(&buf, stub, stub_size);
if (slow & (SLOW_CFM | SLOW_LACP | SLOW_STP)) {
uint32_t pid = dpif_port_get_pid(ofproto->backer->dpif, UINT32_MAX);
- odp_put_userspace_action(pid, &cookie, &buf);
+ odp_put_userspace_action(pid, &cookie, sizeof cookie, &buf);
} else {
put_userspace_action(ofproto, &buf, flow, &cookie);
}
@@ -5602,7 +5612,7 @@ put_userspace_action(const struct ofproto_dpif *ofproto,
pid = dpif_port_get_pid(ofproto->backer->dpif,
ofp_port_to_odp_port(ofproto, flow->in_port));
- return odp_put_userspace_action(pid, cookie, odp_actions);
+ return odp_put_userspace_action(pid, cookie, sizeof *cookie, odp_actions);
}
static void
diff --git a/tests/odp.at b/tests/odp.at
index 687f9c9..36bfd6c 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -76,6 +76,7 @@ userspace(pid=9765,slow_path())
userspace(pid=9765,slow_path(cfm))
userspace(pid=9765,slow_path(cfm,match))
userspace(pid=9123,userdata=0x815309)
+userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f))
set(tun_id(0x7f10354))
set(in_port(2))
set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))
--
1.7.2.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
