-1 from me, sorry, reading previous messages this seems to break too many things, just to adhere to a specific solution that is not standard compliant too.. we need to search for better solution :-)
Would it be possible to create generic crc16() and crc32() calls, like Alan and Sebastien suggests, that would provide / support various implementations based on parameters provided? If so then we could replace old methods with a wrapper to a new call with backward-compatible parameters? For start a good API would have to be defined and some basic / required / used implementations.. that could be extended with new required implementations in future but still self-compatible interface? Thanks :-) Tomek On Mon, Apr 7, 2025 at 7:22 PM Alan C. Assis <acas...@gmail.com> wrote: > > Hi Sebastien, > > Yes, maybe your suggestion to create a simple/base crc16 that will be the > base for other crc16 variants could make more sense. > > It would be nice to have something similar for crc32 as well. > > There are many variations listed in this wikipedia page: > > https://en.wikipedia.org/wiki/Cyclic_redundancy_check#Polynomial_representations > > BR, > > Alan > > On Mon, Apr 7, 2025 at 12:29 PM Sebastien Lorquet <sebast...@lorquet.fr> > wrote: > > > Hello, > > > > I understand the need for improvement but I think there is a better > > solution that I am describing at the bottom of this email after the > > rationale. > > > > I will keep my request to not break crc16 > > > > Also I explain why changing it is useless. > > > > > > -- why it is useless to change crc16 -- > > > > Again, there is NO definition for a single CRC16. There is no CRC16 > > standard algorithm. None is better than the other (as a first order > > approximation). > > > > A CRC16 has several parameters and these all must be specified to be > > sure that it's well defined. > > > > Given a polynomial, you can still change the initial value, the final > > xor, and the order of input and output bits. > > > > These parameters are also encoded in a CRC16 and they are rarely mentioned. > > > > > > So "compatibility" with any other platform is a fallacy, because there > > is no standard, the choice of a CRC is just a de facto rule of the > > strongest. > > > > So there is no point to follow linux compatibility or whatever you name > > because these are not, and will never be, recognized standard. > > > > This is not linux's fault. This is not nuttx's fault. this is CRC16 > > being badly defined. > > > > > > -- what can be done better -- > > > > The best option, to my cryptograpic experience of 15 years, is to > > provide a generic crc16 routine, and implement all variants using > > parameters. > > > > THEN, everywhere you need compatibility, you SPECIFY the crc16 details > > that must be used to ensure compatibility with this particular app. > > > > It can be made compact OR fast, that could also be a build or runtime > > option. > > > > > > This would have my support. > > > > And then, use this to continue to provide the default crc16 > > implementation that was decided a long time ago. > > > > Again, I repeat, do NOT break the default crc16. I reckon that it is > > badly defined and maybe unusual, but it's there, and changing it will > > catastrophically break user code that we are not even remotely aware. > > > > Also, I am not suggesting that basing ANY system on the default CRC > > routines of an operating system is a clean design. But it will exist > > nevertheless. so, it must be stable. > > > > > > Here is my proposal: > > > > -define a set of new configurable CRC routines that works like SHA256 > > (for example a crc16 context storing parameters, and a > > crc16_generic_init(&context, CRC_PARAMS_WHATEVER), with > > crc16_generic_update, crc16_generic_final > > > > Use this to remplement all existing crc16 you find in nuttx. > > > > Use the new API to implement any new CRC that will need proper and > > strong specified interoperable parameters > > > > This will allow NuttX to claim a clean and well defined CRC16 API. > > > > > > -- conclusion -- > > > > Any change you do to change an arbitrary CRC16 by another CRC16 will be > > > > 1 - frustrating > > > > 2 - catastrophic for long term users and unknown code > > > > 3 - not even closer to any standard > > > > > > This is my attempt to suggest a clean and better solution to the crc16 > > problem. A simple substitution, with bad words, is just replacing trash > > with another trash. Absolutely useless. > > > > There is a better way. > > > > > > Again and again, dont break the default crc16. Consider it legacy. Keep > > it. Dont break user protocols and mass storage structures. > > > > > > Sebastien > > > > > > On 07/04/2025 17:09, Alan C. Assis wrote: > > > Hi Sebastien, > > > > > > This is a case where compatibility is more important than API stability. > > > > > > New developers are spending a lot of time trying to figure out why > > existing > > > code is not working and end up discovering that the issue is the CRC > > > incompatible with Linux or other OS. > > > > > > If breaking API is an issue, we could propose these modifications to the > > > next major version (version 13). > > > > > > BR, > > > > > > Alan > > > > > > > > > On Mon, Apr 7, 2025 at 11:27 AM Sebastien Lorquet <sebast...@lorquet.fr> > > > wrote: > > > > > >> Hello, > > >> > > >> No. No way I will EVER be convinced by this change. The default CRC MUST > > >> NOT change. > > >> > > >> > > >> I have been maintaining proprietary code bases for 15 years, and I very > > >> well know this frustration, but the answer is still no. A default > > >> cryptographic algorithm can NOT change. > > >> > > >> All you can do is make sure new code uses your new routines, which are > > >> now appropriately named and form a new stable API. > > >> > > >> But PLEASE keep the API stable. > > >> > > >> > > >> (One-wire has its own crc and if you change it components will stop > > >> working thats all.) > > >> > > >> > > >> CRC used in a "live" system, like a protocol: if you change it, you cant > > >> communicate with other peers of the network > > >> > > >> CRC used in a storage system: You break the on-disk storage structure > > >> and the user looses all their storage. > > >> > > >> > > >> If you change ANY CRC across a release, it results in a catastrophic > > >> failure. > > >> > > >> Please understand: You CANT change a CRC definition if it has been used > > >> ONCE by anyone, even if you are not aware of it. > > >> > > >> And as a reminder, the visible code base is only the tip of the iceberg. > > >> > > >> This breaking change will silently break user code that is already > > >> released. > > >> > > >> > > >> So changing the default crc16 implementation NOW is too much of a > > >> responsibility. > > >> > > >> I refuse this change. > > >> > > >> > > >> As said, I am very welcoming to more CRC algorithms, given they do not > > >> replace the default. > > >> > > >> PLEASE respect this, at least by waiting for another opinion in addition > > >> to mine. > > >> > > >> You cant break protocols and storage systems you are not aware of. > > >> > > >> > > >> I am using strong wording not because I am in a bad mood, but because it > > >> is a really really important point, and I hope I get support from more > > >> project members. > > >> > > >> Again I'm very OK to add several CRC routines, or even a configurable > > >> CRC like crc_init_alg(CRC_TYPE_XMODEM). The CRC table can be generated > > >> dynamically. > > >> > > >> CRC have many different parameters > > >> > > >> -the polynomial > > >> > > >> -the initial value > > >> > > >> -the order of bits from the input > > >> > > >> -the order of bits in the CRC > > >> > > >> -the final inversion of data > > >> > > >> You cant have one crc routine for each. The general case need that much > > >> generality. > > >> > > >> I said please, several times. I BEG you, dont change the default CRC. > > >> Pretty please. > > >> > > >> Best regards, > > >> > > >> Sebastien > > >> > > >> > > >> On 07/04/2025 16:13, chao an wrote: > > >>> Hi, sebastien > > >>> > > >>> I fully understand your concerns, and I agree that changing the default > > >>> algorithm may cause trouble to currently active projects. > > >>> However, I have searched the code for the application scope of crc16 in > > >>> nuttx, and I found that many drivers are using customized CRC-16/IBM > > >>> implementations, which are not capabilities provided by the system. > > >>> > > >>> CRC-16/IBM: > > >>> > > >>> Below drivers implement need CRC-16/IBM support: > > >>> > > >>> 1. 1wire_crc private implementation: > > >>> (poly: 0x8005 (0xA001) initial seed: 0x0000, xor output: 0x0000) > > >>> > > >> > > https://github.com/apache/nuttx/blob/master/drivers/1wire/1wire_crc.c#L73-L98 > > >>> 2. spi/uart driver communicate with linux: > > >>> > > >>> > > >> > > https://github.com/apache/nuttx/blob/master/drivers/rpmsg/rpmsg_port_spi_slave.c#L45-L46 > > >> > > https://github.com/apache/nuttx/blob/master/drivers/rpmsg/rpmsg_port_spi.c#L45-L46 > > >> > > https://github.com/apache/nuttx/blob/master/drivers/rpmsg/rpmsg_port_uart.c#L59-L60 > > >>> ---------------------------------------- > > >>> CRC-16/XMODEM: > > >>> > > >>> CRC-16/XMODEM is currently only used in y/zmodem in nuttx-apps: > > >>> > > >>> > > >> > > https://github.com/apache/nuttx-apps/blob/master/system/ymodem/ymodem.c#L176 > > >> > > https://github.com/apache/nuttx-apps/blob/master/system/zmodem/zm_state.c#L204 > > >> > > https://github.com/apache/nuttx-apps/blob/master/system/zmodem/zm_proto.c#L194 > > >> > > https://github.com/apache/nuttx-apps/blob/master/system/zmodem/zm_send.c#L996 > > >>> Furthermore, we found that many communication protocols are using > > >>> CRC-16/IBM implementation. > > >>> In the email, I also explained that this is because popular operating > > >>> systems, such as Linux/OpenBSD/FreeBSD are using CRC-16/IBM by default: > > >>> > > >>> OpenBSD: > > >>> https://github.com/openbsd/src/blob/master/sys/lib/libkern/crc16.h > > >>> > > >>> FreeBSD: > > >>> https://github.com/freebsd/freebsd-src/blob/main/sys/libkern/crc16.c > > >>> > > >>> Linux: > > >>> https://github.com/torvalds/linux/blob/master/lib/crc16.c > > >>> > > >>> So I want to convince you further, I think this is better for the > > future > > >>> development of NuttX > > >>> > > >>> Sebastien Lorquet <sebast...@lorquet.fr> 于2025年4月7日周一 21:19写道： > > >>> > > >>>> Hello, > > >>>> > > >>>> Compatibility with other OSes in this domain is dubious. > > >>>> > > >>>> What is the actual need for cross-OS crc16 compatibility? > > >>>> > > >>>> Self-compatibilty is much more important. What is a non volatile > > storage > > >>>> structure was created by the old CRC (old release) is checked with the > > >>>> new CRC? > > >>>> > > >>>> This will be an immediate fatal error with potentially grave > > >> consequences. > > >>>> I am FULLY AGAINST this change. > > >>>> > > >>>> It MUST NOT PROCEED. > > >>>> > > >>>> I still think you can introduce additional routines to compute other > > CRC > > >>>> variations, but the default crc MUST DEFINITELY NOT CHANGE. > > >>>> > > >>>> Use the new routines in a wrapper in your code, but DO NOT CHANGE the > > >>>> default CRC algorithm > > >>>> > > >>>> This is an ABSOLUTELY CRITICAL ISSUE! > > >>>> > > >>>> -1 > > >>>> > > >>>> Sebastien > > >>>> > > >>>> > > >>>> On 07/04/2025 10:35, chao an wrote: > > >>>>> Hi Community, > > >>>>> > > >>>>> I plan to switch the default CRC16 algorithm directory of NuttX from > > >>>>> CRC-16/XMODEM to CRC-16/IBM: > > >>>>> https://github.com/apache/nuttx/pull/16147 > > >>>>> > > >>>>> CRC-16/XMODEM as the default implementation has significant > > >> limitations, > > >>>>> especially when communicating with popular operating systems and it > > >> comes > > >>>>> to CRC encryption verification, the algorithm needs to be switched. > > >>>>> > > >>>>> I have conducted research on POSIX-compatible operating systems, and > > >>>> almost > > >>>>> all of them use CRC-16/IBM as the default implementation: > > >>>>> > > >>>>> OpenBSD: > > >>>>> https://github.com/openbsd/src/blob/master/sys/lib/libkern/crc16.h > > >>>>> > > >>>>> FreeBSD: > > >>>>> https://github.com/freebsd/freebsd-src/blob/main/sys/libkern/crc16.c > > >>>>> > > >>>>> Linux: > > >>>>> https://github.com/torvalds/linux/blob/master/lib/crc16.c > > >>>>> > > >>>>> So I need your vote here: > > >>>>> If you prefer CRC-16/XMODEM, please reply with -1. > > >>>>> If you recommend CRC-16/IBM, please reply with +1. > > >>>>> > > >>>>> BRs, > > >>>>> > > -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
