| Message ID | 1500989968-30889-8-git-send-email-andrew.boyer@dell.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer@dell.com> wrote:
> If the buffer contents are changing,
can you please elaborate on that a little further? what are unchanged
contents and what are changing contents?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/25/17, 1:34 PM, "Or Gerlitz" <gerlitz.or@gmail.com> wrote: >On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer@dell.com> >wrote: >> If the buffer contents are changing, > >can you please elaborate on that a little further? what are unchanged >contents and what are changing contents? In qperf, the ud_bi_bw test allocates a single buffer on each side. Then it posts that one buffer 1024 times for receive and 1024 times for send. While RXE is building up the send packet, the buffer is being overwritten by incoming receives. The current process of first calculating the CRC and then copying the buffer into the packet leaves a window in which the contents and the CRC can get out of sync. By copying the buffer into the packet and then calculating the CRC based on the packet contents we eliminate the window. This seems like very strange/reckless client behavior, but whether the client has mangled the buffer or not RXE should be able to transfer it reliably. Thanks, Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> This seems like very strange/reckless client behavior, but whether the > client has mangled the buffer or not RXE should be able to transfer it > reliably. > And what if was overwritten during the memcpy? Then we end up with wrong ICRC again, right? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c index e37cc89..5c2684b 100644 --- a/drivers/infiniband/sw/rxe/rxe_mr.c +++ b/drivers/infiniband/sw/rxe/rxe_mr.c @@ -367,11 +367,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length, dest = (dir == to_mem_obj) ? ((void *)(uintptr_t)iova) : addr; + memcpy(dest, src, length); + if (crcp) *crcp = rxe_crc32(to_rdev(mem->pd->ibpd.device), - *crcp, src, length); - - memcpy(dest, src, length); + *crcp, dest, length); return 0; } @@ -401,11 +401,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length, if (bytes > length) bytes = length; + memcpy(dest, src, bytes); + if (crcp) crc = rxe_crc32(to_rdev(mem->pd->ibpd.device), - crc, src, bytes); - - memcpy(dest, src, bytes); + crc, dest, bytes); length -= bytes; addr += bytes;
If the buffer contents are changing, the current code can generate a packet with a bogus CRC. This can be seen with qperf's ud_bi_bw test. No matter what behavior the client is expecting in this case, it shouldn't result in a malformed packet. Fixes: 8700e3e7c485 ("Soft RoCE driver") Signed-off-by: Andrew Boyer <andrew.boyer@dell.com> --- drivers/infiniband/sw/rxe/rxe_mr.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)