On Sun, Nov 08, 2015 at 05:04:55PM +0200, Matan Barak wrote:
> > +static int verify_command_mask(struct ib_device *ib_dev, __u32 command)
> > +{
> > + u64 mask;
> > +
> > + if (command <= IB_USER_VERBS_CMD_OPEN_QP)
>
> I think using IB_USER_VERBS_CMD_THRESHOLD is clearer, but I don't
> think we need two uverbs_cmd_mask vendor variables.
> IMHO, a vendor shouldn't care if it's an extended/legacy uverb
> command. Maybe we should replace uverbs_cmd_mask with a bitmap.
>
> > + mask = ib_dev->uverbs_cmd_mask;
> > + else
> > + mask = ib_dev->uverbs_ex_cmd_mask;
> > +
> > + if (mask & ((u64)1 << command))
> > + return 0;
> > +
> > + return -1;
> > +}
> > +
But IB_USER_VERBS_CMD_OPEN_QP is the last legacy verbs and I prefer a
more restrictive approach to avoid potentail issues in the future.
> > 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 = -EINVAL;
>
> Why did you replace ENOSYS with EINVAL?
>
Like Haggai mentioned in the other response, checkpatch issues error
on this claiming that ENOSYS is reserved to unavailable system calls.
Is it applicable only for new implementations I am not sure. I don't
have clear preference for either ENOSYS or EINAVL.
> > + goto out;
> > + }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
