On 11/10/2013 8:57 PM, Yann Droneaud wrote:
The unused field in the extended header is a perfect candidate
to hold the command "comp_mask" (eg. bit field used to handle
compatibility). This was suggested by Roland Dreier in a previous
review[1].
So this patch move comp_mask from create_flow/destroy_flow commands
to the extended command header. Then comp_mask is passed as part
of function parameters.
[1]:
http://marc.info/?iÊ[email protected]
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 | 3 ++-
drivers/infiniband/core/uverbs_cmd.c | 15 ++++++++++-----
drivers/infiniband/core/uverbs_main.c | 6 ++++--
include/uapi/rdma/ib_user_verbs.h | 6 +++---
4 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index bdc842e..a12b516 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -245,7 +245,8 @@ IB_UVERBS_DECLARE_CMD(close_xrcd);
#define IB_UVERBS_DECLARE_EX_CMD(name) \
int ib_uverbs_ex_##name(struct ib_uverbs_file *file, \
struct ib_udata *ucore, \
- struct ib_udata *uhw)
+ struct ib_udata *uhw, \
+ __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 e35877d..a8ecb3a 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2633,7 +2633,8 @@ static int kern_spec_to_ib_spec(struct
ib_uverbs_flow_spec *kern_spec,
int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
struct ib_udata *ucore,
- struct ib_udata *uhw)
+ struct ib_udata *uhw,
+ __u32 comp_mask)
{
struct ib_uverbs_create_flow cmd;
struct ib_uverbs_create_flow_resp resp;
@@ -2647,6 +2648,9 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
void *ib_spec;
int i;
+ if (comp_mask)
+ return -EINVAL;
+
if (ucore->outlen < sizeof(resp))
return -ENOSPC;
@@ -2657,9 +2661,6 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
ucore->inbuf +=izeof(cmd);
ucore->inlen -=izeof(cmd);
- if (cmd.comp_mask)
- return -EINVAL;
-
if ((cmd.flow_attr.type =IB_FLOW_ATTR_SNIFFER &&
!capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
return -EPERM;
@@ -2785,13 +2786,17 @@ err_free_attr:
int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
struct ib_udata *ucore,
- struct ib_udata *uhw)
+ struct ib_udata *uhw,
+ __u32 comp_mask)
{
struct ib_uverbs_destroy_flow cmd;
struct ib_flow *flow_id;
struct ib_uobject *uobj;
int ret;
+ if (comp_mask)
+ return -EINVAL;
+
ret =b_copy_from_udata(&cmd, ucore, sizeof(cmd));
if (ret)
return ret;
diff --git a/drivers/infiniband/core/uverbs_main.c
b/drivers/infiniband/core/uverbs_main.c
index 4f00654..a9687dd 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -119,7 +119,8 @@ 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) =
+ struct ib_udata *uhw,
+ __u32 comp_mask) =
[IB_USER_VERBS_EX_CMD_CREATE_FLOW] =b_uverbs_ex_create_flow,
[IB_USER_VERBS_EX_CMD_DESTROY_FLOW] =b_uverbs_ex_destroy_flow
};
@@ -689,7 +690,8 @@ static ssize_t ib_uverbs_write(struct file *filp, const
char __user *buf,
err =verbs_ex_cmd_table[command](file,
&ucore,
- &uhw);
+ &uhw,
+ ex_hdr.comp_mask);
if (err)
return err;
diff --git a/include/uapi/rdma/ib_user_verbs.h
b/include/uapi/rdma/ib_user_verbs.h
index cbfdd4c..095a2bd 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -139,7 +139,7 @@ struct ib_uverbs_ex_cmd_hdr {
__u64 response;
__u16 provider_in_words;
__u16 provider_out_words;
- __u32 cmd_hdr_reserved;
+ __u32 comp_mask;
};
struct ib_uverbs_get_context {
@@ -783,8 +783,8 @@ struct ib_uverbs_flow_attr {
};
struct ib_uverbs_create_flow {
- __u32 comp_mask;
__u32 qp_handle;
+ __u32 reserved;
struct ib_uverbs_flow_attr flow_attr;
};
@@ -794,8 +794,8 @@ struct ib_uverbs_create_flow_resp {
};
struct ib_uverbs_destroy_flow {
- __u32 comp_mask;
__u32 flow_handle;
+ __u32 reserved;
};
struct ib_uverbs_create_srq {
--
1.8.3.1
I think that's a good idea to make the comp_mask field a part of our
infrastructure. But, I think we should consider making this symmetrical.
If we use the command's comp_mask as an input parameter, shouldn't we
use the response's comp_mask as an output parameter ?
What's about the provider's comp_mask ?
Matan
--
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
