Re: [PATCH bpf-next 4/8] tls: convert to generic sk_msg interface

2018-10-12 Thread Daniel Borkmann 
On 10/12/2018 10:16 PM, Dave Watson wrote:
> On 10/11/18 02:45 AM, Daniel Borkmann wrote:
>> Convert kTLS over to make use of sk_msg interface for plaintext and
>> encrypted scattergather data, so it reuses all the sk_msg helpers
>> and data structure which later on in a second step enables to glue
>> this to BPF.
> 
> Looks very clean, thanks!

Thanks, it indeed allows for getting rid of quite a bit of open coded
code by converting to sk_msg API. As it was mentioned in the other mail,
we'd also be able to reuse this framework in future for other potential
additions or ULPs aside from that; consolidating sockmap and tls to work
on the same data structure also helped a lot in testing.

>> -static int zerocopy_from_iter(struct sock *sk, struct iov_iter *from,
>> -  int length, int *pages_used,
>> -  unsigned int *size_used,
>> -  struct scatterlist *to, int to_max_pages,
>> -  bool charge)
>> -{
> 
> ...
> 
>> -err = zerocopy_from_iter(sk, out_iov, data_len, ,
>> - chunk, [1],
>> - (n_sgout - 1), false);
>> +err = tls_setup_from_iter(sk, out_iov, data_len,
>> +  , chunk, [1],
>> +  (n_sgout - 1));
> 
> Any reason not to add the 'bool charge' to sk_msg_zerocopy_from_iter?
> Then tls_setup_from_iter is not necessary.

I left this bit aside for now by leaving the tls_setup_from_iter() as is,
basically as current zerocopy_from_iter() in current tls code minus the
charge since not used here. Given this is only triggered in RX path (which
is not sk_msg based right now) I didn't want to wrap it into a fake/temp
sk_msg object just for calling into sk_msg_zerocopy_from_iter(), felt a bit
unclean and given the complexity we already have probably more appropriate
to pursue in a second step.

Thanks,
Daniel

Re: [PATCH bpf-next 4/8] tls: convert to generic sk_msg interface

2018-10-12 Thread Dave Watson 
On 10/11/18 02:45 AM, Daniel Borkmann wrote:
> Convert kTLS over to make use of sk_msg interface for plaintext and
> encrypted scattergather data, so it reuses all the sk_msg helpers
> and data structure which later on in a second step enables to glue
> this to BPF.

Looks very clean, thanks!

> 
> -static int zerocopy_from_iter(struct sock *sk, struct iov_iter *from,
> -   int length, int *pages_used,
> -   unsigned int *size_used,
> -   struct scatterlist *to, int to_max_pages,
> -   bool charge)
> -{

...

> - err = zerocopy_from_iter(sk, out_iov, data_len, ,
> -  chunk, [1],
> -  (n_sgout - 1), false);
> + err = tls_setup_from_iter(sk, out_iov, data_len,
> +   , chunk, [1],
> +   (n_sgout - 1));

Any reason not to add the 'bool charge' to sk_msg_zerocopy_from_iter?
Then tls_setup_from_iter is not necessary.

[PATCH bpf-next 4/8] tls: convert to generic sk_msg interface

2018-10-10 Thread Daniel Borkmann 
Convert kTLS over to make use of sk_msg interface for plaintext and
encrypted scattergather data, so it reuses all the sk_msg helpers
and data structure which later on in a second step enables to glue
this to BPF.

Joint work with John.

Signed-off-by: Daniel Borkmann 
Signed-off-by: John Fastabend 
---
 include/linux/skmsg.h |   2 +
 include/net/sock.h|   4 -
 include/net/tls.h |  18 +-
 net/core/skmsg.c  |  39 
 net/core/sock.c   |  61 --
 net/tls/Kconfig   |   1 +
 net/tls/tls_device.c  |   2 +-
 net/tls/tls_sw.c  | 511 ++
 8 files changed, 236 insertions(+), 402 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 9567810..5950ef1 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -102,6 +102,8 @@ struct sk_psock {
 
 int sk_msg_alloc(struct sock *sk, struct sk_msg *msg, int len,
 int elem_first_coalesce);
+int sk_msg_clone(struct sock *sk, struct sk_msg *dst, struct sk_msg *src,
+ u32 off, u32 len);
 void sk_msg_trim(struct sock *sk, struct sk_msg *msg, int len);
 int sk_msg_free(struct sock *sk, struct sk_msg *msg);
 int sk_msg_free_nocharge(struct sock *sk, struct sk_msg *msg);
diff --git a/include/net/sock.h b/include/net/sock.h
index 751549a..7470c45 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2214,10 +2214,6 @@ static inline struct page_frag *sk_page_frag(struct sock 
*sk)
 
 bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag);
 
-int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
-   int sg_start, int *sg_curr, unsigned int *sg_size,
-   int first_coalesce);
-
 /*
  * Default write policy as shown to user space via poll/select/SIGIO
  */
diff --git a/include/net/tls.h b/include/net/tls.h
index 5e85383..3d22d8a 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -39,6 +39,8 @@
 #include 
 #include 
 #include 
+#include 
+
 #include 
 #include 
 #include 
@@ -103,15 +105,13 @@ struct tls_rec {
int tx_flags;
int inplace_crypto;
 
-   /* AAD | sg_plaintext_data | sg_tag */
-   struct scatterlist sg_plaintext_data[MAX_SKB_FRAGS + 1];
-   /* AAD | sg_encrypted_data (data contain overhead for hdr) */
-   struct scatterlist sg_encrypted_data[MAX_SKB_FRAGS + 1];
+   struct sk_msg msg_plaintext;
+   struct sk_msg msg_encrypted;
 
-   unsigned int sg_plaintext_size;
-   unsigned int sg_encrypted_size;
-   int sg_plaintext_num_elem;
-   int sg_encrypted_num_elem;
+   /* AAD | msg_plaintext.sg.data | sg_tag */
+   struct scatterlist sg_aead_in[2];
+   /* AAD | msg_encrypted.sg.data (data contains overhead for hdr & iv & 
tag) */
+   struct scatterlist sg_aead_out[2];
 
char aad_space[TLS_AAD_SPACE_SIZE];
struct aead_request aead_req;
@@ -223,8 +223,8 @@ struct tls_context {
 
unsigned long flags;
bool in_tcp_sendpages;
+   bool pending_open_record_frags;
 
-   u16 pending_open_record_frags;
int (*push_pending_record)(struct sock *sk, int flags);
 
void (*sk_write_space)(struct sock *sk);
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index d9e67da..d7c5501 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -73,6 +73,45 @@ int sk_msg_alloc(struct sock *sk, struct sk_msg *msg, int 
len,
 }
 EXPORT_SYMBOL_GPL(sk_msg_alloc);
 
+int sk_msg_clone(struct sock *sk, struct sk_msg *dst, struct sk_msg *src,
+u32 off, u32 len)
+{
+   int i = src->sg.start;
+   struct scatterlist *sge = sk_msg_elem(src, i);
+   u32 sge_len, sge_off;
+
+   if (sk_msg_full(dst))
+   return -ENOSPC;
+
+   while (off) {
+   if (sge->length > off)
+   break;
+   off -= sge->length;
+   sk_msg_iter_var_next(i);
+   if (i == src->sg.end && off)
+   return -ENOSPC;
+   sge = sk_msg_elem(src, i);
+   }
+
+   while (len) {
+   sge_len = sge->length - off;
+   sge_off = sge->offset + off;
+   if (sge_len > len)
+   sge_len = len;
+   off = 0;
+   len -= sge_len;
+   sk_msg_page_add(dst, sg_page(sge), sge_len, sge_off);
+   sk_mem_charge(sk, sge_len);
+   sk_msg_iter_var_next(i);
+   if (i == src->sg.end && len)
+   return -ENOSPC;
+   sge = sk_msg_elem(src, i);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(sk_msg_clone);
+
 void sk_msg_return_zero(struct sock *sk, struct sk_msg *msg, int bytes)
 {
int i = msg->sg.start;
diff --git a/net/core/sock.c b/net/core/sock.c
index 7e8796a..52e4f1c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2238,67 +2238,6 @@ bool sk_page_frag_refill(struct sock *sk, struct 
page_frag *pfrag)
 }