| Message ID | 2c3b64e8b42c3bd7434b03e3439c58df874992f2.1383773832.git.ydroneaud@opteya.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
Hi, It seems that even after so many iterations, I failed to provide a "perfect" patch. Le 06.11.2013 23:21, Yann Droneaud a écrit : > 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 > @@ -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; ^^^^^^^^^^^^^^ What a complicated way to mean + return ret; > } > #endif /* CONFIG_INFINIBAND_EXPERIMENTAL_UVERBS_FLOW_STEERING */ > Will roll a v3 if Roland have not yet picked the patchset. Regards.
On Fri, Nov 8, 2013 at 12:01 PM, Yann Droneaud <ydroneaud@opteya.com> wrote: > Hi, > > It seems that even after so many iterations, > I failed to provide a "perfect" patch. You, that perfect is the biggest enemy of the excellent, it (patches/code) is what it is (human made) and sometimes things need to be fixed, Roland has not full this, so feel free to roll V3 just for this tiny fix, if you like. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 8, 2013 at 2:01 AM, Yann Droneaud <ydroneaud@opteya.com> wrote:
> Will roll a v3 if Roland have not yet picked the patchset.
I'll roll that cleanup into this patch as I apply it (now).
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Nov 9, 2013 at 12:22 AM, Roland Dreier <roland@purestorage.com> wrote: > On Fri, Nov 8, 2013 at 2:01 AM, Yann Droneaud <ydroneaud@opteya.com> wrote: >> Will roll a v3 if Roland have not yet picked the patchset. > > I'll roll that cleanup into this patch as I apply it (now). Hi Roland, I don't see this series in your kernel.org clone, anything went wrong or its work in progress? Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Nov 9, 2013 Roland Dreier <roland@purestorage.com> wrote: > On Fri, Nov 8, 2013 Yann Droneaud <ydroneaud@opteya.com> wrote: >> Will roll a v3 if Roland have not yet picked the patchset. > I'll roll that cleanup into this patch as I apply it (now). Hi Roland, just to make sure this doesn't fall between the cracks and we are to loose aother kernel cycle... for some reason this series doesn't show up on the kernel.org infiniband tree for-next branch. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Yann, Or- Q: Do u guys have the corresponding libibverbs changes? Upinder On Nov 6, 2013, at 2:21 PM, Yann Droneaud <ydroneaud@opteya.com> wrote: > 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@mail.gmail.com > > [2]: > http://marc.info/?i=CAL1RGDXJtrc849M6_XNZT5xO1+ybKtLWGq6yg6LhoSsKpsmkYA@mail.gmail.com > > [3]: > http://marc.info/?i=525C1149.6000701@mellanox.com > > Cc: Igor Ivanov <Igor.Ivanov@itseez.com> > Cc: Matan Barak <matanb@mellanox.com> > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com> > Link: http://marc.info/?i=cover.1383773832.git.ydroneaud@opteya.com > --- > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; };
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@mail.gmail.com [2]: http://marc.info/?i=CAL1RGDXJtrc849M6_XNZT5xO1+ybKtLWGq6yg6LhoSsKpsmkYA@mail.gmail.com [3]: http://marc.info/?i=525C1149.6000701@mellanox.com Cc: Igor Ivanov <Igor.Ivanov@itseez.com> Cc: Matan Barak <matanb@mellanox.com> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com> Link: http://marc.info/?i=cover.1383773832.git.ydroneaud@opteya.com --- 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(-)