Re: [PATCH net-next] net/tls: Removed redundant variable from 'struct tls_sw_context_rx'

[Boris Pismenny]

Hi Vakul,

On 7/12/2018 7:03 AM, Vakul Garg wrote:
The variable 'decrypted' in 'struct tls_sw_context_rx' is redundant and
is being set/unset without purpose. Simplified the code by removing it.

AFAIU, this variable has an important use here. It keeps the state 
whether the current record has been decrypted between invocations of the 
recv/splice system calls. Otherwise, some records would be decrypted 
more than once if the entire record was not read.

Signed-off-by: Vakul Garg <vakul.g...@nxp.com>
---
  include/net/tls.h |  1 -
  net/tls/tls_sw.c  | 87 ++++++++++++++++++++++++-------------------------------
  2 files changed, 38 insertions(+), 50 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 70c273777fe9..528d0c2d6cc2 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -113,7 +113,6 @@ struct tls_sw_context_rx {
                                struct poll_table_struct *wait);
        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];
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0d670c8adf18..e5f2de2c3fd6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -81,8 +81,6 @@ static int tls_do_decryption(struct sock *sk,
        rxm->full_len -= tls_ctx->rx.overhead_size;
        tls_advance_record_sn(sk, &tls_ctx->rx);
  
-	ctx->decrypted = true;
-
        ctx->saved_data_ready(sk);
  
  out:
@@ -756,6 +754,9 @@ int tls_sw_recvmsg(struct sock *sk,
        bool cmsg = false;
        int target, err = 0;
        long timeo;
+       int page_count;
+       int to_copy;
+
  
  	flags |= nonblock;
  
@@ -792,46 +793,38 @@ int tls_sw_recvmsg(struct sock *sk,
                        goto recv_end;
                }
  
-		if (!ctx->decrypted) {
-                       int page_count;
-                       int to_copy;
-
-                       page_count = iov_iter_npages(&msg->msg_iter,
-                                                    MAX_SKB_FRAGS);
-                       to_copy = rxm->full_len - tls_ctx->rx.overhead_size;
-                       if (to_copy <= len && page_count < MAX_SKB_FRAGS &&
-                           likely(!(flags & MSG_PEEK)))  {
-                               struct scatterlist sgin[MAX_SKB_FRAGS + 1];
-                               int pages = 0;
-
-                               zc = true;
-                               sg_init_table(sgin, MAX_SKB_FRAGS + 1);
-                               sg_set_buf(&sgin[0], ctx->rx_aad_plaintext,
-                                          TLS_AAD_SPACE_SIZE);
-
-                               err = zerocopy_from_iter(sk, &msg->msg_iter,
-                                                        to_copy, &pages,
-                                                        &chunk, &sgin[1],
-                                                        MAX_SKB_FRAGS, false);
-                               if (err < 0)
-                                       goto fallback_to_reg_recv;
-
-                               err = decrypt_skb(sk, skb, sgin);
-                               for (; pages > 0; pages--)
-                                       put_page(sg_page(&sgin[pages]));
-                               if (err < 0) {
-                                       tls_err_abort(sk, EBADMSG);
-                                       goto recv_end;
-                               }
-                       } else {
+               page_count = iov_iter_npages(&msg->msg_iter, MAX_SKB_FRAGS);
+               to_copy = rxm->full_len - tls_ctx->rx.overhead_size;
+
+               if (to_copy <= len && page_count < MAX_SKB_FRAGS &&
+                   likely(!(flags & MSG_PEEK)))  {
+                       struct scatterlist sgin[MAX_SKB_FRAGS + 1];
+                       int pages = 0;
+
+                       zc = true;
+                       sg_init_table(sgin, MAX_SKB_FRAGS + 1);
+                       sg_set_buf(&sgin[0], ctx->rx_aad_plaintext,
+                                  TLS_AAD_SPACE_SIZE);
+                       err = zerocopy_from_iter(sk, &msg->msg_iter, to_copy,
+                                                &pages, &chunk, &sgin[1],
+                                                MAX_SKB_FRAGS, false);
+                       if (err < 0)
+                               goto fallback_to_reg_recv;
+
+                       err = decrypt_skb(sk, skb, sgin);
+                       for (; pages > 0; pages--)
+                               put_page(sg_page(&sgin[pages]));
+                       if (err < 0) {
+                               tls_err_abort(sk, EBADMSG);
+                               goto recv_end;
+                       }
+               } else {
  fallback_to_reg_recv:
-                               err = decrypt_skb(sk, skb, NULL);
-                               if (err < 0) {
-                                       tls_err_abort(sk, EBADMSG);
-                                       goto recv_end;
-                               }
+                       err = decrypt_skb(sk, skb, NULL);
+                       if (err < 0) {
+                               tls_err_abort(sk, EBADMSG);
+                               goto recv_end;
                        }
-                       ctx->decrypted = true;
                }
  
  		if (!zc) {
@@ -895,15 +888,13 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t 
*ppos,
                goto splice_read_end;
        }
  
-	if (!ctx->decrypted) {
-               err = decrypt_skb(sk, skb, NULL);
+       err = decrypt_skb(sk, skb, NULL);
  
-		if (err < 0) {
-                       tls_err_abort(sk, EBADMSG);
-                       goto splice_read_end;
-               }
-               ctx->decrypted = true;
+       if (err < 0) {
+               tls_err_abort(sk, EBADMSG);
+               goto splice_read_end;
        }
+
        rxm = strp_msg(skb);
  
  	chunk = min_t(unsigned int, rxm->full_len, len);
@@ -998,8 +989,6 @@ static void tls_queue(struct strparser *strp, struct 
sk_buff *skb)
        struct tls_context *tls_ctx = tls_get_ctx(strp->sk);
        struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
  
-	ctx->decrypted = false;
-
        ctx->recv_pkt = skb;
        strp_pause(strp);