| Message ID | 20240524-mcrc64-upstream-v3-0-24b94d8e8578@ti.com |
|---|---|
| Headers | show |
| Series | Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode | expand |
<kamlesh@ti.com> writes: > From: Kamlesh Gurudasani <kamlesh@ti.com> > > MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC) > according to the ISO 3309 standard. > Hi Herbert, Could you please review this and let me know if any changes are needed to get it merged. Thanks, Kamlesh
On Mon, Jun 10, 2024 at 08:03:44PM +0530, Kamlesh Gurudasani wrote: > <kamlesh@ti.com> writes: > > > From: Kamlesh Gurudasani <kamlesh@ti.com> > > > > MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC) > > according to the ISO 3309 standard. > > Could you please review this and let me know if any changes are needed > to get it merged. Eric Biggers had concerns about adding this to the kernel. I'd like know if he's OK with this or not. Thanks,
On Tue, Jun 11, 2024 at 10:31:45AM +0800, Herbert Xu wrote: > On Mon, Jun 10, 2024 at 08:03:44PM +0530, Kamlesh Gurudasani wrote: > > <kamlesh@ti.com> writes: > > > > > From: Kamlesh Gurudasani <kamlesh@ti.com> > > > > > > MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC) > > > according to the ISO 3309 standard. > > > > Could you please review this and let me know if any changes are needed > > to get it merged. > > Eric Biggers had concerns about adding this to the kernel. I'd > like know if he's OK with this or not. > I thought the rule is that there needs to be an in-kernel user to add algorithms to the crypto API? Is there any precedent for adding new algorithms purely so that accelerators that implement them can be accessed from userspace via AF_ALG? Even if acceptable, the motivation for this one does seem weak, given that a userspace software implementation would actually be faster. It could be marginally useful for freeing up the CPU for other tasks if the inputs being processed are very large (probably at least several megabytes), though. - Eric
On Mon, Jun 10, 2024 at 08:13:14PM -0700, Eric Biggers wrote: > > I thought the rule is that there needs to be an in-kernel user to add algorithms > to the crypto API? Is there any precedent for adding new algorithms purely so > that accelerators that implement them can be accessed from userspace via AF_ALG? I agree. Perhaps this driver could instead be added as a simple char device that is then used directly by the intended user without going through the Crypto API at all. That would make it a lot simpler. Cheers,
Herbert Xu <herbert@gondor.apana.org.au> writes: > On Mon, Jun 10, 2024 at 08:13:14PM -0700, Eric Biggers wrote: >> >> I thought the rule is that there needs to be an in-kernel user to add algorithms >> to the crypto API? Is there any precedent for adding new algorithms purely so >> that accelerators that implement them can be accessed from userspace via AF_ALG? > > I agree. Perhaps this driver could instead be added as a simple > char device that is then used directly by the intended user without > going through the Crypto API at all. > > That would make it a lot simpler. Thanks for all the support Herbert, Eric. Just wanted to confirm, if this is being rejected primarily because 1. there is no in-kernel user for crc64-iso3309 2. or poor performance benefit of using it from userspace The context for asking is that we have another superset IP known as MCRC (this one is MCRC64), which supports crc8/16/32/64(iso-3309). That IP has working DMA and will give good offloading numbers. We are planning to send drivers for crc8/16/32 for MCRC 1.should I put efforts for crc64-iso3309 as well or 2.drop the crc64-iso3309 and send only for remaining crc8/16/32(standard algorithms with already in-kernel user). All our devices either have MCRC or MCRC64. Thanks, Kamlesh
On Fri, Sep 06, 2024 at 04:44:44PM +0530, Kamlesh Gurudasani wrote: > > Just wanted to confirm, if this is being rejected primarily because > 1. there is no in-kernel user for crc64-iso3309 > 2. or poor performance benefit of using it from userspace Essentially we don't want to add every random algorithm to the crypto API because we may end up having to maintain them long after the users have disappeared. For a special-purpose algorithm like this, it's perfectly fine to have a custom driver to be made so that your user-space app can access the hardware. > The context for asking is that we have another superset IP known as MCRC > (this one is MCRC64), which supports crc8/16/32/64(iso-3309). > > That IP has working DMA and will give good offloading numbers. > > We are planning to send drivers for crc8/16/32 for MCRC > 1.should I put efforts for crc64-iso3309 as well or > 2.drop the crc64-iso3309 and send only for remaining > crc8/16/32(standard algorithms with already in-kernel user). > > All our devices either have MCRC or MCRC64. Do any existing kernel users benefit sufficiently from these algorithms being offloaded? If no then there is no need to bother. Cheers,