Re: [PATCH] nvmet-tcp: fix data digest pointer calculation

On Wed, Oct 27, 2021 at 12:49:31PM +0300, Sagi Grimberg wrote:

quoted

quoted

quoted

quoted

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 07ee347..d641bfa 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c

@@ -702,7 +702,7 @@ static int nvmet_try_send_ddgst(struct nvmet_tcp_cmd *cmd, bool last_in_batch)
  	struct nvmet_tcp_queue *queue = cmd->queue;
  	struct msghdr msg = { .msg_flags = MSG_DONTWAIT };
  	struct kvec iov = {
-		.iov_base = &cmd->exp_ddgst + cmd->offset,
+		.iov_base = (u8 *)&cmd->exp_ddgst + cmd->offset,

Wouldn't be the better fix to divide cmd->offset by 4 instead of the
casts?  I can fix this up inline if that is ok.

Don't really mind, what makes it a better fix?

I generally prefer to avoid casts as they paper over bugs.

I'm fine either way.

cmd->offset can have value 0,1,2,3. cmd->offset / 4 will always be 0.
If 2 bytes of digest are transmitted in the first call to kernel_sendmsg()
then in the second call remaining 2 bytes has to be transmitted so data digest
buffer pointer should increase by 2 bytes.

There is one more bug in nvmet_try_send_ddgst() it currently does not check
whether digest is send successfully. For successful digest send
cmd->offset + ret should be equal to NVME_TCP_DIGEST_LENGTH.