linux-kernel - Re: [PATCH v3 2/6] scripts/gen-crc-consts: add gen-crc-consts.py

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250206234100.GA2582574@google.com>
Date: Thu, 6 Feb 2025 23:41:00 +0000
From: Eric Biggers <ebiggers@...nel.org>
To: David Laight <david.laight.linux@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
	x86@...nel.org, linux-block@...r.kernel.org,
	Ard Biesheuvel <ardb@...nel.org>, Keith Busch <kbusch@...nel.org>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	"Martin K . Petersen" <martin.petersen@...cle.com>
Subject: Re: [PATCH v3 2/6] scripts/gen-crc-consts: add gen-crc-consts.py

On Thu, Feb 06, 2025 at 10:28:53PM +0000, David Laight wrote:
> On Thu, 6 Feb 2025 12:08:43 -0800
> Eric Biggers <ebiggers@...nel.org> wrote:
> 
> > On Thu, Feb 06, 2025 at 07:31:17PM +0000, David Laight wrote:
> > > On Wed,  5 Feb 2025 23:39:44 -0800
> > > Eric Biggers <ebiggers@...nel.org> wrote:
> > >   
> > > > From: Eric Biggers <ebiggers@...gle.com>
> > > > 
> > > > Add a Python script that generates constants for computing the given CRC
> > > > variant(s) using x86's pclmulqdq or vpclmulqdq instructions.
> > > > 
> > > > This is specifically tuned for x86's crc-pclmul-template.S.  However,
> > > > other architectures with a 64x64 => 128-bit carryless multiplication
> > > > instruction should be able to use the generated constants too.  (Some
> > > > tweaks may be warranted based on the exact instructions available on
> > > > each arch, so the script may grow an arch argument in the future.)
> > > > 
> > > > The script also supports generating the tables needed for table-based
> > > > CRC computation.  Thus, it can also be used to reproduce the tables like
> > > > t10_dif_crc_table[] and crc16_table[] that are currently hardcoded in
> > > > the source with no generation script explicitly documented.
> > > > 
> > > > Python is used rather than C since it enables implementing the CRC math
> > > > in the simplest way possible, using arbitrary precision integers.  The
> > > > outputs of this script are intended to be checked into the repo, so
> > > > Python will continue to not be required to build the kernel, and the
> > > > script has been optimized for simplicity rather than performance.  
> > > 
> > > It might be better to output #defines that just contain array
> > > initialisers rather than the definition of the actual array itself.
> > > 
> > > Then any code that wants the values can include the header and
> > > just use the constant data it wants to initialise its own array.
> > > 
> > > 	David  
> > 
> > The pclmul constants use structs, not arrays.  Maybe you are asking for the
> > script to only generate the struct initializers?
> 
> I'd not read the python that closely.
> 
> > This suggestion seems a bit more complicated than just having everything in one place.
> 
> It'll be in several places anyway since the python file is only going
> to generate the lookup tables.
> 
> > It would allow
> > putting the struct definitions in the CRC-variant-specific files while keeping
> > the struct initializers all in one file, so __maybe_unused would no longer need
> > to be used on the definitions.  But the actual result would be the same, just
> > achieved in what seems like a slightly more difficult way.
> 
> It would leave the variable declarations in the file that used them - making
> it easier to see what they are.
> It also gives the option of minor changes in the variable name/attributes
> which might be useful at some point (or some architecture).
> 
> I've got some similar tables for a normal byte-lookup crc16 (hdlc).
> And for doing the hdlc bit-stuffing and flag/abort detection on
> a byte-by-byte basis
> The whole lot is 11k - quite a lot of memory inside an fpga!
> I started with the 'header' containing the initialised data, but
> later changed it to just #define for the initialiser - worked better
> that way.

Again, for now gen-crc-consts.py is only used for generating the structs of
constants used by the x86 pclmul code.  They are indeed all in one header file.
They all need the same declarations and attributes, and it is convenient to
generate them all at once, and name and document them in a consistent way.
Please take a closer look at the patchset (if you are interested), as it's not
entirely clear what you're attempting to comment on exactly.

gen-crc-consts.py does also have support for generating slice-by-N tables, which
it seems is what you may be attempting to comment on, but that is not currently
used for real.  It just seemed convenient to include, since it is just ~20 extra
lines in the script, and it is sufficient to reproduce any of the existing CRC
tables that are hardcoded in the kernel, and the table for any new CRC variant
that might get added in the future.  If we decide to start using that part "for
real", we could always tweak the exact formatting of the tables a bit if needed.

- Eric