Re: [PATCH v2 2/8] fwctl: Basic ioctl dispatch for the character device

Jason Gunthorpe
2024-08-01T13:05:55Z

On Tue, Jul 30, 2024 at 06:28:08PM +0100, Jonathan Cameron wrote:
> > And the basic userspace pattern is:
> >
> > struct fwctl_info info = {.size = sizeof(info), ...);
> > ioctl(fd, FWCTL_INFO, &info);
> >
> > This works today and generates the 24 byte command.
> >
> > Tomorrow the kernel adds a new member:
> >
> > struct fwctl_info {
> > __u32 size;
> > __u32 flags;
> > __u32 out_device_type;
> > __u32 device_data_len;
> > __aligned_u64 out_device_data;
> > __aligned_u64 new_thing;
> > };
> >
> > Current builds of the userpace use a 24 byte command. A new kernel
> > will see the 24 bytes and behave as before.
> >
> > When I recompile the userspace with the updated header it will issue a
> > 32 byte command with no source change.
> >
> > Old kernel will see a 32 byte command with the trailing bytes it
> > doesn't understand as 0 and keep working.
> >
> > The new kernel will see the new_thing bytes are zero and behave the
> > same as before.
> >
> > If then the userspace decides to set new_thing the old kernel will
> > stop working. Userspace can use some 'try and fail' approach to try
> > again with new_thing = 0.
>
> I'm not keen on try and fail interfaces because they become messy
> if this has potentially be extended multiple times. Rest
> of argument is fair enough. Thanks for the explanation.

I'd say try-and-fail is just the universal option, if there is merit
we can put cap bits and other things to positively indicate increased
kernel capability.

We have quite a deep experiance on this topic now in RDMA, and there
we've been doing both options, depending on the situation.

For instance you might introduce a new API that returns FOO and extend
a prior API to optionally accept FOO as well. A cap flag that the new
API exists is useful [1], but it is not for the prior API. The
userspace can just blindly pass FOO to the prior API, and if it
happened to get a non-zero FOO somehow then the kernel must also
support it..

[1] "try and fail" works well here too you can invoke the IOCTL with a
0 size and you get ENOTTY if the IOCTL is not understood, and another
error code if it is.

Jason