Re: [PATCH v10 crypto 09/11] chtls: Inline TLS request Tx/Rx
flat view
From: Stefano Brivio <hidden>
Date: 2018-03-12 13:28:33
On Sat, 10 Mar 2018 00:40:14 +0530 Atul Gupta [off-list ref] wrote:
quoted hunk ↗ jump to hunk
TLS handler for record transmit and receive. Create Inline TLS work request and post to FW. Create Inline TLS record CPLs for hardware Signed-off-by: Atul Gupta <redacted> --- drivers/crypto/chelsio/chtls/chtls_io.c | 1863 +++++++++++++++++++++++++++++++ 1 file changed, 1863 insertions(+) create mode 100644 drivers/crypto/chelsio/chtls/chtls_io.cdiff --git a/drivers/crypto/chelsio/chtls/chtls_io.c b/drivers/crypto/chelsio/chtls/chtls_io.c new file mode 100644 index 0000000..f7f5826 --- /dev/null +++ b/drivers/crypto/chelsio/chtls/chtls_io.c@@ -0,0 +1,1863 @@ +/* + * Copyright (c) 2017 Chelsio Communications, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Written by: Atul Gupta (atul.gupta@chelsio.com) + */ + +#include <linux/module.h> +#include <linux/list.h> +#include <linux/workqueue.h> +#include <linux/skbuff.h> +#include <linux/timer.h> +#include <linux/notifier.h> +#include <linux/inetdevice.h> +#include <linux/ip.h> +#include <linux/tcp.h> +#include <linux/sched/signal.h> +#include <net/tcp.h> +#include <net/busy_poll.h> +#include <crypto/aes.h> + +#include "chtls.h" +#include "chtls_cm.h" + +static bool is_tls_hw(struct chtls_sock *csk) +{ + return csk->tlshws.ofld; +}
Do you actually need this function?
quoted hunk ↗ jump to hunk
+static bool is_tls_rx(struct chtls_sock *csk) +{ + return (csk->tlshws.rxkey >= 0); +}
You can drop the ().
quoted hunk ↗ jump to hunk
+ +static bool is_tls_tx(struct chtls_sock *csk) +{ + return (csk->tlshws.txkey >= 0); +}
You can drop the ().
quoted hunk ↗ jump to hunk
+static bool is_tls_skb(struct chtls_sock *csk, const struct sk_buff *skb) +{ + return (is_tls_hw(csk) && skb_ulp_tls_skb_flags(skb)); +}
You can drop the ().
quoted hunk ↗ jump to hunk
+static int key_size(void *sk) +{ + return 16; /* Key on DDR */ +}
Do you actually need this function? Can't you turn that into a #define?
quoted hunk ↗ jump to hunk
+ +#define ceil(x, y) \ + ({ unsigned long __x = (x), __y = (y); (__x + __y - 1) / __y; })
This doesn't look like a ceiling function, could you perhaps name it more descriptively?
quoted hunk ↗ jump to hunk
+static int data_sgl_len(const struct sk_buff *skb) +{ + unsigned int cnt; + + cnt = skb_shinfo(skb)->nr_frags; + return (sgl_len(cnt) * 8);
You can drop the ().
quoted hunk ↗ jump to hunk
+} + +static int nos_ivs(struct sock *sk, unsigned int size) +{ + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); + + return ceil(size, csk->tlshws.mfs); +} + +#define TLS_WR_CPL_LEN \ + (sizeof(struct fw_tlstx_data_wr) + \ + sizeof(struct cpl_tx_tls_sfo)) + +static int is_ivs_imm(struct sock *sk, const struct sk_buff *skb)
Shouldn't this be a bool?
quoted hunk ↗ jump to hunk
+{ + int ivs_size = nos_ivs(sk, skb->len) * CIPHER_BLOCK_SIZE; + int hlen = TLS_WR_CPL_LEN + data_sgl_len(skb); + + if ((hlen + key_size(sk) + ivs_size) < + MAX_IMM_OFLD_TX_DATA_WR_LEN) { + ULP_SKB_CB(skb)->ulp.tls.iv = 1; + return 1; + } + ULP_SKB_CB(skb)->ulp.tls.iv = 0;
Are these assignments intended? They don't seem to fit with the name of the function.
quoted hunk ↗ jump to hunk
+ return 0; +} + +static int max_ivs_size(struct sock *sk, int size) +{ + return (nos_ivs(sk, size) * CIPHER_BLOCK_SIZE); +}
You can drop the ().
quoted hunk ↗ jump to hunk
+ +static int ivs_size(struct sock *sk, const struct sk_buff *skb) +{ + return (is_ivs_imm(sk, skb) ? (nos_ivs(sk, skb->len) * + CIPHER_BLOCK_SIZE) : 0); +}
You can drop the ().
[...]
+static u64 tls_sequence_number(struct chtls_hws *hws)
+{
+ return (hws->tx_seq_no++);
+}You can drop the (), and I find the name of this function a bit misleading as you are actually increasing the sequence number, not just returning it.
quoted hunk ↗ jump to hunk
+ +static int is_sg_request(const struct sk_buff *skb) +{ + return (skb->peeked || + (skb->len > MAX_IMM_ULPTX_WR_LEN)); +}
You can drop the (). This should be a bool.
quoted hunk ↗ jump to hunk
+ +/* + * Returns true if an sk_buff carries urgent data. + */ +static int skb_urgent(struct sk_buff *skb) +{ + return (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_URG) != 0; +}
This should be a bool, you can drop the (), and avoid that != 0 comparison. -- Stefano