Re: ioctl number change / backwards compatibility doubt
On Sat, Mar 12, 2022 at 01:05:46PM +1300, Paulo Miguel Almeida wrote: > On Mon, Jan 24, 2022 at 07:20:45AM +0100, Greg KH wrote: > > On Mon, Jan 24, 2022 at 05:49:06PM +1300, Paulo Miguel Almeida wrote: > > > 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 /* ioctl */ > > > > > > int file_desc = open("/dev/pi433.0", O_RDWR); > > > struct pi433_tx_cfg tx_cfg = { > > > .frequency = 43300, > > > .bit_rate = 4800, > > > ... > > > }; > > > > > > 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 > > 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, ); > 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) For staging drivers, the user/kernel api usually needs to be changed, so yes, as long as you can change the tools that are being used to talk to this api, you should be fine. thanks, greg k-h ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Friday, 11 March 2022, Paulo Miguel Almeida < paulo.miguel.almeida.rode...@gmail.com> wrote: > On Mon, Jan 24, 2022 at 07:20:45AM +0100, Greg KH wrote: > > On Mon, Jan 24, 2022 at 05:49:06PM +1300, Paulo Miguel Almeida wrote: > > > 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 /* ioctl */ > > > > > > int file_desc = open("/dev/pi433.0", O_RDWR); > > > struct pi433_tx_cfg tx_cfg = { > > > .frequency = 43300, > > > .bit_rate = 4800, > > > ... > > > }; > > > > > > 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 > > 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, ); > 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 > Kernelnewbies@kernelnewbies.org > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies > No, it isn't OK. If a struct name is changed, it breaks source compatibility. -Rogério Valentim ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Mon, Jan 24, 2022 at 07:20:45AM +0100, Greg KH wrote: > On Mon, Jan 24, 2022 at 05:49:06PM +1300, Paulo Miguel Almeida wrote: > > 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 /* ioctl */ > > > > int file_desc = open("/dev/pi433.0", O_RDWR); > > struct pi433_tx_cfg tx_cfg = { > > .frequency = 43300, > > .bit_rate = 4800, > > ... > > }; > > > > 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 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, ); 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 Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Mon, Jan 24, 2022 at 05:49:06PM +1300, Paulo Miguel Almeida wrote: > On Sun, Jan 23, 2022 at 12:04:48PM +0100, Greg KH wrote: > > On Sun, Jan 23, 2022 at 08:55:30PM +1300, Paulo Miguel Almeida wrote: > > > > > > 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 /* ioctl */ > > int file_desc = open("/dev/pi433.0", O_RDWR); > struct pi433_tx_cfg tx_cfg = { > .frequency = 43300, > .bit_rate = 4800, > ... > }; > > 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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Sun, Jan 23, 2022 at 12:04:48PM +0100, Greg KH wrote: > On Sun, Jan 23, 2022 at 08:55:30PM +1300, Paulo Miguel Almeida wrote: > > > > 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 /* ioctl */ int file_desc = open("/dev/pi433.0", O_RDWR); struct pi433_tx_cfg tx_cfg = { .frequency = 43300, .bit_rate = 4800, ... }; int ret_val = ioctl(file_desc, PI433_IOC_WR_TX_CFG, tx_cfg); thanks, Paulo Almeida ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Sun, Jan 23, 2022 at 08:55:30PM +1300, Paulo Miguel Almeida wrote: > > > 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 :( If there is no tool, why was the ioctl code written at all? Something had to call it. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Mon, Jan 17, 2022 at 01:58:42PM +0100, Greg KH wrote: > 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. Noted > > 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 :( > > 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. > Dan Carpenter suggested in one of patch reviews that we keep the ioctl for backwards compatibility but start a brand new sysfs implementation to encompass existing functionality to make it easier to add new features in future. I will wrap my head around it and send some RFC patches soon. thanks, Paulo Almeida ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: ioctl number change / backwards compatibility doubt
On Mon, Jan 17, 2022 at 08:01:25PM +1300, Paulo Miguel Almeida wrote: > 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 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. > 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) Great! > 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? 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. > 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. thanks, greg k-h ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies