* [PATCH for -next v2 1/7] IB/core: clarify overflow/underflow checks on ib_create/destroy_flow
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 2/7] IB/core: Rename 'flow' structs to match other uverbs structs Yann Droneaud
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
This patch fixes the following issues:
1. Unneeded checks were removed
2. Removed the fixed size out of flow_attr.size and by thus
simplifying the checks.
3. Remove a 32bit hole on 64bit systems with strict alignment
in struct ib_kern_flow_att by adding a reserved field.
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
drivers/infiniband/core/uverbs_cmd.c | 32 +++++++++++++++-----------------
include/uapi/rdma/ib_user_verbs.h | 1 +
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 2f0f01b..26ee2d2 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2657,7 +2657,6 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
void *kern_spec;
void *ib_spec;
int i;
- int kern_attr_size;
if (out_len < sizeof(resp))
return -ENOSPC;
@@ -2672,32 +2671,28 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
!capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
return -EPERM;
- if (cmd.flow_attr.num_of_specs < 0 ||
- cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
+ if (cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
return -EINVAL;
- kern_attr_size = cmd.flow_attr.size - sizeof(cmd) -
- sizeof(struct ib_uverbs_cmd_hdr_ex);
-
- if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > in_len ||
- kern_attr_size < 0 || kern_attr_size >
+ if (cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
+ cmd.flow_attr.size >
(cmd.flow_attr.num_of_specs * sizeof(struct ib_kern_spec)))
return -EINVAL;
if (cmd.flow_attr.num_of_specs) {
- kern_flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
+ kern_flow_attr = kmalloc(sizeof(*kern_flow_attr) + cmd.flow_attr.size,
+ GFP_KERNEL);
if (!kern_flow_attr)
return -ENOMEM;
memcpy(kern_flow_attr, &cmd.flow_attr, sizeof(*kern_flow_attr));
if (copy_from_user(kern_flow_attr + 1, buf + sizeof(cmd),
- kern_attr_size)) {
+ cmd.flow_attr.size)) {
err = -EFAULT;
goto err_free_attr;
}
} else {
kern_flow_attr = &cmd.flow_attr;
- kern_attr_size = sizeof(cmd.flow_attr);
}
uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
@@ -2714,7 +2709,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
goto err_uobj;
}
- flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
+ flow_attr = kmalloc(sizeof(*flow_attr) + cmd.flow_attr.size, GFP_KERNEL);
if (!flow_attr) {
err = -ENOMEM;
goto err_put;
@@ -2729,19 +2724,22 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
kern_spec = kern_flow_attr + 1;
ib_spec = flow_attr + 1;
- for (i = 0; i < flow_attr->num_of_specs && kern_attr_size > 0; i++) {
+ for (i = 0; i < flow_attr->num_of_specs &&
+ cmd.flow_attr.size > offsetof(struct ib_kern_spec, reserved) &&
+ cmd.flow_attr.size >=
+ ((struct ib_kern_spec *)kern_spec)->size; i++) {
err = kern_spec_to_ib_spec(kern_spec, ib_spec);
if (err)
goto err_free;
flow_attr->size +=
((union ib_flow_spec *) ib_spec)->size;
- kern_attr_size -= ((struct ib_kern_spec *) kern_spec)->size;
+ cmd.flow_attr.size -= ((struct ib_kern_spec *)kern_spec)->size;
kern_spec += ((struct ib_kern_spec *) kern_spec)->size;
ib_spec += ((union ib_flow_spec *) ib_spec)->size;
}
- if (kern_attr_size) {
- pr_warn("create flow failed, %d bytes left from uverb cmd\n",
- kern_attr_size);
+ if (cmd.flow_attr.size || (i != flow_attr->num_of_specs)) {
+ pr_warn("create flow failed, flow %d: %d bytes left from uverb cmd\n",
+ i, cmd.flow_attr.size);
goto err_free;
}
flow_id = ib_create_flow(qp, flow_attr, IB_FLOW_DOMAIN_USER);
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index e3ddd86..99a58bd 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -771,6 +771,7 @@ struct ib_kern_flow_attr {
struct ib_uverbs_create_flow {
__u32 comp_mask;
+ __u32 reserved;
__u64 response;
__u32 qp_handle;
struct ib_kern_flow_attr flow_attr;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 2/7] IB/core: Rename 'flow' structs to match other uverbs structs
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2013-11-06 22:21 ` [PATCH for -next v2 1/7] IB/core: clarify overflow/underflow checks on ib_create/destroy_flow Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 3/7] IB/core: Makes uverbs flow structure using names more similar to verbs ones Yann Droneaud
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yann Droneaud
Commit 436f2ad added public data structures to support
receive flow steering. The new structs are not following
the 'uverbs' pattern: they're lacking the common prefix
'ib_uverbs'.
This patch replaces ib_kern prefix by ib_uverbs.
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
drivers/infiniband/core/uverbs_cmd.c | 14 +++++++-------
include/uapi/rdma/ib_user_verbs.h | 36 ++++++++++++++++++------------------
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 26ee2d2..f83c1dc 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2602,7 +2602,7 @@ out_put:
}
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-static int kern_spec_to_ib_spec(struct ib_kern_spec *kern_spec,
+static int kern_spec_to_ib_spec(struct ib_uverbs_spec *kern_spec,
union ib_flow_spec *ib_spec)
{
ib_spec->type = kern_spec->type;
@@ -2650,7 +2650,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
struct ib_uverbs_create_flow_resp resp;
struct ib_uobject *uobj;
struct ib_flow *flow_id;
- struct ib_kern_flow_attr *kern_flow_attr;
+ struct ib_uverbs_flow_attr *kern_flow_attr;
struct ib_flow_attr *flow_attr;
struct ib_qp *qp;
int err = 0;
@@ -2676,7 +2676,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
if (cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
cmd.flow_attr.size >
- (cmd.flow_attr.num_of_specs * sizeof(struct ib_kern_spec)))
+ (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_spec)))
return -EINVAL;
if (cmd.flow_attr.num_of_specs) {
@@ -2725,16 +2725,16 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
kern_spec = kern_flow_attr + 1;
ib_spec = flow_attr + 1;
for (i = 0; i < flow_attr->num_of_specs &&
- cmd.flow_attr.size > offsetof(struct ib_kern_spec, reserved) &&
+ cmd.flow_attr.size > offsetof(struct ib_uverbs_spec, reserved) &&
cmd.flow_attr.size >=
- ((struct ib_kern_spec *)kern_spec)->size; i++) {
+ ((struct ib_uverbs_spec *)kern_spec)->size; i++) {
err = kern_spec_to_ib_spec(kern_spec, ib_spec);
if (err)
goto err_free;
flow_attr->size +=
((union ib_flow_spec *) ib_spec)->size;
- cmd.flow_attr.size -= ((struct ib_kern_spec *)kern_spec)->size;
- kern_spec += ((struct ib_kern_spec *) kern_spec)->size;
+ cmd.flow_attr.size -= ((struct ib_uverbs_spec *)kern_spec)->size;
+ kern_spec += ((struct ib_uverbs_spec *) kern_spec)->size;
ib_spec += ((union ib_flow_spec *) ib_spec)->size;
}
if (cmd.flow_attr.size || (i != flow_attr->num_of_specs)) {
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 99a58bd..96a6605 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -701,61 +701,61 @@ struct ib_uverbs_detach_mcast {
};
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-struct ib_kern_eth_filter {
+struct ib_uverbs_eth_filter {
__u8 dst_mac[6];
__u8 src_mac[6];
__be16 ether_type;
__be16 vlan_tag;
};
-struct ib_kern_spec_eth {
+struct ib_uverbs_spec_eth {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_kern_eth_filter val;
- struct ib_kern_eth_filter mask;
+ struct ib_uverbs_eth_filter val;
+ struct ib_uverbs_eth_filter mask;
};
-struct ib_kern_ipv4_filter {
+struct ib_uverbs_ipv4_filter {
__be32 src_ip;
__be32 dst_ip;
};
-struct ib_kern_spec_ipv4 {
+struct ib_uverbs_spec_ipv4 {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_kern_ipv4_filter val;
- struct ib_kern_ipv4_filter mask;
+ struct ib_uverbs_ipv4_filter val;
+ struct ib_uverbs_ipv4_filter mask;
};
-struct ib_kern_tcp_udp_filter {
+struct ib_uverbs_tcp_udp_filter {
__be16 dst_port;
__be16 src_port;
};
-struct ib_kern_spec_tcp_udp {
+struct ib_uverbs_spec_tcp_udp {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_kern_tcp_udp_filter val;
- struct ib_kern_tcp_udp_filter mask;
+ struct ib_uverbs_tcp_udp_filter val;
+ struct ib_uverbs_tcp_udp_filter mask;
};
-struct ib_kern_spec {
+struct ib_uverbs_spec {
union {
struct {
__u32 type;
__u16 size;
__u16 reserved;
};
- struct ib_kern_spec_eth eth;
- struct ib_kern_spec_ipv4 ipv4;
- struct ib_kern_spec_tcp_udp tcp_udp;
+ struct ib_uverbs_spec_eth eth;
+ struct ib_uverbs_spec_ipv4 ipv4;
+ struct ib_uverbs_spec_tcp_udp tcp_udp;
};
};
-struct ib_kern_flow_attr {
+struct ib_uverbs_flow_attr {
__u32 type;
__u16 size;
__u16 priority;
@@ -774,7 +774,7 @@ struct ib_uverbs_create_flow {
__u32 reserved;
__u64 response;
__u32 qp_handle;
- struct ib_kern_flow_attr flow_attr;
+ struct ib_uverbs_flow_attr flow_attr;
};
struct ib_uverbs_create_flow_resp {
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 3/7] IB/core: Makes uverbs flow structure using names more similar to verbs ones
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2013-11-06 22:21 ` [PATCH for -next v2 1/7] IB/core: clarify overflow/underflow checks on ib_create/destroy_flow Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 2/7] IB/core: Rename 'flow' structs to match other uverbs structs Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 4/7] IB/core: Uses a common header for uverbs flow_specs Yann Droneaud
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yann Droneaud
This patch add "flow" prefix to most of data structure added as part
of commit 436f2ad to keep those names in sync with the data structures
added in commit 319a441.
It's just a matter of translating 'ib_flow' to 'ib_uverbs_flow'.
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
drivers/infiniband/core/uverbs_cmd.c | 12 ++++++------
include/uapi/rdma/ib_user_verbs.h | 32 ++++++++++++++++----------------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index f83c1dc..19f14d8 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2602,7 +2602,7 @@ out_put:
}
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-static int kern_spec_to_ib_spec(struct ib_uverbs_spec *kern_spec,
+static int kern_spec_to_ib_spec(struct ib_uverbs_flow_spec *kern_spec,
union ib_flow_spec *ib_spec)
{
ib_spec->type = kern_spec->type;
@@ -2676,7 +2676,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
if (cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
cmd.flow_attr.size >
- (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_spec)))
+ (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
return -EINVAL;
if (cmd.flow_attr.num_of_specs) {
@@ -2725,16 +2725,16 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
kern_spec = kern_flow_attr + 1;
ib_spec = flow_attr + 1;
for (i = 0; i < flow_attr->num_of_specs &&
- cmd.flow_attr.size > offsetof(struct ib_uverbs_spec, reserved) &&
+ cmd.flow_attr.size > offsetof(struct ib_uverbs_flow_spec, reserved) &&
cmd.flow_attr.size >=
- ((struct ib_uverbs_spec *)kern_spec)->size; i++) {
+ ((struct ib_uverbs_flow_spec *)kern_spec)->size; i++) {
err = kern_spec_to_ib_spec(kern_spec, ib_spec);
if (err)
goto err_free;
flow_attr->size +=
((union ib_flow_spec *) ib_spec)->size;
- cmd.flow_attr.size -= ((struct ib_uverbs_spec *)kern_spec)->size;
- kern_spec += ((struct ib_uverbs_spec *) kern_spec)->size;
+ cmd.flow_attr.size -= ((struct ib_uverbs_flow_spec *)kern_spec)->size;
+ kern_spec += ((struct ib_uverbs_flow_spec *) kern_spec)->size;
ib_spec += ((union ib_flow_spec *) ib_spec)->size;
}
if (cmd.flow_attr.size || (i != flow_attr->num_of_specs)) {
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 96a6605..ef2be64 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -701,57 +701,57 @@ struct ib_uverbs_detach_mcast {
};
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-struct ib_uverbs_eth_filter {
+struct ib_uverbs_flow_eth_filter {
__u8 dst_mac[6];
__u8 src_mac[6];
__be16 ether_type;
__be16 vlan_tag;
};
-struct ib_uverbs_spec_eth {
+struct ib_uverbs_flow_spec_eth {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_uverbs_eth_filter val;
- struct ib_uverbs_eth_filter mask;
+ struct ib_uverbs_flow_eth_filter val;
+ struct ib_uverbs_flow_eth_filter mask;
};
-struct ib_uverbs_ipv4_filter {
+struct ib_uverbs_flow_ipv4_filter {
__be32 src_ip;
__be32 dst_ip;
};
-struct ib_uverbs_spec_ipv4 {
+struct ib_uverbs_flow_spec_ipv4 {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_uverbs_ipv4_filter val;
- struct ib_uverbs_ipv4_filter mask;
+ struct ib_uverbs_flow_ipv4_filter val;
+ struct ib_uverbs_flow_ipv4_filter mask;
};
-struct ib_uverbs_tcp_udp_filter {
+struct ib_uverbs_flow_tcp_udp_filter {
__be16 dst_port;
__be16 src_port;
};
-struct ib_uverbs_spec_tcp_udp {
+struct ib_uverbs_flow_spec_tcp_udp {
__u32 type;
__u16 size;
__u16 reserved;
- struct ib_uverbs_tcp_udp_filter val;
- struct ib_uverbs_tcp_udp_filter mask;
+ struct ib_uverbs_flow_tcp_udp_filter val;
+ struct ib_uverbs_flow_tcp_udp_filter mask;
};
-struct ib_uverbs_spec {
+struct ib_uverbs_flow_spec {
union {
struct {
__u32 type;
__u16 size;
__u16 reserved;
};
- struct ib_uverbs_spec_eth eth;
- struct ib_uverbs_spec_ipv4 ipv4;
- struct ib_uverbs_spec_tcp_udp tcp_udp;
+ struct ib_uverbs_flow_spec_eth eth;
+ struct ib_uverbs_flow_spec_ipv4 ipv4;
+ struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
};
};
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 4/7] IB/core: Uses a common header for uverbs flow_specs
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2013-11-06 22:21 ` [PATCH for -next v2 3/7] IB/core: Makes uverbs flow structure using names more similar to verbs ones Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 5/7] IB/core: Remove ib_uverbs_flow_spec structure from userspace Yann Droneaud
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yann Droneaud
A common header will allows better checking of
flow specs size, while ensuring strict alignment
to 64bits.
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
include/uapi/rdma/ib_user_verbs.h | 53 +++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index ef2be64..4301498 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -701,6 +701,14 @@ struct ib_uverbs_detach_mcast {
};
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
+struct ib_uverbs_flow_spec_hdr {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ /* followed by flow_spec */
+ __u64 flow_spec_data[0];
+};
+
struct ib_uverbs_flow_eth_filter {
__u8 dst_mac[6];
__u8 src_mac[6];
@@ -709,9 +717,14 @@ struct ib_uverbs_flow_eth_filter {
};
struct ib_uverbs_flow_spec_eth {
- __u32 type;
- __u16 size;
- __u16 reserved;
+ union {
+ struct ib_uverbs_flow_spec_hdr hdr;
+ struct {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ };
+ };
struct ib_uverbs_flow_eth_filter val;
struct ib_uverbs_flow_eth_filter mask;
};
@@ -722,9 +735,14 @@ struct ib_uverbs_flow_ipv4_filter {
};
struct ib_uverbs_flow_spec_ipv4 {
- __u32 type;
- __u16 size;
- __u16 reserved;
+ union {
+ struct ib_uverbs_flow_spec_hdr hdr;
+ struct {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ };
+ };
struct ib_uverbs_flow_ipv4_filter val;
struct ib_uverbs_flow_ipv4_filter mask;
};
@@ -735,19 +753,27 @@ struct ib_uverbs_flow_tcp_udp_filter {
};
struct ib_uverbs_flow_spec_tcp_udp {
- __u32 type;
- __u16 size;
- __u16 reserved;
+ union {
+ struct ib_uverbs_flow_spec_hdr hdr;
+ struct {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ };
+ };
struct ib_uverbs_flow_tcp_udp_filter val;
struct ib_uverbs_flow_tcp_udp_filter mask;
};
struct ib_uverbs_flow_spec {
union {
- struct {
- __u32 type;
- __u16 size;
- __u16 reserved;
+ union {
+ struct ib_uverbs_flow_spec_hdr hdr;
+ struct {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ };
};
struct ib_uverbs_flow_spec_eth eth;
struct ib_uverbs_flow_spec_ipv4 ipv4;
@@ -767,6 +793,7 @@ struct ib_uverbs_flow_attr {
* struct ib_flow_spec_xxx
* struct ib_flow_spec_yyy
*/
+ struct ib_uverbs_flow_spec_hdr flow_specs[0];
};
struct ib_uverbs_create_flow {
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 5/7] IB/core: Remove ib_uverbs_flow_spec structure from userspace
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2013-11-06 22:21 ` [PATCH for -next v2 4/7] IB/core: Uses a common header for uverbs flow_specs Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-06 22:21 ` [PATCH for -next v2 6/7] IB/core: extended command: an improved infrastructure for uverbs commands Yann Droneaud
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yann Droneaud
The structure holding any types of flow_spec is of no use to userspace.
It would be wrong for userspace to do:
struct ib_uverbs_flow_spec flow_spec;
flow_spec.type = IB_FLOW_SPEC_TCP;
flow_spec.size = sizeof(flow_spec);
Instead, userspace should use the dedicated flow_spec structure for
- Ethernet : struct ib_uverbs_flow_spec_eth,
- IPv4 : struct ib_uverbs_flow_spec_ipv4,
- TCP/UDP : struct ib_uverbs_flow_spec_tcp_udp.
In other words, struct ib_uverbs_flow_spec is a "virtual"
data structure that can only be use by the kernel as an alias
to the other.
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
drivers/infiniband/core/uverbs.h | 16 ++++++++++++++++
include/uapi/rdma/ib_user_verbs.h | 16 ----------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index d8f9c6c..777954f 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -178,6 +178,22 @@ void ib_uverbs_event_handler(struct ib_event_handler *handler,
struct ib_event *event);
void ib_uverbs_dealloc_xrcd(struct ib_uverbs_device *dev, struct ib_xrcd *xrcd);
+struct ib_uverbs_flow_spec {
+ union {
+ union {
+ struct ib_uverbs_flow_spec_hdr hdr;
+ struct {
+ __u32 type;
+ __u16 size;
+ __u16 reserved;
+ };
+ };
+ struct ib_uverbs_flow_spec_eth eth;
+ struct ib_uverbs_flow_spec_ipv4 ipv4;
+ struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
+ };
+};
+
#define IB_UVERBS_DECLARE_CMD(name) \
ssize_t ib_uverbs_##name(struct ib_uverbs_file *file, \
const char __user *buf, int in_len, \
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 4301498..fc9bbe3 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -765,22 +765,6 @@ struct ib_uverbs_flow_spec_tcp_udp {
struct ib_uverbs_flow_tcp_udp_filter mask;
};
-struct ib_uverbs_flow_spec {
- union {
- union {
- struct ib_uverbs_flow_spec_hdr hdr;
- struct {
- __u32 type;
- __u16 size;
- __u16 reserved;
- };
- };
- struct ib_uverbs_flow_spec_eth eth;
- struct ib_uverbs_flow_spec_ipv4 ipv4;
- struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
- };
-};
-
struct ib_uverbs_flow_attr {
__u32 type;
__u16 size;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 6/7] IB/core: extended command: an improved infrastructure for uverbs commands
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2013-11-06 22:21 ` [PATCH for -next v2 5/7] IB/core: Remove ib_uverbs_flow_spec structure from userspace Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
[not found] ` <2c3b64e8b42c3bd7434b03e3439c58df874992f2.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2013-11-06 22:21 ` [PATCH for -next v2 7/7] IB/core: re-enable create_flow/destroy_flow uverbs Yann Droneaud
2013-11-07 9:33 ` [PATCH for-next v2 0/7] uverbs extensions fixes Matan Barak
7 siblings, 1 reply; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yann Droneaud, Igor Ivanov
Commit 400dbc9 added an infrastructure for extensible uverbs
commands while later commit 436f2ad exported ib_create_flow()/ib_destroy_flow()
functions using this new infrastructure.
According to the commit 400dbc9, the purpose of this infrastructure is
to support passing around provider (eg. hardware) specific buffers
when userspace issue commands to the kernel, so that it would be possible
to extend uverbs (eg. core) buffers independently from the provider buffers.
But the new kernel command function prototypes were not modified
to take advantage of this extension. This issue was exposed by
Roland Dreier in a previous review[1].
So the following patch is an attempt to a revised extensible command
infrastructure.
This improved extensible command infrastructure distinguish between
core (eg. legacy)'s command/response buffers from provider (eg. hardware)'s
command/response buffers: each extended command implementing function
is given a struct ib_udata to hold core (eg. uverbs) input and output buffers,
and another struct ib_udata to hold the hw (eg. provider) input and output
buffers.
Having those buffers identified separately make it easier to increase one
buffer to support extension without having to add some code to guess
the exact size of each command/response parts: This should make the extended
functions more reliable.
Additionally, instead of relying on command identifier being greater
than IB_USER_VERBS_CMD_THRESHOLD, the proposed infrastructure
rely on unused bits in command field: on the 32 bits provided by
command field, only 6 bits are really needed to encode the identifier
of commands currently supported by the kernel. (Even using only 6 bits
leaves room for about 23 new commands).
So this patch makes use of some high order bits in command field
to store flags, leaving enough room for more command identifiers
than one will ever need (eg. 256).
The new flags are used to specify if the command should be processed
as an extended one or a legacy one. While designing the new command format,
care was taken to make usage of flags itself extensible.
Using high order bits of the commands field ensure
that newer libibverbs on older kernel will properly fail
when trying to call extended commands. On the other hand,
older libibverbs on newer kernel will never be able to issue
calls to extended commands.
The extended command header includes the optional response
pointer so that output buffer length and output buffer pointer
are located together in the command, allowing proper parameters
checking. This should make implementing functions easier and safer.
Additionally the extended header ensure 64bits alignment,
while making all sizes multiple of 8 bytes, extending
the maximum buffer size:
legacy extended
Maximum command buffer: 256KBytes 1024KBytes (512KBytes + 512KBytes)
Maximum response buffer: 256KBytes 1024KBytes (512KBytes + 512KBytes)
For the purpose of doing proper buffer size accounting, the headers size
are no more taken in account in "in_words".
One of the odds of the current extensible infrastructure, reading twice
the "legacy" command header, is fixed by removing the "legacy" command header
from the extended command header: they are processed as two different parts
of the command: memory is read once and information are not duplicated:
it's making clear that's an extended command scheme and not a different
command scheme.
The proposed scheme will format input (command) and output (response)
buffers this way:
- command:
legacy header +
extended header +
command data (core + hw):
+----------------------------------------+
| flags | 00 00 | command |
| in_words | out_words |
+----------------------------------------+
| response |
| response |
| provider_in_words | provider_out_words |
| padding |
+----------------------------------------+
| |
. <uverbs input> .
. (in_words * 8) .
| |
+----------------------------------------+
| |
. <provider input> .
. (provider_in_words * 8) .
| |
+----------------------------------------+
- response, if present:
+----------------------------------------+
| |
. <uverbs output space> .
. (out_words * 8) .
| |
+----------------------------------------+
| |
. <provider output space> .
. (provider_out_words * 8) .
| |
+----------------------------------------+
The overall design is to ensure that the extensible
infrastructure is itself extensible while begin more
reliable with more input and bound checking.
Note:
The unused field in the extended header would be 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[2]. But "comp_mask" field is likely to be present in the
uverb input and/or provider input, likewise for the response,
as noted by Matan Barak[3], so it doesn't make sense to put
"comp_mask" in the header.
[1]:
http://marc.info/?i=CAL1RGDWxmM17W2o_era24A-TTDeKyoL6u3NRu_=t_dhV_ZA9MA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
[2]:
http://marc.info/?i=CAL1RGDXJtrc849M6_XNZT5xO1+ybKtLWGq6yg6LhoSsKpsmkYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
[3]:
http://marc.info/?i=525C1149.6000701-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Cc: Igor Ivanov <Igor.Ivanov-wN0M4riKYwLQT0dZR+AlfA@public.gmane.org>
Cc: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
drivers/infiniband/core/uverbs.h | 20 +++++-
drivers/infiniband/core/uverbs_cmd.c | 56 +++++++--------
drivers/infiniband/core/uverbs_main.c | 127 ++++++++++++++++++++++++++--------
drivers/infiniband/hw/mlx4/main.c | 6 +-
include/rdma/ib_verbs.h | 1 +
include/uapi/rdma/ib_user_verbs.h | 23 +++---
6 files changed, 160 insertions(+), 73 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 777954f..24adccb 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -47,6 +47,14 @@
#include <rdma/ib_umem.h>
#include <rdma/ib_user_verbs.h>
+#define INIT_UDATA(udata, ibuf, obuf, ilen, olen) \
+ do { \
+ (udata)->inbuf = (void __user *) (ibuf); \
+ (udata)->outbuf = (void __user *) (obuf); \
+ (udata)->inlen = (ilen); \
+ (udata)->outlen = (olen); \
+ } while (0)
+
/*
* Our lifetime rules for these structs are the following:
*
@@ -233,9 +241,17 @@ IB_UVERBS_DECLARE_CMD(destroy_srq);
IB_UVERBS_DECLARE_CMD(create_xsrq);
IB_UVERBS_DECLARE_CMD(open_xrcd);
IB_UVERBS_DECLARE_CMD(close_xrcd);
+
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-IB_UVERBS_DECLARE_CMD(create_flow);
-IB_UVERBS_DECLARE_CMD(destroy_flow);
+
+#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)
+
+IB_UVERBS_DECLARE_EX_CMD(create_flow);
+IB_UVERBS_DECLARE_EX_CMD(destroy_flow);
+
#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
#endif /* UVERBS_H */
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 19f14d8..8ea9aa5 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -58,14 +58,6 @@ static struct uverbs_lock_class xrcd_lock_class = { .name = "XRCD-uobj" };
static struct uverbs_lock_class rule_lock_class = { .name = "RULE-uobj" };
#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
-#define INIT_UDATA(udata, ibuf, obuf, ilen, olen) \
- do { \
- (udata)->inbuf = (void __user *) (ibuf); \
- (udata)->outbuf = (void __user *) (obuf); \
- (udata)->inlen = (ilen); \
- (udata)->outlen = (olen); \
- } while (0)
-
/*
* The ib_uobject locking scheme is as follows:
*
@@ -2642,9 +2634,9 @@ static int kern_spec_to_ib_spec(struct ib_uverbs_flow_spec *kern_spec,
return 0;
}
-ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
- const char __user *buf, int in_len,
- int out_len)
+int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
+ struct ib_udata *ucore,
+ struct ib_udata *uhw)
{
struct ib_uverbs_create_flow cmd;
struct ib_uverbs_create_flow_resp resp;
@@ -2658,11 +2650,15 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
void *ib_spec;
int i;
- if (out_len < sizeof(resp))
+ if (ucore->outlen < sizeof(resp))
return -ENOSPC;
- if (copy_from_user(&cmd, buf, sizeof(cmd)))
- return -EFAULT;
+ err = ib_copy_from_udata(&cmd, ucore, sizeof(cmd));
+ if (err)
+ return err;
+
+ ucore->inbuf += sizeof(cmd);
+ ucore->inlen -= sizeof(cmd);
if (cmd.comp_mask)
return -EINVAL;
@@ -2674,7 +2670,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
if (cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
return -EINVAL;
- if (cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
+ if (cmd.flow_attr.size > ucore->inlen ||
cmd.flow_attr.size >
(cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
return -EINVAL;
@@ -2686,11 +2682,10 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
return -ENOMEM;
memcpy(kern_flow_attr, &cmd.flow_attr, sizeof(*kern_flow_attr));
- if (copy_from_user(kern_flow_attr + 1, buf + sizeof(cmd),
- cmd.flow_attr.size)) {
- err = -EFAULT;
+ err = ib_copy_from_udata(kern_flow_attr + 1, ucore,
+ cmd.flow_attr.size);
+ if (err)
goto err_free_attr;
- }
} else {
kern_flow_attr = &cmd.flow_attr;
}
@@ -2758,11 +2753,10 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
memset(&resp, 0, sizeof(resp));
resp.flow_handle = uobj->id;
- if (copy_to_user((void __user *)(unsigned long) cmd.response,
- &resp, sizeof(resp))) {
- err = -EFAULT;
+ err = ib_copy_to_udata(ucore,
+ &resp, sizeof(resp));
+ if (err)
goto err_copy;
- }
put_qp_read(qp);
mutex_lock(&file->mutex);
@@ -2775,7 +2769,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
kfree(flow_attr);
if (cmd.flow_attr.num_of_specs)
kfree(kern_flow_attr);
- return in_len;
+ return 0;
err_copy:
idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
destroy_flow:
@@ -2792,16 +2786,18 @@ err_free_attr:
return err;
}
-ssize_t ib_uverbs_destroy_flow(struct ib_uverbs_file *file,
- const char __user *buf, int in_len,
- int out_len) {
+int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
+ struct ib_udata *ucore,
+ struct ib_udata *uhw)
+{
struct ib_uverbs_destroy_flow cmd;
struct ib_flow *flow_id;
struct ib_uobject *uobj;
int ret;
- if (copy_from_user(&cmd, buf, sizeof(cmd)))
- return -EFAULT;
+ ret = ib_copy_from_udata(&cmd, ucore, sizeof(cmd));
+ if (ret)
+ return ret;
uobj = idr_write_uobj(&ib_uverbs_rule_idr, cmd.flow_handle,
file->ucontext);
@@ -2823,7 +2819,7 @@ ssize_t ib_uverbs_destroy_flow(struct ib_uverbs_file *file,
put_uobj(uobj);
- return ret ? ret : in_len;
+ return ret ? ret : 0;
}
#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 2df31f6..189d99e 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -115,11 +115,16 @@ static ssize_t (*uverbs_cmd_table[])(struct ib_uverbs_file *file,
[IB_USER_VERBS_CMD_CLOSE_XRCD] = ib_uverbs_close_xrcd,
[IB_USER_VERBS_CMD_CREATE_XSRQ] = ib_uverbs_create_xsrq,
[IB_USER_VERBS_CMD_OPEN_QP] = ib_uverbs_open_qp,
+};
+
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
- [IB_USER_VERBS_CMD_CREATE_FLOW] = ib_uverbs_create_flow,
- [IB_USER_VERBS_CMD_DESTROY_FLOW] = ib_uverbs_destroy_flow
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
+static int (*uverbs_ex_cmd_table[])(struct ib_uverbs_file *file,
+ struct ib_udata *ucore,
+ struct ib_udata *uhw) = {
+ [IB_USER_VERBS_EX_CMD_CREATE_FLOW] = ib_uverbs_ex_create_flow,
+ [IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow
};
+#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
static void ib_uverbs_add_one(struct ib_device *device);
static void ib_uverbs_remove_one(struct ib_device *device);
@@ -589,6 +594,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
{
struct ib_uverbs_file *file = filp->private_data;
struct ib_uverbs_cmd_hdr hdr;
+ __u32 flags;
if (count < sizeof hdr)
return -EINVAL;
@@ -596,45 +602,108 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
if (copy_from_user(&hdr, buf, sizeof hdr))
return -EFAULT;
- if (hdr.command >= ARRAY_SIZE(uverbs_cmd_table) ||
- !uverbs_cmd_table[hdr.command])
- return -EINVAL;
+ flags = (hdr.command &
+ IB_USER_VERBS_CMD_FLAGS_MASK) >> IB_USER_VERBS_CMD_FLAGS_SHIFT;
- if (!file->ucontext &&
- hdr.command != IB_USER_VERBS_CMD_GET_CONTEXT)
- return -EINVAL;
+ if (!flags) {
+ __u32 command;
- if (!(file->device->ib_dev->uverbs_cmd_mask & (1ull << hdr.command)))
- return -ENOSYS;
+ if (hdr.command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
+ IB_USER_VERBS_CMD_COMMAND_MASK))
+ return -EINVAL;
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
- if (hdr.command >= IB_USER_VERBS_CMD_THRESHOLD) {
- struct ib_uverbs_cmd_hdr_ex hdr_ex;
+ command = hdr.command & IB_USER_VERBS_CMD_COMMAND_MASK;
- if (copy_from_user(&hdr_ex, buf, sizeof(hdr_ex)))
- return -EFAULT;
+ if (command >= ARRAY_SIZE(uverbs_cmd_table) ||
+ !uverbs_cmd_table[command])
+ return -EINVAL;
- if (((hdr_ex.in_words + hdr_ex.provider_in_words) * 4) != count)
+ if (!file->ucontext &&
+ command != IB_USER_VERBS_CMD_GET_CONTEXT)
return -EINVAL;
- return uverbs_cmd_table[hdr.command](file,
- buf + sizeof(hdr_ex),
- (hdr_ex.in_words +
- hdr_ex.provider_in_words) * 4,
- (hdr_ex.out_words +
- hdr_ex.provider_out_words) * 4);
- } else {
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
+ if (!(file->device->ib_dev->uverbs_cmd_mask & (1ull << command)))
+ return -ENOSYS;
+
if (hdr.in_words * 4 != count)
return -EINVAL;
- return uverbs_cmd_table[hdr.command](file,
- buf + sizeof(hdr),
- hdr.in_words * 4,
- hdr.out_words * 4);
+ return uverbs_cmd_table[command](file,
+ buf + sizeof(hdr),
+ hdr.in_words * 4,
+ hdr.out_words * 4);
+
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
+
+ } else if (flags == IB_USER_VERBS_CMD_FLAG_EXTENDED) {
+ __u32 command;
+
+ struct ib_uverbs_ex_cmd_hdr ex_hdr;
+ struct ib_udata ucore;
+ struct ib_udata uhw;
+ int err;
+ size_t written_count = count;
+
+ if (hdr.command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
+ IB_USER_VERBS_CMD_COMMAND_MASK))
+ return -EINVAL;
+
+ command = hdr.command & IB_USER_VERBS_CMD_COMMAND_MASK;
+
+ if (command >= ARRAY_SIZE(uverbs_ex_cmd_table) ||
+ !uverbs_ex_cmd_table[command])
+ return -ENOSYS;
+
+ if (!file->ucontext)
+ return -EINVAL;
+
+ if (!(file->device->ib_dev->uverbs_ex_cmd_mask & (1ull << command)))
+ return -ENOSYS;
+
+ if (count < (sizeof(hdr) + sizeof(ex_hdr)))
+ return -EINVAL;
+
+ 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) {
+ if (!hdr.out_words && !ex_hdr.provider_out_words)
+ return -EINVAL;
+ } else {
+ if (hdr.out_words || ex_hdr.provider_out_words)
+ return -EINVAL;
+ }
+
+ INIT_UDATA(&ucore,
+ (hdr.in_words) ? buf : 0,
+ (unsigned long)ex_hdr.response,
+ hdr.in_words * 8,
+ hdr.out_words * 8);
+
+ 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_in_words * 8,
+ ex_hdr.provider_out_words * 8);
+
+ err = uverbs_ex_cmd_table[command](file,
+ &ucore,
+ &uhw);
+
+ if (err)
+ return err;
+
+ return written_count;
}
#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
+
+ return -ENOSYS;
}
static int ib_uverbs_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index f061264..4be29fe 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1692,9 +1692,9 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
ibdev->ib_dev.destroy_flow = mlx4_ib_destroy_flow;
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
- ibdev->ib_dev.uverbs_cmd_mask |=
- (1ull << IB_USER_VERBS_CMD_CREATE_FLOW) |
- (1ull << IB_USER_VERBS_CMD_DESTROY_FLOW);
+ ibdev->ib_dev.uverbs_ex_cmd_mask |=
+ (1ull << IB_USER_VERBS_EX_CMD_CREATE_FLOW) |
+ (1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index e393171..a06fc12 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1436,6 +1436,7 @@ struct ib_device {
int uverbs_abi_ver;
u64 uverbs_cmd_mask;
+ u64 uverbs_ex_cmd_mask;
char node_desc[64];
__be64 node_guid;
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index fc9bbe3..6ace125 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -87,11 +87,14 @@ enum {
IB_USER_VERBS_CMD_CLOSE_XRCD,
IB_USER_VERBS_CMD_CREATE_XSRQ,
IB_USER_VERBS_CMD_OPEN_QP,
+};
+
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
- IB_USER_VERBS_CMD_CREATE_FLOW = IB_USER_VERBS_CMD_THRESHOLD,
- IB_USER_VERBS_CMD_DESTROY_FLOW
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
+enum {
+ IB_USER_VERBS_EX_CMD_CREATE_FLOW = IB_USER_VERBS_CMD_THRESHOLD,
+ IB_USER_VERBS_EX_CMD_DESTROY_FLOW
};
+#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
/*
* Make sure that all structs defined in this file remain laid out so
@@ -122,6 +125,12 @@ struct ib_uverbs_comp_event_desc {
* the rest of the command struct based on these value.
*/
+#define IB_USER_VERBS_CMD_COMMAND_MASK 0xff
+#define IB_USER_VERBS_CMD_FLAGS_MASK 0xff000000u
+#define IB_USER_VERBS_CMD_FLAGS_SHIFT 24
+
+#define IB_USER_VERBS_CMD_FLAG_EXTENDED 0x80
+
struct ib_uverbs_cmd_hdr {
__u32 command;
__u16 in_words;
@@ -129,10 +138,8 @@ struct ib_uverbs_cmd_hdr {
};
#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-struct ib_uverbs_cmd_hdr_ex {
- __u32 command;
- __u16 in_words;
- __u16 out_words;
+struct ib_uverbs_ex_cmd_hdr {
+ __u64 response;
__u16 provider_in_words;
__u16 provider_out_words;
__u32 cmd_hdr_reserved;
@@ -782,8 +789,6 @@ struct ib_uverbs_flow_attr {
struct ib_uverbs_create_flow {
__u32 comp_mask;
- __u32 reserved;
- __u64 response;
__u32 qp_handle;
struct ib_uverbs_flow_attr flow_attr;
};
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH for -next v2 7/7] IB/core: re-enable create_flow/destroy_flow uverbs
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2013-11-06 22:21 ` [PATCH for -next v2 6/7] IB/core: extended command: an improved infrastructure for uverbs commands Yann Droneaud
@ 2013-11-06 22:21 ` Yann Droneaud
2013-11-07 9:33 ` [PATCH for-next v2 0/7] uverbs extensions fixes Matan Barak
7 siblings, 0 replies; 15+ messages in thread
From: Yann Droneaud @ 2013-11-06 22:21 UTC (permalink / raw)
To: Roland Dreier, Roland Dreier, Or Gerlitz, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
This commit revert "IB/core: Temporarily disable create_flow/destroy_flow uverbs"
(commit d3bebb918d78755da505f56727cdde2d9c8adbe2).
Since the uverbs extensions functionality was experimental for v3.12,
this patch re-enables the support for them and flow-steering
for v3.13.
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
drivers/infiniband/Kconfig | 11 -----------
drivers/infiniband/core/uverbs.h | 4 ----
drivers/infiniband/core/uverbs_cmd.c | 4 ----
drivers/infiniband/core/uverbs_main.c | 5 -----
drivers/infiniband/hw/mlx4/main.c | 2 --
include/uapi/rdma/ib_user_verbs.h | 6 ------
6 files changed, 32 deletions(-)
diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index b84791f..5ceda71 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -31,17 +31,6 @@ config INFINIBAND_USER_ACCESS
libibverbs, libibcm and a hardware driver library from
<http://www.openfabrics.org/git/>.
-config INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
- bool "Experimental and unstable ABI for userspace access to flow steering verbs"
- depends on INFINIBAND_USER_ACCESS
- depends on STAGING
- ---help---
- The final ABI for userspace access to flow steering verbs
- has not been defined. To use the current ABI, *WHICH WILL
- CHANGE IN THE FUTURE*, say Y here.
-
- If unsure, say N.
-
config INFINIBAND_USER_MEM
bool
depends on INFINIBAND_USER_ACCESS != n
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 24adccb..bdc842e 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -242,8 +242,6 @@ IB_UVERBS_DECLARE_CMD(create_xsrq);
IB_UVERBS_DECLARE_CMD(open_xrcd);
IB_UVERBS_DECLARE_CMD(close_xrcd);
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-
#define IB_UVERBS_DECLARE_EX_CMD(name) \
int ib_uverbs_ex_##name(struct ib_uverbs_file *file, \
struct ib_udata *ucore, \
@@ -252,6 +250,4 @@ IB_UVERBS_DECLARE_CMD(close_xrcd);
IB_UVERBS_DECLARE_EX_CMD(create_flow);
IB_UVERBS_DECLARE_EX_CMD(destroy_flow);
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
-
#endif /* UVERBS_H */
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 8ea9aa5..e35877d 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -54,9 +54,7 @@ static struct uverbs_lock_class qp_lock_class = { .name = "QP-uobj" };
static struct uverbs_lock_class ah_lock_class = { .name = "AH-uobj" };
static struct uverbs_lock_class srq_lock_class = { .name = "SRQ-uobj" };
static struct uverbs_lock_class xrcd_lock_class = { .name = "XRCD-uobj" };
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
static struct uverbs_lock_class rule_lock_class = { .name = "RULE-uobj" };
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
/*
* The ib_uobject locking scheme is as follows:
@@ -2593,7 +2591,6 @@ out_put:
return ret ? ret : in_len;
}
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
static int kern_spec_to_ib_spec(struct ib_uverbs_flow_spec *kern_spec,
union ib_flow_spec *ib_spec)
{
@@ -2821,7 +2818,6 @@ int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
return ret ? ret : 0;
}
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
struct ib_uverbs_create_xsrq *cmd,
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 189d99e..3438694 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -117,14 +117,12 @@ static ssize_t (*uverbs_cmd_table[])(struct ib_uverbs_file *file,
[IB_USER_VERBS_CMD_OPEN_QP] = ib_uverbs_open_qp,
};
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
static int (*uverbs_ex_cmd_table[])(struct ib_uverbs_file *file,
struct ib_udata *ucore,
struct ib_udata *uhw) = {
[IB_USER_VERBS_EX_CMD_CREATE_FLOW] = ib_uverbs_ex_create_flow,
[IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow
};
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
static void ib_uverbs_add_one(struct ib_device *device);
static void ib_uverbs_remove_one(struct ib_device *device);
@@ -633,8 +631,6 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
hdr.in_words * 4,
hdr.out_words * 4);
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
-
} else if (flags == IB_USER_VERBS_CMD_FLAG_EXTENDED) {
__u32 command;
@@ -701,7 +697,6 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
return written_count;
}
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
return -ENOSYS;
}
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 4be29fe..1aad9b3 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1691,11 +1691,9 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
ibdev->ib_dev.create_flow = mlx4_ib_create_flow;
ibdev->ib_dev.destroy_flow = mlx4_ib_destroy_flow;
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
ibdev->ib_dev.uverbs_ex_cmd_mask |=
(1ull << IB_USER_VERBS_EX_CMD_CREATE_FLOW) |
(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
}
mlx4_ib_alloc_eqs(dev, ibdev);
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 6ace125..cbfdd4c 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -89,12 +89,10 @@ enum {
IB_USER_VERBS_CMD_OPEN_QP,
};
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
enum {
IB_USER_VERBS_EX_CMD_CREATE_FLOW = IB_USER_VERBS_CMD_THRESHOLD,
IB_USER_VERBS_EX_CMD_DESTROY_FLOW
};
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
/*
* Make sure that all structs defined in this file remain laid out so
@@ -137,14 +135,12 @@ struct ib_uverbs_cmd_hdr {
__u16 out_words;
};
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
struct ib_uverbs_ex_cmd_hdr {
__u64 response;
__u16 provider_in_words;
__u16 provider_out_words;
__u32 cmd_hdr_reserved;
};
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
struct ib_uverbs_get_context {
__u64 response;
@@ -707,7 +703,6 @@ struct ib_uverbs_detach_mcast {
__u64 driver_data[0];
};
-#ifdef CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING
struct ib_uverbs_flow_spec_hdr {
__u32 type;
__u16 size;
@@ -802,7 +797,6 @@ struct ib_uverbs_destroy_flow {
__u32 comp_mask;
__u32 flow_handle;
};
-#endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */
struct ib_uverbs_create_srq {
__u64 response;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH for-next v2 0/7] uverbs extensions fixes
[not found] ` <cover.1383773832.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (6 preceding siblings ...)
2013-11-06 22:21 ` [PATCH for -next v2 7/7] IB/core: re-enable create_flow/destroy_flow uverbs Yann Droneaud
@ 2013-11-07 9:33 ` Matan Barak
7 siblings, 0 replies; 15+ messages in thread
From: Matan Barak @ 2013-11-07 9:33 UTC (permalink / raw)
To: Yann Droneaud, Roland Dreier, Roland Dreier, Or Gerlitz
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
On 7/11/2013 12:21 AM, Yann Droneaud wrote:
> This series is a continuous improvement for the uverbs
> extension mechanism and flow steering uverbs that were
> introduced as an experimental features for v3.12.
>
> I suggested and implemented the following improvements:
> - improved extensible/extended command infrastructure
> with clear split between core/ (eg. uverbs) and hw/ (eg. provider)
> portion of command and response;
> - structure renaming to match others uverbs public structs;
> - changes usage of the flow_attr.size to not count the
> "extended command header" but to describe only the size
> of the flow specs following flow_attr;
> - removed unneeded flow_spec structure that don't need to be
> exposed to userspace.
> - ensure 64bits alignment
>
> Changes from v1 (patchset [1]):
>
> - re-enable patch by Matan Barack as the last step
> - patch moving comp_mask in command header dropped following
> discussion about having comp_mask for provider data and response.
> - moved variable written_count in an inner block
>
> Changes from v0 (patchset [2]):
>
> - Re-enable flow steering verbs and the extension verbs mechanism.
> - Squashed patches 1 and 2 from the original series
> - ib_uverbs_write should return the number of bytes including the
> header's size (Patch 7).
>
> Changes from v(-1) (patchset [3] and patchset [4]):
>
> - merge infrastructure and uverbs flow fixes
> - rebased against Matan Barack fixes
> - removed changes on function name
>
> Changes from initial (patchset [5]):
>
> - move "comp_mask" in the command header
> - add a response header
> - add strict command length check
>
> [1] [PATCH for-next V1 0/8] uverbs extensions fixes
> http://marc.info/?i=1383126771-7658-1-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>
> [2] [PATCH 0/9] IB/core: batch of fix against create/destroy_flow uverbs for v3.12
> http://marc.info/?i=cover.1381510045.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
>
> [3] [PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)
> http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
>
> [4] [PATCH v2 0/4] IB/core: an improved infrastructure for uverbs commands
> http://marc.info/?i=cover.1381177342.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
>
> [5] [PATCH] An improved infrastructure for uverbs commands (My take at designing extensible scheme)
> http://marc.info/?i=1380039392-15480-1-git-send-email-ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
>
> Matan Barak (2):
> IB/core: clarify overflow/underflow checks on ib_create/destroy_flow
> IB/core: re-enable create_flow/destroy_flow uverbs
>
> Yann Droneaud (5):
> IB/core: Rename 'flow' structs to match other uverbs structs
> IB/core: Makes uverbs flow structure using names more similar to verbs
> ones
> IB/core: Uses a common header for uverbs flow_specs
> IB/core: Remove ib_uverbs_flow_spec structure from userspace
> IB/core: extended command: an improved infrastructure for uverbs
> commands
>
> drivers/infiniband/Kconfig | 11 ---
> drivers/infiniband/core/uverbs.h | 36 ++++++++--
> drivers/infiniband/core/uverbs_cmd.c | 96 ++++++++++++-------------
> drivers/infiniband/core/uverbs_main.c | 128 +++++++++++++++++++++++++---------
> drivers/infiniband/hw/mlx4/main.c | 8 +--
> include/rdma/ib_verbs.h | 1 +
> include/uapi/rdma/ib_user_verbs.h | 95 ++++++++++++++-----------
> 7 files changed, 228 insertions(+), 147 deletions(-)
>
Thanks Yann for sending the updated patches. They look good to me.
Regards,
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread