Re: [PATCH crypto-next v2 1/3] crypto: poly1305 - add new 32 and 64-bit generic versions

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Dec 12, 2019 at 04:35:04PM +0100, Jason A. Donenfeld wrote:
> On Thu, Dec 12, 2019 at 4:30 PM Martin Willi <martin@strongswan.org> wrote:
> > > The principle advantage of this patchset is the 64x64 code
> >
> > If there are platforms / code paths where this code matters, all fine.
> 
> It does matter.
> 
> >
> > But the 64-bit version adds a lot of complexity because of the
> > different state representation and the conversion between these states.
> > I just don't think the gain (?) justifies that added complexity.
> 
> No, there's no conversion between the state representation, or any
> complexity like that added.
> 
> I think if anything, the way this patch works, we wind up with
> something easier to audit and look at. You can examine
> poly1305-donna32.c and poly1305-donna64.c side-by-side and compare
> line-by-line, as clean and isolate implementations. And this is very
> well-known code too.

It's inherently more complex to have multiple alternate implementations, and it
reduces testability because there's no obvious way to even test your 32-bit
version on x86_64 (which most developers use), as it seems your 64-bit version
always gets built instead.

Now, it's possible that the performance gain outweighs this, and I too would
like to have the C implementation of Poly1305 be faster.  So if you'd like to
argue for the performance gain, fine, and if there's a significant performance
gain I don't have an objection.  But I'm not sure why you're at the same time
trying to argue that *adding* an extra implementation somehow makes the code
easier to audit and doesn't add complexity...

- Eric