From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:43256 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751252AbeCLNaV (ORCPT ); Mon, 12 Mar 2018 09:30:21 -0400 Date: Mon, 12 Mar 2018 14:30:14 +0100 From: Stefano Brivio To: Atul Gupta Cc: davejwatson@fb.com, davem@davemloft.net, herbert@gondor.apana.org.au, sd@queasysnail.net, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, ganeshgr@chelsio.com Subject: Re: [PATCH v10 crypto 07/11] chtls: Program the TLS Key Message-ID: <20180312143014.2cb8a4be@epycfail> In-Reply-To: <1520622616-6557-6-git-send-email-atul.gupta@chelsio.com> References: <1520622616-6557-1-git-send-email-atul.gupta@chelsio.com> <1520622616-6557-6-git-send-email-atul.gupta@chelsio.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 10 Mar 2018 00:40:12 +0530 Atul Gupta wrote: > Initialize the space reserved for storing the TLS keys > get and free the location where key is stored for the TLS > connection > Program the tx and rx key as received from user in > struct tls12_crypto_info_aes_gcm_128 and understood by hardware. > > Signed-off-by: Atul Gupta > --- > drivers/crypto/chelsio/chtls/chtls_hw.c | 371 ++++++++++++++++++++++++++++++++ > 1 file changed, 371 insertions(+) > create mode 100644 drivers/crypto/chelsio/chtls/chtls_hw.c > > diff --git a/drivers/crypto/chelsio/chtls/chtls_hw.c b/drivers/crypto/chelsio/chtls/chtls_hw.c > new file mode 100644 > index 0000000..40cf84f > --- /dev/null > +++ b/drivers/crypto/chelsio/chtls/chtls_hw.c > @@ -0,0 +1,371 @@ > +/* > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "chtls.h" > +#include "chtls_cm.h" > + > +static void __set_tcb_field_direct(struct chtls_sock *csk, > + struct cpl_set_tcb_field *req, u16 word, > + u64 mask, u64 val, u8 cookie, int no_reply) > +{ > + struct ulptx_idata *sc; > + > + INIT_TP_WR_CPL(req, CPL_SET_TCB_FIELD, csk->tid); > + req->wr.wr_mid |= htonl(FW_WR_FLOWID_V(csk->tid)); > + req->reply_ctrl = htons(NO_REPLY_V(no_reply) | > + QUEUENO_V(csk->rss_qid)); > + req->word_cookie = htons(TCB_WORD_V(word) | TCB_COOKIE_V(cookie)); > + req->mask = cpu_to_be64(mask); > + req->val = cpu_to_be64(val); > + sc = (struct ulptx_idata *)(req + 1); > + sc->cmd_more = htonl(ULPTX_CMD_V(ULP_TX_SC_NOOP)); > + sc->len = htonl(0); > +} > + > +static void __set_tcb_field(struct sock *sk, struct sk_buff *skb, u16 word, > + u64 mask, u64 val, u8 cookie, int no_reply) > +{ > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + > + req = (struct cpl_set_tcb_field *)__skb_put(skb, wrlen); > + __set_tcb_field_direct(csk, req, word, mask, val, cookie, no_reply); > + set_wr_txq(skb, CPL_PRIORITY_CONTROL, csk->port_id); > +} > + > +/* > + * Send control message to HW, message go as immediate data and packet > + * is freed immediately. > + */ > +static void chtls_set_tcb_field(struct sock *sk, u16 word, u64 mask, u64 val) > +{ > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + struct sk_buff *skb; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + unsigned int credits_needed = DIV_ROUND_UP(wrlen, 16); > + > + skb = alloc_skb(wrlen, GFP_ATOMIC); > + if (!skb) > + return; > + > + __set_tcb_field(sk, skb, word, mask, val, 0, 1); > + set_queue(skb, (csk->txq_idx << 1) | CPL_PRIORITY_DATA, sk); > + csk->wr_credits -= credits_needed; > + csk->wr_unacked += credits_needed; > + enqueue_wr(csk, skb); > + cxgb4_ofld_send(csk->egress_dev, skb); > +} What happens if e.g. alloc_skb() fails here? Is it fine to ignore the error altogether? > + > +/* > + * Set one of the t_flags bits in the TCB. > + */ > +void chtls_set_tcb_tflag(struct sock *sk, unsigned int bit_pos, int val) > +{ > + return chtls_set_tcb_field(sk, 1, 1ULL << bit_pos, > + val << bit_pos); > +} > + > +static void chtls_set_tcb_keyid(struct sock *sk, int keyid) > +{ > + return chtls_set_tcb_field(sk, 31, 0xFFFFFFFFULL, keyid); > +} > + > +static void chtls_set_tcb_seqno(struct sock *sk) > +{ > + return chtls_set_tcb_field(sk, 28, ~0ULL, 0); > +} > + > +static void chtls_set_tcb_quiesce(struct sock *sk, int val) > +{ > + return chtls_set_tcb_field(sk, 1, (1ULL << TF_RX_QUIESCE_S), > + TF_RX_QUIESCE_V(val)); > +} Why would you return from a void function? IMHO, it would be more productive if you could address the comments you are receiving a bit more carefully instead of patching them up quickly and race to re-post, because this doesn't seem to gain any time. -- Stefano