ioctl number change / backwards compatibility doubt

Hi everyone,

Context:

I've been working on a driver called pi433 in the staging area and it basically exposes a char device so the user can read/write stuff to it while obtaining tx/rx configuration via ioctl calls.

Tx/Rx configurations are both structs that (ideally) would be exposed to the userspace to let the developer to #include it on their code.

Info that *might* be relevant about this driver:

- This driver went straight to the staging area due to a few TODOs listed by the original author.
- The ioctl Code and Seq numbers are conflicting with the ones listed in the ioctl-numbers.rst

Problem:

I realized that one of the structs used to pass/retrieve info needs to have some of its members changed (data type and etc)

Questions:

1: Given the driver's history and ioctl number conflit, is the backwards compatibility something to be kept or not to be taken into consideration as ioctl numbering rules weren't followed?

2: The original author lists on the TODO file of the driver that 'he is afraid that using ioctl wasn't a good idea'. I pondered the alternatives and, *in case I can get rid of ioctl*, sysfs || configfs could be used. Does anyone suggests a different approach?

Not many people ever look at that file, and it is ok to have conflicts as the same tool should never have to handle multiple drivers where a conflict happens.

Quoted 3 lines

1: Given the driver's history and ioctl number conflit, is the backwards compatibility something to be kept or not to be taken into consideration as ioctl numbering rules weren't followed?

Try to find out who is using these ioctls. If you can change the userspace tool at the same time, all is fine. If not, then there can be problems.

Quoted 4 lines

2: The original author lists on the TODO file of the driver that 'he is afraid that using ioctl wasn't a good idea'. I pondered the alternatives and, *in case I can get rid of ioctl*, sysfs || configfs could be used. Does anyone suggests a different approach?

Same answer as above, it depends on what userspace tool is using these ioctls. Also it depends on what they do. Many informational ioctls can just be replaced with sysfs files, and many configuration ioctls can be replaced with configfs, but for other things, sometimes you need an ioctl.

So it depends. Try to get ahold of the userspace side and then you can usually work it out.

Quoted 7 lines

Quoted 3 lines

1: Given the driver's history and ioctl number conflit, is the backwards compatibility something to be kept or not to be taken into consideration as ioctl numbering rules weren't followed?

Try to find out who is using these ioctls. If you can change the userspace tool at the same time, all is fine. If not, then there can be problems.

Apologies for the delay, I had emailed the original author and I was waiting for his reply before I could answer this. It turns out I haven't gotten an official answer from him yet. (I do understand that he might be busy)

I googled a fair bit of time and I'm 99% confident that there isn't such userspace/lib tool so I guess this will have done the hard way :(

Quoted 3 lines · Paulo Miguel Almeida

I googled a fair bit of time and I'm 99% confident that there isn't such userspace/lib tool so I guess this will have done the hard way :(

If there is no tool, why was the ioctl code written at all? Something had to call it.

Quoted 8 lines · Greg KH

Quoted 3 lines · Paulo Miguel Almeida

I googled a fair bit of time and I'm 99% confident that there isn't such userspace/lib tool so I guess this will have done the hard way :(

If there is no tool, why was the ioctl code written at all? Something had to call it.

when you told me to look for the userspace tool that interfaced with the ioctl, my interpretation was that you were referring to something akin to what /usr/bin/uname utility is to the syscall uname. Please correct me if I'm wrong.

re: what calls the ioctl created by the driver.

I'm led to believe that users of this driver make ioctl sycall invocations directly from their application's source code like this:

#include "pi433_if.h" /* userspace driver header */
#include <sys/ioctl.h> /* ioctl */

int file_desc = open("/dev/pi433.0", O_RDWR);

struct pi433_tx_cfg tx_cfg = {
	.frequency = 433000000,
	.bit_rate = 4800,
	<omitted for brevity>...
};

int ret_val = ioctl(file_desc, PI433_IOC_WR_TX_CFG, tx_cfg);
....

Quoted 16 lines · Paulo Miguel Almeida

On Sun, Jan 23, 2022 at 12:04:48PM +0100, Greg KH wrote:

when you told me to look for the userspace tool that interfaced with the ioctl, my interpretation was that you were referring to something akin to what /usr/bin/uname utility is to the syscall uname. Please correct me if I'm wrong.

re: what calls the ioctl created by the driver.

I'm led to believe that users of this driver make ioctl sycall invocations directly from their application's source code like this:

#include "pi433_if.h" /* userspace driver header */
#include <sys/ioctl.h> /* ioctl */

int file_desc = open("/dev/pi433.0", O_RDWR);

Quoted 8 lines

struct pi433_tx_cfg tx_cfg = {
	.frequency = 433000000,
	.bit_rate = 4800,
	<omitted for brevity>...
};

int ret_val = ioctl(file_desc, PI433_IOC_WR_TX_CFG, tx_cfg);
....

Yes, sorry for the confusion, this is what I am referring to. Where is that userspace code as that is the code you will be needing to change if you want to change this ioctl interface.

thanks,

greg k-h

Quoted 4 lines · Greg KH

Quoted 3 lines · Paulo Miguel Almeida

On Sun, Jan 23, 2022 at 12:04:48PM +0100, Greg KH wrote:

when you told me to look for the userspace tool that interfaced with

the

Quoted 2 lines

Quoted 2 lines

ioctl, my interpretation was that you were referring to something akin to what /usr/bin/uname utility is to the syscall uname. Please correct

me

Quoted 20 lines

Quoted 11 lines

if I'm wrong.

re: what calls the ioctl created by the driver.

I'm led to believe that users of this driver make ioctl sycall invocations directly from their application's source code like this:

#include "pi433_if.h" /* userspace driver header */
#include <sys/ioctl.h> /* ioctl */

int file_desc = open("/dev/pi433.0", O_RDWR);

Quoted 8 lines

struct pi433_tx_cfg tx_cfg = {
    .frequency = 433000000,
    .bit_rate = 4800,
    <omitted for brevity>...
};

int ret_val = ioctl(file_desc, PI433_IOC_WR_TX_CFG, tx_cfg);
....

Quoted 7 lines

Yes, sorry for the confusion, this is what I am referring to. Where is that userspace code as that is the code you will be needing to change if you want to change this ioctl interface.

thanks,

greg k-h

Hi Greg,

The original author replied my email with that question. He sent me some the code used to interface with char device, however, the source code he provided me is already incompatible with the current version of the driver:

#include "rfxx.h" (file header name has changed)

int main(int argc, char *argv[])

{
        ...
        struct rfxx_send_config sendConfig; (struct name has changed)
        ...
        fd = open("/dev/rf69_0.0", O_RDWR); *(char device name changed)*
        ...
        sendConfig.data_mode = packet; *(property doesn't exist)*
        ...
        (IOCTL call has a different name)
        ret = ioctl(fd, RFXX_IOC_WR_SEND_CONFIG, &sendConfig);
        if (ret == -1)
        ...
}

Assuming that I tweak these tools he provided me with and publish them, will I then be able to tweak the structures passed back and forth via ioctl? (do I need to change ioctl number in this case?)

The reason why I'm asking this is because given the fact that other developers could have written similar code for interfacing with the driver (and that we will never know because code is proprietary), I won't be sure that I've changed all occurences at the same time, right?

All in all, please correct if there are gaps in this train of thought.

- If a change doesn't break *compiled* code (such as struct renaming) then it's 'okay' to make the change ? (this has happened in the this driver before)

Best regards,

Paulo Almeida

_______________________________________________
Kernelnewbies mailing list
[email protected]
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 26 lines · Greg KH

Quoted 16 lines · Paulo Miguel Almeida

On Sun, Jan 23, 2022 at 12:04:48PM +0100, Greg KH wrote:

when you told me to look for the userspace tool that interfaced with the ioctl, my interpretation was that you were referring to something akin to what /usr/bin/uname utility is to the syscall uname. Please correct me if I'm wrong.

re: what calls the ioctl created by the driver.

I'm led to believe that users of this driver make ioctl sycall invocations directly from their application's source code like this:

#include "pi433_if.h" /* userspace driver header */
#include <sys/ioctl.h> /* ioctl */

int file_desc = open("/dev/pi433.0", O_RDWR);

Quoted 8 lines

struct pi433_tx_cfg tx_cfg = {
	.frequency = 433000000,
	.bit_rate = 4800,
	<omitted for brevity>...
};

int ret_val = ioctl(file_desc, PI433_IOC_WR_TX_CFG, tx_cfg);
....

Quoted 7 lines

Yes, sorry for the confusion, this is what I am referring to. Where is that userspace code as that is the code you will be needing to change if you want to change this ioctl interface.

thanks,

greg k-h

Hi Greg,

The original author replied my email with that question. He sent me some the code used to interface with char device, however, the source code he provided me is already incompatible with the current version of the driver:

#include "rfxx.h" (file header name has changed)

int main(int argc, char *argv[])

{
	...
	struct rfxx_send_config sendConfig; (struct name has changed)
	...
        fd = open("/dev/rf69_0.0", O_RDWR); *(char device name changed)*
	...
	sendConfig.data_mode = packet; *(property doesn't exist)*
        ...
	(IOCTL call has a different name)
	ret = ioctl(fd, RFXX_IOC_WR_SEND_CONFIG, &sendConfig);
        if (ret == -1)
	...
}

Assuming that I tweak these tools he provided me with and publish them, will I then be able to tweak the structures passed back and forth via ioctl? (do I need to change ioctl number in this case?)

The reason why I'm asking this is because given the fact that other developers could have written similar code for interfacing with the driver (and that we will never know because code is proprietary), I won't be sure that I've changed all occurences at the same time, right?

All in all, please correct if there are gaps in this train of thought.

- If a change doesn't break *compiled* code (such as struct renaming) then it's 'okay' to make the change ? (this has happened in the this driver before)