Skip to content

Commit 63fab61

Browse files
author
Herton R. Krzesinski
committed
Merge: RDMA/siw: Always consume all skbuf data in sk_data_ready() upcall.
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/1611 Bugzilla: https://bugzilla.redhat.com/2131780 commit 7542098 Author: Bernard Metzler <bmt@zurich.ibm.com> Date: Tue Sep 20 10:12:02 2022 +0200 RDMA/siw: Always consume all skbuf data in sk_data_ready() upcall. For header and trailer/padding processing, siw did not consume new skb data until minimum amount present to fill current header or trailer structure, including potential payload padding. Not consuming any data during upcall may cause a receive stall, since tcp_read_sock() is not upcalling again if no new data arrive. A NFSoRDMA client got stuck at RDMA Write reception of unaligned payload, if the current skb did contain only the expected 3 padding bytes, but not the 4 bytes CRC trailer. Expecting 4 more bytes already arrived in another skb, and not consuming those 3 bytes in the current upcall left the Write incomplete, waiting for the CRC forever. Fixes: 8b6a361 ("rdma/siw: receive path") Reported-by: Olga Kornievskaia <kolga@netapp.com> Tested-by: Olga Kornievskaia <kolga@netapp.com> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> Link: https://lore.kernel.org/r/20220920081202.223629-1-bmt@zurich.ibm.com Signed-off-by: Leon Romanovsky <leon@kernel.org> Signed-off-by: Kamal Heib <kheib@redhat.com> Approved-by: José Ignacio Tornos Martínez <jtornosm@redhat.com> Approved-by: Íñigo Huguet <ihuguet@redhat.com> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
2 parents 5b496c8 + a4c3a52 commit 63fab61

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

drivers/infiniband/sw/siw/siw_qp_rx.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -961,27 +961,28 @@ int siw_proc_terminate(struct siw_qp *qp)
961961
static int siw_get_trailer(struct siw_qp *qp, struct siw_rx_stream *srx)
962962
{
963963
struct sk_buff *skb = srx->skb;
964+
int avail = min(srx->skb_new, srx->fpdu_part_rem);
964965
u8 *tbuf = (u8 *)&srx->trailer.crc - srx->pad;
965966
__wsum crc_in, crc_own = 0;
966967

967968
siw_dbg_qp(qp, "expected %d, available %d, pad %u\n",
968969
srx->fpdu_part_rem, srx->skb_new, srx->pad);
969970

970-
if (srx->skb_new < srx->fpdu_part_rem)
971-
return -EAGAIN;
972-
973-
skb_copy_bits(skb, srx->skb_offset, tbuf, srx->fpdu_part_rem);
971+
skb_copy_bits(skb, srx->skb_offset, tbuf, avail);
974972

975-
if (srx->mpa_crc_hd && srx->pad)
976-
crypto_shash_update(srx->mpa_crc_hd, tbuf, srx->pad);
973+
srx->skb_new -= avail;
974+
srx->skb_offset += avail;
975+
srx->skb_copied += avail;
976+
srx->fpdu_part_rem -= avail;
977977

978-
srx->skb_new -= srx->fpdu_part_rem;
979-
srx->skb_offset += srx->fpdu_part_rem;
980-
srx->skb_copied += srx->fpdu_part_rem;
978+
if (srx->fpdu_part_rem)
979+
return -EAGAIN;
981980

982981
if (!srx->mpa_crc_hd)
983982
return 0;
984983

984+
if (srx->pad)
985+
crypto_shash_update(srx->mpa_crc_hd, tbuf, srx->pad);
985986
/*
986987
* CRC32 is computed, transmitted and received directly in NBO,
987988
* so there's never a reason to convert byte order.
@@ -1083,10 +1084,9 @@ static int siw_get_hdr(struct siw_rx_stream *srx)
10831084
* completely received.
10841085
*/
10851086
if (iwarp_pktinfo[opcode].hdr_len > sizeof(struct iwarp_ctrl_tagged)) {
1086-
bytes = iwarp_pktinfo[opcode].hdr_len - MIN_DDP_HDR;
1087+
int hdrlen = iwarp_pktinfo[opcode].hdr_len;
10871088

1088-
if (srx->skb_new < bytes)
1089-
return -EAGAIN;
1089+
bytes = min_t(int, hdrlen - MIN_DDP_HDR, srx->skb_new);
10901090

10911091
skb_copy_bits(skb, srx->skb_offset,
10921092
(char *)c_hdr + srx->fpdu_part_rcvd, bytes);
@@ -1096,6 +1096,9 @@ static int siw_get_hdr(struct siw_rx_stream *srx)
10961096
srx->skb_new -= bytes;
10971097
srx->skb_offset += bytes;
10981098
srx->skb_copied += bytes;
1099+
1100+
if (srx->fpdu_part_rcvd < hdrlen)
1101+
return -EAGAIN;
10991102
}
11001103

11011104
/*

0 commit comments

Comments
 (0)