Commit 6220191d authored by Jacek Kalwas's avatar Jacek Kalwas Committed by Tomasz Zawadzki
Browse files

sock: fix flush on success return value 

It should be 0 and not number of bytes sent, see API contract.
https://review.spdk.io/c/spdk/spdk/+/26067


Somehow adjusting implementation was missed.

Change-Id: Id4b54a8d99d3ac7f8ac45eee3d75567858356c0e
Signed-off-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26109


Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz@tzawadzki.com>
parent 2bcddb3a

module/sock/posix/posix.c

+3 −5
Original line number Diff line number Diff line
@@ -1282,7 +1282,7 @@ _sock_flush(struct spdk_sock *sock) 
	int retval;

	struct spdk_sock_request *req;

	int i;

	ssize_t rc, sent;

	ssize_t rc;

	unsigned int offset;

	size_t len;

	bool is_zcopy = false;
@@ -1327,8 +1327,6 @@ _sock_flush(struct spdk_sock *sock) 
		return -1;

	}



	sent = rc;



	if (is_zcopy) {

		psock->sendmsg_idx++;

	}
@@ -1358,7 +1356,7 @@ _sock_flush(struct spdk_sock *sock) 
			if (len > (size_t)rc) {

				/* This element was partially sent. */

				req->internal.offset += rc;

				return sent;

				return 0;

			}



			offset = 0;
@@ -1387,7 +1385,7 @@ _sock_flush(struct spdk_sock *sock) 
		req = TAILQ_FIRST(&sock->queued_reqs);

	}



	return sent;

	return 0;

}



static int

module/sock/uring/uring.c

+2 −2
Original line number Diff line number Diff line
@@ -1910,14 +1910,14 @@ uring_sock_flush(struct spdk_sock *_sock) 
		retval = recvmsg(sock->fd, &task->msg, MSG_ERRQUEUE);

		if (retval < 0) {

			if (errno == EWOULDBLOCK || errno == EAGAIN) {

				return rc;

				return 0;

			}

		}

		_sock_check_zcopy(_sock, retval);;

	}

#endif



	return rc;

	return 0;

}



static int

test/unit/lib/sock/posix.c/posix_ut.c

+11 −11
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ flush(void) 
	MOCK_SET(sendmsg, 64);

	cb_arg1 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 64);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));
@@ -96,7 +96,7 @@ flush(void) 
	cb_arg1 = false;

	cb_arg2 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 128);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(cb_arg2 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));
@@ -108,7 +108,7 @@ flush(void) 
	cb_arg1 = false;

	cb_arg2 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 64);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(cb_arg2 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req2);
@@ -120,7 +120,7 @@ flush(void) 
	MOCK_SET(sendmsg, 10);

	cb_arg1 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 10);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req1);
@@ -128,7 +128,7 @@ flush(void) 
	MOCK_SET(sendmsg, 24);

	cb_arg1 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 24);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req1);
@@ -136,7 +136,7 @@ flush(void) 
	MOCK_SET(sendmsg, 30);

	cb_arg1 = false;

	rc = _sock_flush(sock);

	CU_ASSERT(rc == 30);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));
@@ -206,7 +206,7 @@ flush_req_chunks_with_zero_copy_threshold(void) 
	/* Send first chunk above zcopy threshold. */

	MOCK_SET(sendmsg, 75);

	rc = posix_sock_flush(sock);

	CU_ASSERT(rc == 75);

	CU_ASSERT(rc == 0);

	/* Sent partially, request is not completed. */

	CU_ASSERT(req_completed == false);
@@ -215,7 +215,7 @@ flush_req_chunks_with_zero_copy_threshold(void) 
	/* Notification not yet arrived. */

	MOCK_SET(recvmsg, -EAGAIN);

	rc = posix_sock_flush(sock);

	CU_ASSERT(rc == 25);

	CU_ASSERT(rc == 0);

	/* Sent fully, but zcopy not yet arrived, so request is not completed. */

	CU_ASSERT(req_completed == false);
@@ -279,7 +279,7 @@ flush_two_reqs_chunks_with_zero_copy_threshold(void) 
	/* No zcopy notification for req1. */

	MOCK_SET(recvmsg, -EAGAIN);

	rc = posix_sock_flush(sock);

	CU_ASSERT(rc == 100 + 75);

	CU_ASSERT(rc == 0);

	CU_ASSERT(req1_completed == false);

	CU_ASSERT(req2_completed == false);
@@ -291,7 +291,7 @@ flush_two_reqs_chunks_with_zero_copy_threshold(void) 
	MOCK_ENQUEUE(recvmsg, 0); /* Pass notification range high. */

	MOCK_ENQUEUE(recvmsg, -EAGAIN); /* No more messages. */

	rc = posix_sock_flush(sock);

	CU_ASSERT(rc == 20);

	CU_ASSERT(rc == 0);

	CU_ASSERT(req1_completed == true);

	CU_ASSERT(req2_completed == false);
@@ -299,7 +299,7 @@ flush_two_reqs_chunks_with_zero_copy_threshold(void) 
	MOCK_SET(sendmsg, 5);

	/* No need to recvmsg, notification for req2 zcopy chunk already received. */

	rc = posix_sock_flush(sock);

	CU_ASSERT(rc == 5);

	CU_ASSERT(rc == 0);

	/* Req2 should be completed within this flush. */

	CU_ASSERT(req2_completed == true);

test/unit/lib/sock/uring.c/uring_ut.c

+6 −6
Original line number Diff line number Diff line
@@ -87,7 +87,7 @@ flush_client(void) 
	MOCK_SET(sendmsg, 192);

	cb_arg1 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 192);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));
@@ -98,7 +98,7 @@ flush_client(void) 
	cb_arg1 = false;

	cb_arg2 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 256);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(cb_arg2 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));
@@ -110,7 +110,7 @@ flush_client(void) 
	cb_arg1 = false;

	cb_arg2 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 192);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(cb_arg2 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req2);
@@ -122,7 +122,7 @@ flush_client(void) 
	MOCK_SET(sendmsg, 10);

	cb_arg1 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 10);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req1);
@@ -130,7 +130,7 @@ flush_client(void) 
	MOCK_SET(sendmsg, 52);

	cb_arg1 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 52);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == false);

	CU_ASSERT(TAILQ_FIRST(&sock->queued_reqs) == req1);
@@ -138,7 +138,7 @@ flush_client(void) 
	MOCK_SET(sendmsg, 130);

	cb_arg1 = false;

	rc = uring_sock_flush(sock);

	CU_ASSERT(rc == 130);

	CU_ASSERT(rc == 0);

	CU_ASSERT(cb_arg1 == true);

	CU_ASSERT(TAILQ_EMPTY(&sock->queued_reqs));