RE: [PATCH net-next v2 1/1] net/tls: Combined memory allocation for decryption request

2018-08-10 Thread Vakul Garg


> -Original Message-
> From: Dave Watson [mailto:davejwat...@fb.com]
> Sent: Thursday, August 9, 2018 9:56 PM
> To: Vakul Garg 
> Cc: netdev@vger.kernel.org; bor...@mellanox.com;
> avia...@mellanox.com; da...@davemloft.net
> Subject: Re: [PATCH net-next v2 1/1] net/tls: Combined memory allocation
> for decryption request
> 
> On 08/09/18 04:56 AM, Vakul Garg wrote:
> > For preparing decryption request, several memory chunks are required
> > (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request
> > to an accelerator, it is required that the buffers which are read by
> > the accelerator must be dma-able and not come from stack. The buffers
> > for aad and iv can be separately kmalloced each, but it is inefficient.
> > This patch does a combined allocation for preparing decryption request
> > and then segments into aead_req || sgin || sgout || iv || aad.
> >
> > Signed-off-by: Vakul Garg 
> 
> Reviewed-by: Dave Watson 
> 
> Thanks, looks good to me now.

I had to rebase the patch so as not to use any other patch which is under 
review.
Since Doron's skb_nsg() patch needs rework, I submitted v3 after removing 
reference to function skb_nsg() and 
replaced it with skb_cow_data().

Kindly review.
 



Re: [PATCH net-next v2 1/1] net/tls: Combined memory allocation for decryption request

2018-08-09 Thread David Miller
From: Vakul Garg 
Date: Thu,  9 Aug 2018 04:56:23 +0530

> For preparing decryption request, several memory chunks are required
> (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
> an accelerator, it is required that the buffers which are read by the
> accelerator must be dma-able and not come from stack. The buffers for
> aad and iv can be separately kmalloced each, but it is inefficient.
> This patch does a combined allocation for preparing decryption request
> and then segments into aead_req || sgin || sgout || iv || aad.
> 
> Signed-off-by: Vakul Garg 
> ---
> This patch needs to be applied over Doron Roberts-Kedes's patch.
>   net/tls: Calculate nsg for zerocopy path without skb_cow_data.

That's going to have many changes, I gave feedback on it yesterday.

Please do not post patches which have pre-requisites which are in
the process of changing or similar as that makes a lot more work
for me and you are also asking people to review changes on top
of code which is going to change.

Thanks.


Re: [PATCH net-next v2 1/1] net/tls: Combined memory allocation for decryption request

2018-08-09 Thread Dave Watson
On 08/09/18 04:56 AM, Vakul Garg wrote:
> For preparing decryption request, several memory chunks are required
> (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
> an accelerator, it is required that the buffers which are read by the
> accelerator must be dma-able and not come from stack. The buffers for
> aad and iv can be separately kmalloced each, but it is inefficient.
> This patch does a combined allocation for preparing decryption request
> and then segments into aead_req || sgin || sgout || iv || aad.
> 
> Signed-off-by: Vakul Garg 

Reviewed-by: Dave Watson 

Thanks, looks good to me now.


[PATCH net-next v2 1/1] net/tls: Combined memory allocation for decryption request

2018-08-08 Thread Vakul Garg
For preparing decryption request, several memory chunks are required
(aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
an accelerator, it is required that the buffers which are read by the
accelerator must be dma-able and not come from stack. The buffers for
aad and iv can be separately kmalloced each, but it is inefficient.
This patch does a combined allocation for preparing decryption request
and then segments into aead_req || sgin || sgout || iv || aad.

Signed-off-by: Vakul Garg 
---
This patch needs to be applied over Doron Roberts-Kedes's patch.
net/tls: Calculate nsg for zerocopy path without skb_cow_data.

Changes since patch v1:
- Removed unused label 'no_zerocopy'


 include/net/tls.h |   4 -
 net/tls/tls_sw.c  | 257 ++
 2 files changed, 145 insertions(+), 116 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index d8b3b6578c01..d5c683e8bb22 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -124,10 +124,6 @@ struct tls_sw_context_rx {
struct sk_buff *recv_pkt;
u8 control;
bool decrypted;
-
-   char rx_aad_ciphertext[TLS_AAD_SPACE_SIZE];
-   char rx_aad_plaintext[TLS_AAD_SPACE_SIZE];
-
 };
 
 struct tls_record_info {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 55eb04415bb3..13e365011347 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -122,19 +122,13 @@ static int tls_do_decryption(struct sock *sk,
 struct scatterlist *sgout,
 char *iv_recv,
 size_t data_len,
-struct sk_buff *skb,
-gfp_t flags)
+struct aead_request *aead_req)
 {
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
-   struct aead_request *aead_req;
-
int ret;
 
-   aead_req = aead_request_alloc(ctx->aead_recv, flags);
-   if (!aead_req)
-   return -ENOMEM;
-
+   aead_request_set_tfm(aead_req, ctx->aead_recv);
aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
aead_request_set_crypt(aead_req, sgin, sgout,
   data_len + tls_ctx->rx.tag_size,
@@ -143,8 +137,6 @@ static int tls_do_decryption(struct sock *sk,
  crypto_req_done, >async_wait);
 
ret = crypto_wait_req(crypto_aead_decrypt(aead_req), >async_wait);
-
-   aead_request_free(aead_req);
return ret;
 }
 
@@ -731,8 +723,135 @@ static struct sk_buff *tls_wait_data(struct sock *sk, int 
flags,
return skb;
 }
 
+/* This function decrypts the input skb into either out_iov or in out_sg
+ * or in skb buffers itself. The input parameter 'zc' indicates if
+ * zero-copy mode needs to be tried or not. With zero-copy mode, either
+ * out_iov or out_sg must be non-NULL. In case both out_iov and out_sg are
+ * NULL, then the decryption happens inside skb buffers itself, i.e.
+ * zero-copy gets disabled and 'zc' is updated.
+ */
+
+static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
+   struct iov_iter *out_iov,
+   struct scatterlist *out_sg,
+   int *chunk, bool *zc)
+{
+   struct tls_context *tls_ctx = tls_get_ctx(sk);
+   struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
+   struct strp_msg *rxm = strp_msg(skb);
+   int n_sgin, n_sgout, nsg, mem_size, aead_size, err, pages = 0;
+   struct aead_request *aead_req;
+   struct sk_buff *unused;
+   u8 *aad, *iv, *mem = NULL;
+   struct scatterlist *sgin = NULL;
+   struct scatterlist *sgout = NULL;
+   const int data_len = rxm->full_len - tls_ctx->rx.overhead_size;
+
+   if (*zc && (out_iov || out_sg)) {
+   if (out_iov)
+   n_sgout = iov_iter_npages(out_iov, INT_MAX) + 1;
+   else
+   n_sgout = sg_nents(out_sg);
+
+   n_sgin = skb_nsg(skb, rxm->offset + tls_ctx->rx.prepend_size,
+rxm->full_len - tls_ctx->rx.prepend_size);
+   } else {
+   n_sgout = 0;
+   *zc = false;
+   n_sgin = skb_cow_data(skb, 0, );
+   }
+
+   if (n_sgin < 1)
+   return -EBADMSG;
+
+   /* Increment to accommodate AAD */
+   n_sgin = n_sgin + 1;
+
+   nsg = n_sgin + n_sgout;
+
+   aead_size = sizeof(*aead_req) + crypto_aead_reqsize(ctx->aead_recv);
+   mem_size = aead_size + (nsg * sizeof(struct scatterlist));
+   mem_size = mem_size + TLS_AAD_SPACE_SIZE;
+   mem_size = mem_size + crypto_aead_ivsize(ctx->aead_recv);
+
+   /* Allocate a single block of memory which contains
+* aead_req || sgin[] || sgout[] || aad || iv.
+* This order achieves correct alignment for aead_req, sgin, sgout.
+*/
+