| Message ID | 1447178410-15360-3-git-send-email-eli@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Nov 10, 2015 at 08:00:09PM +0200, Eli Cohen wrote: > When an extended verbs is an extension to a legacy verb, the original > functionality is preserved. Hence we do not require each hardware driver > to set the extended capability. This will allow to use the extended verb > in its simple form with drivers that do not support the extended > capability. Can we please get rid of uverbs_cmd_mask and uverbs_ex_cmd_mask ? The driver should set the function pointers it does not support to NULL. We don't need the bitmask because we already know the answer. For performance, I recommend having the core replace the NULL op pointers from the driver with a stub that returns ENOTSUP, and then drop all the checking in the fast path. Jason -- 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 Tue, Nov 10, 2015 at 11:21:07AM -0700, Jason Gunthorpe wrote: > On Tue, Nov 10, 2015 at 08:00:09PM +0200, Eli Cohen wrote: > > When an extended verbs is an extension to a legacy verb, the original > > functionality is preserved. Hence we do not require each hardware driver > > to set the extended capability. This will allow to use the extended verb > > in its simple form with drivers that do not support the extended > > capability. > > Can we please get rid of uverbs_cmd_mask and uverbs_ex_cmd_mask ? > > The driver should set the function pointers it does not support to > NULL. We don't need the bitmask because we already know the answer. > > For performance, I recommend having the core replace the NULL op > pointers from the driver with a stub that returns ENOTSUP, and then > drop all the checking in the fast path. > Yes, we can do this but I think this should be the subject for another patch, agree? Regarding using stabs, it may be nice but I don't think performance is the issue here. Most verbs implementations involve relatively long i/o operations anyway so the gain will not be noticable. -- 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 Tue, Nov 10, 2015 at 09:57:13PM +0200, Eli Cohen wrote: > Yes, we can do this but I think this should be the subject for another > patch, agree? Sure > Regarding using stabs, it may be nice but I don't think performance is > the issue here. Most verbs implementations involve relatively long i/o > operations anyway so the gain will not be noticable. Intel keeps optimizing this common path since they use it for post.. Jason -- 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_main.c b/drivers/infiniband/core/uverbs_main.c index e93ba9125198..ecb907385b85 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -672,6 +672,21 @@ out: return ev_file; } +static int verify_command_mask(struct ib_device *ib_dev, __u32 command) +{ + u64 mask; + + if (command <= IB_USER_VERBS_CMD_OPEN_QP) + mask = ib_dev->uverbs_cmd_mask; + else + mask = ib_dev->uverbs_ex_cmd_mask; + + if (mask & ((u64)1 << command)) + return 0; + + return -1; +} + static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, size_t count, loff_t *pos) { @@ -704,6 +719,10 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, } command = hdr.command & IB_USER_VERBS_CMD_COMMAND_MASK; + if (verify_command_mask(ib_dev, command)) { + ret = -EOPNOTSUPP; + goto out; + } flags = (hdr.command & IB_USER_VERBS_CMD_FLAGS_MASK) >> IB_USER_VERBS_CMD_FLAGS_SHIFT; @@ -721,11 +740,6 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, goto out; } - if (!(ib_dev->uverbs_cmd_mask & (1ull << command))) { - ret = -ENOSYS; - goto out; - } - if (hdr.in_words * 4 != count) { ret = -EINVAL; goto out; @@ -753,11 +767,6 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, goto out; } - if (!(ib_dev->uverbs_ex_cmd_mask & (1ull << command))) { - ret = -ENOSYS; - goto out; - } - if (count < (sizeof(hdr) + sizeof(ex_hdr))) { ret = -EINVAL; goto out;
When an extended verbs is an extension to a legacy verb, the original functionality is preserved. Hence we do not require each hardware driver to set the extended capability. This will allow to use the extended verb in its simple form with drivers that do not support the extended capability. Signed-off-by: Eli Cohen <eli@mellanox.com> --- Changes from previous version: If verify_command_mask fails return -EOPNOTSUPP. drivers/infiniband/core/uverbs_main.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)