RDMA/uverbs: Use an unambiguous errno for method not supported
PATCH 1/1 RDMA/uverbs: Use an unambiguous errno for method not supported
Returning EOPNOTSUPP is problematic because it can also be returned by the method function, and we use it in quite a few places in drivers these days.
Instead, dedicate EPROTONOSUPPORT to indicate that the ioctl framework is enabled but the requested object and method are not supported by the kernel. No other case will return this code, and it lets userspace know to fall back to write().
grep says we do not use it today in drivers/infiniband subsystem.
--- drivers/infiniband/core/uverbs_ioctl.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
This goes along with the series I am cooking to enable the new ioctl interface in rdma-core:
https://github.com/jgunthorpe/rdma-plumbing/tree/ioctl
drivers/infiniband/core/uverbs_ioctl.c +13 -6
@@ -243,16 +243,13 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,size_t ctx_size;uintptr_t data[UVERBS_OPTIMIZE_USING_STACK_SZ /sizeof (uintptr_t )]; -if (hdr->reserved) -return -EINVAL ; - object_spec =uverbs_get_object (ib_dev, hdr->object_id);if (!object_spec) -return -EOPNOTSUPP ; +return -EPROTONOSUPPORT ; method_spec =uverbs_get_method (object_spec, hdr->method_id);if (!method_spec) -return -EOPNOTSUPP ; +return -EPROTONOSUPPORT ;if ((method_spec->flags &UVERBS_ACTION_FLAG_CREATE_ROOT ) ^ !file->ucontext)return -EINVAL ;
@@ -305,6 +302,16 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev, err =uverbs_handle_method (buf, ctx->uattrs, hdr->num_attrs, ib_dev, file, method_spec, ctx->uverbs_attr_bundle); + +/* +* EPROTONOSUPPORT is ONLY to be returned if the ioctl framework can +* not invoke the method because the request is not supported. No +* other cases should return this code. +*/ +if (unlikely (err == -EPROTONOSUPPORT )) { +WARN_ON_ONCE (err == -EPROTONOSUPPORT ); + err = -EINVAL ; + } out:if (ctx != (void *)data)kfree (ctx);
@@ -341,7 +348,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) }if (hdr.reserved) { - err = -EOPNOTSUPP ; + err = -EPROTONOSUPPORT ; goto out; }
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180125025834.GA25674-uk2M96/[email protected]
Matan Barak
Reviewed-byQuoted 63 lines · Jason Gunthorpe
Returning EOPNOTSUPP is problematic because it can also be returned by the method function, and we use it in quite a few places in drivers these days.
Instead, dedicate EPROTONOSUPPORT to indicate that the ioctl framework is enabled but the requested object and method are not supported by the kernel. No other case will return this code, and it lets userspace know to fall back to
write().grep says we do not use it today in
drivers/infinibandsubsystem.--- drivers/infiniband/core/uverbs_ioctl.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)This goes along with the series I am cooking to enable the new ioctl interface in rdma-core:
https://github.com/jgunthorpe/rdma-plumbing/tree/ioctl
drivers/infiniband/core/uverbs_ioctl.c +12
-5@@ -243,16 +243,13 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,size_t ctx_size;uintptr_t data[UVERBS_OPTIMIZE_USING_STACK_SZ /sizeof (uintptr_t )]; -if (hdr->reserved) -return -EINVAL ; - object_spec =uverbs_get_object (ib_dev, hdr->object_id);if (!object_spec) -return -EOPNOTSUPP ; +return -EPROTONOSUPPORT ; method_spec =uverbs_get_method (object_spec, hdr->method_id);if (!method_spec) -return -EOPNOTSUPP ; +return -EPROTONOSUPPORT ;if ((method_spec->flags &UVERBS_ACTION_FLAG_CREATE_ROOT ) ^ !file->ucontext)return -EINVAL ;@@ -305,6 +302,16 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev, err =uverbs_handle_method (buf, ctx->uattrs, hdr->num_attrs, ib_dev, file, method_spec, ctx->uverbs_attr_bundle); + +/* +* EPROTONOSUPPORT is ONLY to be returned if the ioctl framework can +* not invoke the method because the request is not supported. No +* other cases should return this code. +*/ +if (unlikely (err == -EPROTONOSUPPORT )) { +WARN_ON_ONCE (err == -EPROTONOSUPPORT ); + err = -EINVAL ; + } out:if (ctx != (void *)data)kfree (ctx);
drivers/infiniband/core/uverbs_ioctl.c long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) · line 354
Quoted 8 lines
drivers/infiniband/core/uverbs_ioctl.c +1
-1@@ -341,7 +348,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) }if (hdr.reserved) { - err = -EOPNOTSUPP ; + err = -EPROTONOSUPPORT ; goto out; }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Ledford
AppliedQuoted 4 lines · Jason Gunthorpe
Returning EOPNOTSUPP is problematic because it can also be returned by the method function, and we use it in quite a few places in drivers these days.
Quoted 8 lines
Instead, dedicate EPROTONOSUPPORT to indicate that the ioctl framework is enabled but the requested object and method are not supported by the kernel. No other case will return this code, and it lets userspace know to fall back to
write().grep says we do not use it today in
drivers/infinibandsubsystem.
Thanks, applied.
Signature · 3 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD