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,
