* [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes
@ 2021-04-15 7:47 Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 1/4] ch_ktls: Fix kernel panic Vinay Kumar Yadav
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Vinay Kumar Yadav @ 2021-04-15 7:47 UTC (permalink / raw)
To: netdev, davem, kuba, borisp, john.fastabend; +Cc: secdev, Vinay Kumar Yadav
This series of patches fix following bugs in Chelsio inline tls driver.
Patch1: kernel panic.
Patch2: connection close issue.
Patch3: tcb close call issue.
Patch4: unnecessary snd_una update.
Vinay Kumar Yadav (4):
ch_ktls: Fix kernel panic
ch_ktls: fix device connection close
ch_ktls: tcb close causes tls connection failure
ch_ktls: do not send snd_una update to TCB in middle
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 102 ++----------------
1 file changed, 11 insertions(+), 91 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/4] ch_ktls: Fix kernel panic
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
@ 2021-04-15 7:47 ` Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 2/4] ch_ktls: fix device connection close Vinay Kumar Yadav
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Vinay Kumar Yadav @ 2021-04-15 7:47 UTC (permalink / raw)
To: netdev, davem, kuba, borisp, john.fastabend
Cc: secdev, Vinay Kumar Yadav, Rohit Maheshwari
Taking page refcount is not ideal and causes kernel panic
sometimes. It's better to take tx_ctx lock for the complete
skb transmit, to avoid page cleanup if ACK received in middle.
Fixes: 5a4b9fe7fece ("cxgb4/chcr: complete record tx handling")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 24 ++++---------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index 1115b8f9ea4e..e39fa0940367 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -2010,12 +2010,11 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
* we will send the complete record again.
*/
+ spin_lock_irqsave(&tx_ctx->base.lock, flags);
+
do {
- int i;
cxgb4_reclaim_completed_tx(adap, &q->q, true);
- /* lock taken */
- spin_lock_irqsave(&tx_ctx->base.lock, flags);
/* fetch the tls record */
record = tls_get_record(&tx_ctx->base, tcp_seq,
&tx_info->record_no);
@@ -2074,11 +2073,11 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
tls_end_offset, skb_offset,
0);
- spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
if (ret) {
/* free the refcount taken earlier */
if (tls_end_offset < data_len)
dev_kfree_skb_any(skb);
+ spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
goto out;
}
@@ -2088,16 +2087,6 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
continue;
}
- /* increase page reference count of the record, so that there
- * won't be any chance of page free in middle if in case stack
- * receives ACK and try to delete the record.
- */
- for (i = 0; i < record->num_frags; i++)
- __skb_frag_ref(&record->frags[i]);
- /* lock cleared */
- spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
-
-
/* if a tls record is finishing in this SKB */
if (tls_end_offset <= data_len) {
ret = chcr_end_part_handler(tx_info, skb, record,
@@ -2122,13 +2111,9 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
data_len = 0;
}
- /* clear the frag ref count which increased locally before */
- for (i = 0; i < record->num_frags; i++) {
- /* clear the frag ref count */
- __skb_frag_unref(&record->frags[i]);
- }
/* if any failure, come out from the loop. */
if (ret) {
+ spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
if (th->fin)
dev_kfree_skb_any(skb);
@@ -2143,6 +2128,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
} while (data_len > 0);
+ spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
atomic64_inc(&port_stats->ktls_tx_encrypted_packets);
atomic64_add(skb_data_len, &port_stats->ktls_tx_encrypted_bytes);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 2/4] ch_ktls: fix device connection close
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 1/4] ch_ktls: Fix kernel panic Vinay Kumar Yadav
@ 2021-04-15 7:47 ` Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 3/4] ch_ktls: tcb close causes tls connection failure Vinay Kumar Yadav
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Vinay Kumar Yadav @ 2021-04-15 7:47 UTC (permalink / raw)
To: netdev, davem, kuba, borisp, john.fastabend
Cc: secdev, Vinay Kumar Yadav, Rohit Maheshwari
When sge queue is full and chcr_ktls_xmit_wr_complete()
returns failure, skb is not freed if it is not the last tls record in
this skb, causes refcount never gets freed and tls_dev_del()
never gets called on this connection.
Fixes: 5a4b9fe7fece ("cxgb4/chcr: complete record tx handling")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
.../net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index e39fa0940367..a626560f8365 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1735,7 +1735,9 @@ static int chcr_end_part_handler(struct chcr_ktls_info *tx_info,
struct sge_eth_txq *q, u32 skb_offset,
u32 tls_end_offset, bool last_wr)
{
+ bool free_skb_if_tx_fails = false;
struct sk_buff *nskb = NULL;
+
/* check if it is a complete record */
if (tls_end_offset == record->len) {
nskb = skb;
@@ -1758,6 +1760,8 @@ static int chcr_end_part_handler(struct chcr_ktls_info *tx_info,
if (last_wr)
dev_kfree_skb_any(skb);
+ else
+ free_skb_if_tx_fails = true;
last_wr = true;
@@ -1769,6 +1773,8 @@ static int chcr_end_part_handler(struct chcr_ktls_info *tx_info,
record->num_frags,
(last_wr && tcp_push_no_fin),
mss)) {
+ if (free_skb_if_tx_fails)
+ dev_kfree_skb_any(skb);
goto out;
}
tx_info->prev_seq = record->end_seq;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 3/4] ch_ktls: tcb close causes tls connection failure
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 1/4] ch_ktls: Fix kernel panic Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 2/4] ch_ktls: fix device connection close Vinay Kumar Yadav
@ 2021-04-15 7:47 ` Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 4/4] ch_ktls: do not send snd_una update to TCB in middle Vinay Kumar Yadav
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Vinay Kumar Yadav @ 2021-04-15 7:47 UTC (permalink / raw)
To: netdev, davem, kuba, borisp, john.fastabend
Cc: secdev, Vinay Kumar Yadav, Rohit Maheshwari
HW doesn't need marking TCB closed. This TCB state change
sometimes causes problem to the new connection which gets
the same tid.
Fixes: 34aba2c45024 ("cxgb4/chcr : Register to tls add and del callback")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index a626560f8365..8559eec161f0 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -349,18 +349,6 @@ static int chcr_set_tcb_field(struct chcr_ktls_info *tx_info, u16 word,
return cxgb4_ofld_send(tx_info->netdev, skb);
}
-/*
- * chcr_ktls_mark_tcb_close: mark tcb state to CLOSE
- * @tx_info - driver specific tls info.
- * return: NET_TX_OK/NET_XMIT_DROP.
- */
-static int chcr_ktls_mark_tcb_close(struct chcr_ktls_info *tx_info)
-{
- return chcr_set_tcb_field(tx_info, TCB_T_STATE_W,
- TCB_T_STATE_V(TCB_T_STATE_M),
- CHCR_TCB_STATE_CLOSED, 1);
-}
-
/*
* chcr_ktls_dev_del: call back for tls_dev_del.
* Remove the tid and l2t entry and close the connection.
@@ -395,8 +383,6 @@ static void chcr_ktls_dev_del(struct net_device *netdev,
/* clear tid */
if (tx_info->tid != -1) {
- /* clear tcb state and then release tid */
- chcr_ktls_mark_tcb_close(tx_info);
cxgb4_remove_tid(&tx_info->adap->tids, tx_info->tx_chan,
tx_info->tid, tx_info->ip_family);
}
@@ -574,7 +560,6 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
return 0;
free_tid:
- chcr_ktls_mark_tcb_close(tx_info);
#if IS_ENABLED(CONFIG_IPV6)
/* clear clip entry */
if (tx_info->ip_family == AF_INET6)
@@ -672,10 +657,6 @@ static int chcr_ktls_cpl_act_open_rpl(struct adapter *adap,
if (tx_info->pending_close) {
spin_unlock(&tx_info->lock);
if (!status) {
- /* it's a late success, tcb status is established,
- * mark it close.
- */
- chcr_ktls_mark_tcb_close(tx_info);
cxgb4_remove_tid(&tx_info->adap->tids, tx_info->tx_chan,
tid, tx_info->ip_family);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 4/4] ch_ktls: do not send snd_una update to TCB in middle
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
` (2 preceding siblings ...)
2021-04-15 7:47 ` [PATCH net 3/4] ch_ktls: tcb close causes tls connection failure Vinay Kumar Yadav
@ 2021-04-15 7:47 ` Vinay Kumar Yadav
2021-04-15 17:30 ` [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Jakub Kicinski
2021-04-16 0:00 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: Vinay Kumar Yadav @ 2021-04-15 7:47 UTC (permalink / raw)
To: netdev, davem, kuba, borisp, john.fastabend
Cc: secdev, Vinay Kumar Yadav, Rohit Maheshwari
snd_una update should not be done when the same skb is being
sent out.chcr_short_record_handler() sends it again even
though SND_UNA update is already sent for the skb in
chcr_ktls_xmit(), which causes mismatch in un-acked
TCP seq number, later causes problem in sending out
complete record.
Fixes: 429765a149f1 ("chcr: handle partial end part of a record")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 53 -------------------
1 file changed, 53 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index 8559eec161f0..a3f5b80888e5 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1644,54 +1644,6 @@ static void chcr_ktls_copy_record_in_skb(struct sk_buff *nskb,
refcount_add(nskb->truesize, &nskb->sk->sk_wmem_alloc);
}
-/*
- * chcr_ktls_update_snd_una: Reset the SEND_UNA. It will be done to avoid
- * sending the same segment again. It will discard the segment which is before
- * the current tx max.
- * @tx_info - driver specific tls info.
- * @q - TX queue.
- * return: NET_TX_OK/NET_XMIT_DROP.
- */
-static int chcr_ktls_update_snd_una(struct chcr_ktls_info *tx_info,
- struct sge_eth_txq *q)
-{
- struct fw_ulptx_wr *wr;
- unsigned int ndesc;
- int credits;
- void *pos;
- u32 len;
-
- len = sizeof(*wr) + roundup(CHCR_SET_TCB_FIELD_LEN, 16);
- ndesc = DIV_ROUND_UP(len, 64);
-
- credits = chcr_txq_avail(&q->q) - ndesc;
- if (unlikely(credits < 0)) {
- chcr_eth_txq_stop(q);
- return NETDEV_TX_BUSY;
- }
-
- pos = &q->q.desc[q->q.pidx];
-
- wr = pos;
- /* ULPTX wr */
- wr->op_to_compl = htonl(FW_WR_OP_V(FW_ULPTX_WR));
- wr->cookie = 0;
- /* fill len in wr field */
- wr->flowid_len16 = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(len, 16)));
-
- pos += sizeof(*wr);
-
- pos = chcr_write_cpl_set_tcb_ulp(tx_info, q, tx_info->tid, pos,
- TCB_SND_UNA_RAW_W,
- TCB_SND_UNA_RAW_V(TCB_SND_UNA_RAW_M),
- TCB_SND_UNA_RAW_V(0), 0);
-
- chcr_txq_advance(&q->q, ndesc);
- cxgb4_ring_tx_db(tx_info->adap, &q->q, ndesc);
-
- return 0;
-}
-
/*
* chcr_end_part_handler: This handler will handle the record which
* is complete or if record's end part is received. T6 adapter has a issue that
@@ -1892,11 +1844,6 @@ static int chcr_short_record_handler(struct chcr_ktls_info *tx_info,
/* reset tcp_seq as per the prior_data_required len */
tcp_seq -= prior_data_len;
}
- /* reset snd una, so the middle record won't send the already
- * sent part.
- */
- if (chcr_ktls_update_snd_una(tx_info, q))
- goto out;
atomic64_inc(&tx_info->adap->ch_ktls_stats.ktls_tx_middle_pkts);
} else {
atomic64_inc(&tx_info->adap->ch_ktls_stats.ktls_tx_start_pkts);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
` (3 preceding siblings ...)
2021-04-15 7:47 ` [PATCH net 4/4] ch_ktls: do not send snd_una update to TCB in middle Vinay Kumar Yadav
@ 2021-04-15 17:30 ` Jakub Kicinski
2021-04-16 0:00 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-04-15 17:30 UTC (permalink / raw)
To: Vinay Kumar Yadav; +Cc: netdev, davem, borisp, john.fastabend, secdev
On Thu, 15 Apr 2021 13:17:44 +0530 Vinay Kumar Yadav wrote:
> This series of patches fix following bugs in Chelsio inline tls driver.
Nothing objectionable here.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
` (4 preceding siblings ...)
2021-04-15 17:30 ` [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Jakub Kicinski
@ 2021-04-16 0:00 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-04-16 0:00 UTC (permalink / raw)
To: Vinay Kumar Yadav; +Cc: netdev, davem, kuba, borisp, john.fastabend, secdev
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Thu, 15 Apr 2021 13:17:44 +0530 you wrote:
> This series of patches fix following bugs in Chelsio inline tls driver.
> Patch1: kernel panic.
> Patch2: connection close issue.
> Patch3: tcb close call issue.
> Patch4: unnecessary snd_una update.
>
> Vinay Kumar Yadav (4):
> ch_ktls: Fix kernel panic
> ch_ktls: fix device connection close
> ch_ktls: tcb close causes tls connection failure
> ch_ktls: do not send snd_una update to TCB in middle
>
> [...]
Here is the summary with links:
- [net,1/4] ch_ktls: Fix kernel panic
https://git.kernel.org/netdev/net/c/1a73e427b824
- [net,2/4] ch_ktls: fix device connection close
https://git.kernel.org/netdev/net/c/bc16efd24306
- [net,3/4] ch_ktls: tcb close causes tls connection failure
https://git.kernel.org/netdev/net/c/21d8c25e3f4b
- [net,4/4] ch_ktls: do not send snd_una update to TCB in middle
https://git.kernel.org/netdev/net/c/e8a4155567b3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-16 0:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 7:47 [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 1/4] ch_ktls: Fix kernel panic Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 2/4] ch_ktls: fix device connection close Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 3/4] ch_ktls: tcb close causes tls connection failure Vinay Kumar Yadav
2021-04-15 7:47 ` [PATCH net 4/4] ch_ktls: do not send snd_una update to TCB in middle Vinay Kumar Yadav
2021-04-15 17:30 ` [PATCH net 0/4] chelsio/ch_ktls: chelsio inline tls driver bug fixes Jakub Kicinski
2021-04-16 0:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).