* ioctl number change / backwards compatibility doubt @ 2022-01-17 7:01 Paulo Miguel Almeida 2022-01-17 12:58 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Paulo Miguel Almeida @ 2022-01-17 7:01 UTC (permalink / raw) To: kernelnewbies; +Cc: paulo.miguel.almeida.rodenas 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? Best regards, Paulo Almeida _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-17 7:01 ioctl number change / backwards compatibility doubt Paulo Miguel Almeida @ 2022-01-17 12:58 ` Greg KH 2022-01-23 7:55 ` Paulo Miguel Almeida 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2022-01-17 12:58 UTC (permalink / raw) To: Paulo Miguel Almeida, kernelnewbies 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-17 12:58 ` Greg KH @ 2022-01-23 7:55 ` Paulo Miguel Almeida 2022-01-23 11:04 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Paulo Miguel Almeida @ 2022-01-23 7:55 UTC (permalink / raw) To: Greg KH; +Cc: kernelnewbies 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-23 7:55 ` Paulo Miguel Almeida @ 2022-01-23 11:04 ` Greg KH 2022-01-24 4:49 ` Paulo Miguel Almeida 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2022-01-23 11:04 UTC (permalink / raw) To: Paulo Miguel Almeida, kernelnewbies 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-23 11:04 ` Greg KH @ 2022-01-24 4:49 ` Paulo Miguel Almeida 2022-01-24 6:20 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Paulo Miguel Almeida @ 2022-01-24 4:49 UTC (permalink / raw) To: Greg KH; +Cc: kernelnewbies 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 <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); .... thanks, Paulo Almeida _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-24 4:49 ` Paulo Miguel Almeida @ 2022-01-24 6:20 ` Greg KH 2022-03-12 0:05 ` Paulo Miguel Almeida 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2022-01-24 6:20 UTC (permalink / raw) To: Paulo Miguel Almeida, kernelnewbies 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 <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); > .... 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-01-24 6:20 ` Greg KH @ 2022-03-12 0:05 ` Paulo Miguel Almeida 2022-03-14 12:30 ` Rogério Valentim Feitoza da Silva 2022-03-16 14:07 ` Greg KH 0 siblings, 2 replies; 9+ messages in thread From: Paulo Miguel Almeida @ 2022-03-12 0:05 UTC (permalink / raw) To: Greg KH; +Cc: kernelnewbies 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 <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); > > .... > > 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 Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-03-12 0:05 ` Paulo Miguel Almeida @ 2022-03-14 12:30 ` Rogério Valentim Feitoza da Silva 2022-03-16 14:07 ` Greg KH 1 sibling, 0 replies; 9+ messages in thread From: Rogério Valentim Feitoza da Silva @ 2022-03-14 12:30 UTC (permalink / raw) To: Paulo Miguel Almeida, Greg KH, kernelnewbies@kernelnewbies.org [-- Attachment #1.1: Type: text/plain, Size: 3324 bytes --] On Friday, 11 March 2022, Paulo Miguel Almeida < paulo.miguel.almeida.rodenas@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 <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); > > > .... > > > > 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 > 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 [-- Attachment #1.2: Type: text/html, Size: 4230 bytes --] [-- Attachment #2: Type: text/plain, Size: 170 bytes --] _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ioctl number change / backwards compatibility doubt 2022-03-12 0:05 ` Paulo Miguel Almeida 2022-03-14 12:30 ` Rogério Valentim Feitoza da Silva @ 2022-03-16 14:07 ` Greg KH 1 sibling, 0 replies; 9+ messages in thread From: Greg KH @ 2022-03-16 14:07 UTC (permalink / raw) To: Paulo Miguel Almeida, kernelnewbies 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 <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); > > > .... > > > > 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) 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-16 14:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-17 7:01 ioctl number change / backwards compatibility doubt Paulo Miguel Almeida 2022-01-17 12:58 ` Greg KH 2022-01-23 7:55 ` Paulo Miguel Almeida 2022-01-23 11:04 ` Greg KH 2022-01-24 4:49 ` Paulo Miguel Almeida 2022-01-24 6:20 ` Greg KH 2022-03-12 0:05 ` Paulo Miguel Almeida 2022-03-14 12:30 ` Rogério Valentim Feitoza da Silva 2022-03-16 14:07 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).