Re: [PATCH v2 0/4] crypto: aesni - Use zero-copy for gcm(aes) buffers that are partially contiguous

[Steffen Klassert]

On Wed, Jan 31, 2018 at 10:54:57AM -0800, Junaid Shahid wrote:
> On Wed, Jan 31, 2018 at 12:13 AM, Steffen Klassert
>  wrote:
> >
> > I wonder which special usecase you have in mind that will be improved
> > by your patches.
> >
> 
> This is not related to IPsec. We have an internal use case where the
> data buffer itself is a single memory page but the authentication tag
> needs to be in a separate buffer. This patch set allows us to have
> zero copy in that case.

We usually don't add code for 'internal use cases' to the mainline
kernel. If you need a SG version of gcm-aesni, then please implement
the generic version so that everybody can benefit from it.

Think about how this will look like if everybody would add
a workaround for its very special use case.

Re: [PATCH v2 0/4] crypto: aesni - Use zero-copy for gcm(aes) buffers that are partially contiguous

[Junaid Shahid]

Hi Steffen,

On Wed, Jan 31, 2018 at 12:13 AM, Steffen Klassert
 wrote:
>
> I wonder which special usecase you have in mind that will be improved
> by your patches.
>

This is not related to IPsec. We have an internal use case where the
data buffer itself is a single memory page but the authentication tag
needs to be in a separate buffer. This patch set allows us to have
zero copy in that case.

>
> Maybe it would be better to investigate in the direction to
> support SG operations with gcm-aesni instead of trying to
> find all corner cases where the existing implementation can
> do zero-copy.
>

Yes, if gcm-aesni could be implemented to handle arbitrarily
fragmented buffers, that would be better, but it would likely be a
much bigger and more complicated change. Just handling the case where
the AAD/data/authtag are separate contiguous buffers is relatively
simpler but still useful.

Thanks,
Junaid

Re: [PATCH v2 0/4] crypto: aesni - Use zero-copy for gcm(aes) buffers that are partially contiguous

[Steffen Klassert]

Hi Junaid.

On Tue, Jan 23, 2018 at 12:19:12PM -0800, Junaid Shahid wrote:
> Changes in v2:
> - Integrated https://patchwork.kernel.org/patch/10173981
> 
> Currently, the AESNI gcm(aes) implementation uses zero-copy only when the
> entire src and dest request buffers, including the AAD, the data and the
> Auth Tag are contiguous. This series enables the use of zero-copy even if the
> AAD and/or Auth Tag are in different buffers than the actual data, as long as
> each of them individually satisfies the zero-copy conditions (i.e. the entire
> buffer is either in low-mem or within a single high-mem page). Furthermore,
> it also enables the use of zero-copy even if only one of src and dest 
> satisfies
> these conditions rather than only when both of them do.

I wonder which special usecase you have in mind that will be improved
by your patches.

For IPsec, the crypto layer either gets a contiguous buffer because
networking already linearized the packet data, or the packet data
and IPsec trailer are scattered over multiple page fragments.
So in the first case, it is OK already without your patches. In
the second case, your zero-copy conditions are not satisfied.

This looks a bit like a workaround of the real problem, that is
that the gcm-aesni implementation does not support SG operations.

Maybe it would be better to investigate in the direction to
support SG operations with gcm-aesni instead of trying to
find all corner cases where the existing implementation can
do zero-copy.

[PATCH v2 0/4] crypto: aesni - Use zero-copy for gcm(aes) buffers that are partially contiguous

[Junaid Shahid]

Changes in v2:
- Integrated https://patchwork.kernel.org/patch/10173981

Currently, the AESNI gcm(aes) implementation uses zero-copy only when the
entire src and dest request buffers, including the AAD, the data and the
Auth Tag are contiguous. This series enables the use of zero-copy even if the
AAD and/or Auth Tag are in different buffers than the actual data, as long as
each of them individually satisfies the zero-copy conditions (i.e. the entire
buffer is either in low-mem or within a single high-mem page). Furthermore,
it also enables the use of zero-copy even if only one of src and dest satisfies
these conditions rather than only when both of them do.

Junaid Shahid (4):
  crypto: aesni - Fix out-of-bounds access of the AAD buffer in AVX
gcm-aesni
  crypto: aesni - Enable one-sided zero copy for gcm(aes) request
buffers
  crypto: aesni - Directly use kmap_atomic instead of scatter_walk
object in gcm(aes)
  crypto: aesni - Use zero-copy for gcm(aes) even if the
AAD/Data/AuthTag are separate

 arch/x86/crypto/aesni-intel_avx-x86_64.S | 154 +---
 arch/x86/crypto/aesni-intel_glue.c   | 307 +++
 2 files changed, 227 insertions(+), 234 deletions(-)

-- 
2.16.0.rc1.238.g530d649a79-goog