Re: [PATCH net-next 7/7] selftests/net: add zerocopy support for PF_RDS test case

On Sun, Jan 28, 2018 at 5:18 PM, Sowmini Varadhan
[off-list ref] wrote:

Re-ordering review comments for selftests a bit..

quoted

quoted

+       cm->cmsg_level = SOL_RDS;
+       cm->cmsg_type = RDS_CMSG_ZCOPY_COOKIE;
+       ++cookie;
+       memcpy(CMSG_DATA(cm), &cookie, sizeof(cookie));

cookie is not initialized

it's a static uint32_t in the function. It will get initialized to 0.

Oh right. I missed that.

But the whole test program is rather simplistic, since it doesnt
actually verify the value of the cookies (hopefully me pending
updates to rds-stress will provide better testing/examples in this
space, because I actully have something multi-threaded, that must
necesarily use a truly random value for the cookie, and must really
make sure that the returned value from the kernel matches some
cookie that is pending)

It might be nice to at least increment the variable on each
successful send. The test is single threaded anyway. And
then we can test that the returned values are in the defined
range.

Also, coalescing some related comments for patch 6/7:

quoted

+       memset(&msg, 0, sizeof(msg));
+       msg.msg_name = &din;
+       msg.msg_namelen = sizeof(din);

Not read, so no need to configure. In that case a simpler
recv will do and is more concise than setting up recvmsg.

Real RDS applications actually have to use recvmsg, since they
can get cmsg info about other things like congestion notification
so let's leave this as recvmsg - it does no harm, and provides
test coverage for the recvmsg case as well.

Ah, okay. Yes, let's keep it as, then.