Re: [PATCH ib-next 2/3] IB/core: IB/core: Allow legacy verbs through extended interfaces

[Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

On Mon, Nov 09, 2015 at 03:35:31PM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 09, 2015 at 08:48:36AM +0200, Haggai Eran wrote:
> > On 08/11/2015 17:04, Matan Barak wrote:
> > >> @@ -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?
> > 
> > I think ENOSYS is meant only to represent a system call number that
> > isn't supported. There was a change in checkpatch that now warns about
> > it [1]. I'm not sure the intention was to fix existing uses though.
> 
> Within verbs we should have two kinds of return - not supported
> request, and supported request with invalid parameters.
> 
> Maybe use EOPNOTSUPP ?
> 

What about Matan's concern regarding legacy code? Maybe we should
leave ENOSYS as that's how it is all over the IB stack code.

quote:
I think it could break old applications:
err = extended_verb(the_first_extension_we_added);
if (err == ENOSYS)
      err = legacy_verb();
if (err)
    return err;

Such applications used the first extension (that was added during the
addition of the extended verb) and when they realized it's not
supported, they dropped to the legacy verb. This change can now cause
the return of -EINVAL an early termination with an error.

--
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