This patch introduces a common response header for extended commands.
This common header is designed to hold
- the returned comp_mask,
- the response sizes from core/ (eg. uverbs) and hw/ (eg. provider).
This header is part of the core response buffer, consuming 8 bytes,
so response buffer should be at least large enough to hold it.
To make the behavior consistent between command and response,
the command header is taken in account while checking command size,
just like it was for current commands.
In order to be able to report the size of the response from core/
and hw/, UDATA structures passed to handler functions are updated
each time space is used response buffer.
To report the comp_mask of the response, a pointer to comp_mask is
given as an input/output parameter of handler functions.
This provide a common framework to handle comp_mask for all extend
responses, additionally having the response buffer size would help
userspace to better handle extended response.
The new layout for command and response buffer is:
- command:
+----------------------------------------+
| flags | 00 00 | command |
| in_words | out_words |
+----------------------------------------+
| response |
| response |
| provider_in_words | provider_out_words |
| comp_mask |
+----------------------------------------+
| |
. <uverbs input> .
. (in_words * 8) .
. - sizeof(hdr) - sizeof(ex_hdr) .
| |
+----------------------------------------+
| |
. <provider input> .
. (provider_in_words * 8) .
| |
+----------------------------------------+
- response, if present:
+----------------------------------------+
| <extended |
| response header> |
+----------------------------------------+
| |
. <uverbs output space> .
. (out_words * 8) .
. - sizeof(ex_resp) .
| |
+----------------------------------------+
| |
. <provider output space> .
. (provider_out_words * 8) .
| |
+----------------------------------------+
For a successful command, the response buffer will be
presented with an updated header:
+----------------------------------------+
| comp_mask |
| out_words | provider_out_words |
+----------------------------------------+
| |
. <uverbs output> .
. (out_words * 8) .
. - sizeof(ex_resp) .
| |
+----------------------------------------+
| |
. <provider output> .
. (provider_out_words * 8) .
| |
+----------------------------------------+
The most notable drawbacks of this approach are:
- increase in complexity,
- inability to handle error gracefully when writing the response header.
Cc: Igor Ivanov <[email protected]>
Cc: Matan Barak <[email protected]>
Signed-off-by: Yann Droneaud <[email protected]>
Link: http://marc.info/[email protected]
Link: http://mid.gmane.org/[email protected]
---
drivers/infiniband/core/uverbs.h | 2 +-
drivers/infiniband/core/uverbs_cmd.c | 17 +++++++----
drivers/infiniband/core/uverbs_main.c | 54 +++++++++++++++++++++++++++++------
include/rdma/ib_verbs.h | 22 ++++++++++++++
include/uapi/rdma/ib_user_verbs.h | 8 +++++-
5 files changed, 87 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 03f6a0a..a1df33f 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -230,7 +230,7 @@ IB_UVERBS_DECLARE_CMD(close_xrcd);
int ib_uverbs_ex_##name(struct ib_uverbs_file *file, \
struct ib_udata *ucore, \
struct ib_udata *uhw, \
- __u32 comp_mask)
+ __u32 *comp_mask)
IB_UVERBS_DECLARE_EX_CMD(create_flow);
IB_UVERBS_DECLARE_EX_CMD(destroy_flow);
diff --git a/drivers/infiniband/core/uverbs_cmd.c
b/drivers/infiniband/core/uverbs_cmd.c
index 3b09f1d..3d2f0ad 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2634,7 +2634,7 @@ static int kern_spec_to_ib_spec(struct ib_kern_spec
*kern_spec,
int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
struct ib_udata *ucore,
struct ib_udata *uhw,
- __u32 comp_mask)
+ __u32 *comp_mask)
{
struct ib_uverbs_create_flow cmd;
struct ib_uverbs_create_flow_resp resp;
@@ -2649,7 +2649,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
int i;
int kern_attr_size;
- if (comp_mask)
+ if (*comp_mask)
return -EINVAL;
if (ucore->inlen != sizeof(cmd))
@@ -2662,8 +2662,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
if (err)
return err;
- ucore->inbuf += sizeof(cmd);
- ucore->inlen -= sizeof(cmd);
+ ib_udata_consume_in(ucore, sizeof(cmd));
if ((cmd.flow_attr.type == IB_FLOW_ATTR_SNIFFER &&
!capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
@@ -2693,6 +2692,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
err = -EFAULT;
goto err_free_attr;
}
+ ib_udata_consume_in(ucore, kern_attr_size);
} else {
kern_flow_attr = &cmd.flow_attr;
kern_attr_size = sizeof(cmd.flow_attr);
@@ -2763,6 +2763,8 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
if (err)
goto err_copy;
+ ib_udata_consume_out(ucore, sizeof(resp));
+
put_qp_read(qp);
mutex_lock(&file->mutex);
list_add_tail(&uobj->list, &file->ucontext->rule_list);
@@ -2774,6 +2776,9 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
kfree(flow_attr);
if (cmd.flow_attr.num_of_specs)
kfree(kern_flow_attr);
+
+ *comp_mask = 0;
+
return 0;
err_copy:
idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
@@ -2794,14 +2799,14 @@ err_free_attr:
int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
struct ib_udata *ucore,
struct ib_udata *uhw,
- __u32 comp_mask)
+ __u32 *comp_mask)
{
struct ib_uverbs_destroy_flow cmd;
struct ib_flow *flow_id;
struct ib_uobject *uobj;
int ret;
- if (comp_mask)
+ if (*comp_mask)
return -EINVAL;
if (ucore->inlen != sizeof(cmd))
diff --git a/drivers/infiniband/core/uverbs_main.c
b/drivers/infiniband/core/uverbs_main.c
index a9687dd..12b44a2 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -120,7 +120,7 @@ static ssize_t (*uverbs_cmd_table[])(struct ib_uverbs_file
*file,
static int (*uverbs_ex_cmd_table[])(struct ib_uverbs_file *file,
struct ib_udata *ucore,
struct ib_udata *uhw,
- __u32 comp_mask) = {
+ __u32 *comp_mask) = {
[IB_USER_VERBS_EX_CMD_CREATE_FLOW] = ib_uverbs_ex_create_flow,
[IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow
};
@@ -636,8 +636,13 @@ static ssize_t ib_uverbs_write(struct file *filp, const
char __user *buf,
__u32 command;
struct ib_uverbs_ex_cmd_hdr ex_hdr;
+ struct ib_uverbs_ex_resp_hdr ex_resp;
struct ib_udata ucore;
struct ib_udata uhw;
+ unsigned long response;
+ size_t ucore_outlen = 0;
+ size_t uhw_outlen = 0;
+ __u32 comp_mask;
int err;
if (hdr.command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
@@ -662,13 +667,12 @@ static ssize_t ib_uverbs_write(struct file *filp, const
char __user *buf,
if (copy_from_user(&ex_hdr, buf + sizeof(hdr), sizeof(ex_hdr)))
return -EFAULT;
- count -= sizeof(hdr) + sizeof(ex_hdr);
- buf += sizeof(hdr) + sizeof(ex_hdr);
-
if ((hdr.in_words + ex_hdr.provider_in_words) * 8 != count)
return -EINVAL;
- if (ex_hdr.response) {
+ response = (unsigned long)ex_hdr.response;
+
+ if (response) {
if (!hdr.out_words && !ex_hdr.provider_out_words)
return -EINVAL;
} else {
@@ -678,24 +682,58 @@ static ssize_t ib_uverbs_write(struct file *filp, const
char __user *buf,
INIT_UDATA(&ucore,
(hdr.in_words) ? buf : 0,
- (unsigned long)ex_hdr.response,
+ response,
hdr.in_words * 8,
hdr.out_words * 8);
+ ib_udata_consume_in(&ucore, sizeof(hdr) + sizeof(ex_hdr));
+
+ if (response) {
+
+ ucore_outlen = ucore.outlen; /* keep in mind total
available len
+ (including response
header size) */
+
+ err = ib_udata_consume_out(&ucore, sizeof(ex_resp));
+ if (err)
+ return err;
+
+ /* fail early to not have to recover from invalid
response buffer */
+ memset(&ex_resp, 0, sizeof(ex_resp));
+ if (copy_to_user((void __user *)response,
+ &ex_resp, sizeof(ex_resp)))
+ return -EFAULT;
+ }
+
INIT_UDATA(&uhw,
(ex_hdr.provider_in_words) ? buf + ucore.inlen : 0,
- (ex_hdr.provider_out_words) ? (unsigned
long)ex_hdr.response + ucore.outlen : 0,
+ (ex_hdr.provider_out_words) ? response +
ucore.outlen : 0,
ex_hdr.provider_in_words * 8,
ex_hdr.provider_out_words * 8);
+ if (response)
+ uhw_outlen = uhw.outlen;
+
+ comp_mask = ex_hdr.comp_mask;
+
err = uverbs_ex_cmd_table[command](file,
&ucore,
&uhw,
- ex_hdr.comp_mask);
+ &comp_mask);
if (err)
return err;
+ if (response) {
+
+ ex_resp.comp_mask = comp_mask;
+ ex_resp.out_words = (ucore_outlen - ucore.outlen) / 8;
+ ex_resp.provider_out_words = (uhw_outlen - uhw.outlen)
/ 8;
+
+ if (copy_to_user((void __user *)response,
+ &ex_resp, sizeof(ex_resp)))
+ return -EFAULT;
+ }
+
return count;
}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index a06fc12..9dd7dad 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1478,6 +1478,28 @@ static inline int ib_copy_to_udata(struct ib_udata
*udata, void *src, size_t len
return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
}
+static inline int ib_udata_consume_in(struct ib_udata *udata, size_t len)
+{
+ if (udata->inlen < len)
+ return -EINVAL;
+
+ udata->inlen -= len;
+ udata->inbuf += len;
+
+ return 0;
+}
+
+static inline int ib_udata_consume_out(struct ib_udata *udata, size_t len)
+{
+ if (udata->outlen < len)
+ return -ENOSPC;
+
+ udata->outlen -= len;
+ udata->outbuf += len;
+
+ return 0;
+}
+
/**
* ib_modify_qp_is_ok - Check that the supplied attribute mask
* contains all required attributes and no attributes not allowed for
diff --git a/include/uapi/rdma/ib_user_verbs.h
b/include/uapi/rdma/ib_user_verbs.h
index 13b3008..0da06d0 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -142,6 +142,12 @@ struct ib_uverbs_ex_cmd_hdr {
__u32 comp_mask;
};
+struct ib_uverbs_ex_resp_hdr {
+ __u32 comp_mask;
+ __u16 out_words;
+ __u16 provider_out_words;
+};
+
struct ib_uverbs_get_context {
__u64 response;
__u64 driver_data[0];
@@ -778,8 +784,8 @@ struct ib_uverbs_create_flow {
};
struct ib_uverbs_create_flow_resp {
- __u32 comp_mask;
__u32 flow_handle;
+ __u32 reserved;
};
struct ib_uverbs_destroy_flow {
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
