| Message ID | 20241221091056.282098-27-ebiggers@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: scatterlist handling improvements | expand |
On Sat, 21 Dec 2024 01:10:53 -0800 Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Replace calls to the deprecated function scatterwalk_copychunks() with > memcpy_from_scatterwalk(), memcpy_to_scatterwalk(), or > scatterwalk_skip() as appropriate. > > The new functions behave more as expected and eliminate the need to call > scatterwalk_done() or scatterwalk_pagedone(). This was not always being > done when needed, and therefore the old code appears to have also had a > bug where the dcache of the destination page(s) was not always being > flushed on architectures that need that. > > Cc: Boris Pismenny <borisp@nvidia.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: John Fastabend <john.fastabend@gmail.com> > Cc: netdev@vger.kernel.org > Signed-off-by: Eric Biggers <ebiggers@google.com> Acked-by: Jakub Kicinski <kuba@kernel.org>
On Mon, Dec 23, 2024 at 07:48:25AM -0800, Jakub Kicinski wrote: > On Sat, 21 Dec 2024 01:10:53 -0800 Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > Replace calls to the deprecated function scatterwalk_copychunks() with > > memcpy_from_scatterwalk(), memcpy_to_scatterwalk(), or > > scatterwalk_skip() as appropriate. > > > > The new functions behave more as expected and eliminate the need to call > > scatterwalk_done() or scatterwalk_pagedone(). This was not always being > > done when needed, and therefore the old code appears to have also had a > > bug where the dcache of the destination page(s) was not always being > > flushed on architectures that need that. > > > > Cc: Boris Pismenny <borisp@nvidia.com> > > Cc: Jakub Kicinski <kuba@kernel.org> > > Cc: John Fastabend <john.fastabend@gmail.com> > > Cc: netdev@vger.kernel.org > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > Acked-by: Jakub Kicinski <kuba@kernel.org> Thanks. FYI I will need to update this patch in the next version, as I did not take into consideration what chain_to_walk() is doing. This code seems to be a bit unique in how it is using the scatterwalk functions. Also I think the second paragraph of my commit message is wrong, as the calls to scatterwalk_done() in tls_enc_records() are the ones I thought were missing. - Eric
On Mon, 23 Dec 2024 11:42:49 -0800 Eric Biggers wrote: > > Acked-by: Jakub Kicinski <kuba@kernel.org> > > Thanks. FYI I will need to update this patch in the next version, as I did not > take into consideration what chain_to_walk() is doing. This code seems to be a > bit unique in how it is using the scatterwalk functions. > > Also I think the second paragraph of my commit message is wrong, as the calls to > scatterwalk_done() in tls_enc_records() are the ones I thought were missing. netdev@ only got CCed on this one patch so TBH I have trusted the conversion :( FWIW tls has a relatively solid selftest: tools/testing/selftests/net/tls.c
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c index f9e3d3d90dcf..ec7017c80b6a 100644 --- a/net/tls/tls_device_fallback.c +++ b/net/tls/tls_device_fallback.c @@ -67,20 +67,17 @@ static int tls_enc_record(struct aead_request *aead_req, DEBUG_NET_WARN_ON_ONCE(!cipher_desc || !cipher_desc->offloadable); buf_size = TLS_HEADER_SIZE + cipher_desc->iv; len = min_t(int, *in_len, buf_size); - scatterwalk_copychunks(buf, in, len, 0); - scatterwalk_copychunks(buf, out, len, 1); + memcpy_from_scatterwalk(buf, in, len); + memcpy_to_scatterwalk(out, buf, len); *in_len -= len; if (!*in_len) return 0; - scatterwalk_pagedone(in, 0, 1); - scatterwalk_pagedone(out, 1, 1); - len = buf[4] | (buf[3] << 8); len -= cipher_desc->iv; tls_make_aad(aad, len - cipher_desc->tag, (char *)&rcd_sn, buf[0], prot); @@ -108,14 +105,12 @@ static int tls_enc_record(struct aead_request *aead_req, *in_len = 0; } if (*in_len) { - scatterwalk_copychunks(NULL, in, len, 2); - scatterwalk_pagedone(in, 0, 1); - scatterwalk_copychunks(NULL, out, len, 2); - scatterwalk_pagedone(out, 1, 1); + scatterwalk_skip(in, len); + scatterwalk_skip(out, len); } len -= cipher_desc->tag; aead_request_set_crypt(aead_req, sg_in, sg_out, len, iv); @@ -160,13 +155,10 @@ static int tls_enc_records(struct aead_request *aead_req, cpu_to_be64(rcd_sn), &in, &out, &len, prot); rcd_sn++; } while (rc == 0 && len); - scatterwalk_done(&in, 0, 0); - scatterwalk_done(&out, 1, 0); - return rc; } /* Can't use icsk->icsk_af_ops->send_check here because the ip addresses * might have been changed by NAT.