Re-ordering review comments for selftests a bit.. > > + 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. 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) > > +static void add_zcopy_cookie(struct msghdr *msg) > Please just allocate ahead of time. And since cookie size is fixed > and small just define a local variable on the stack in do_sendmsg. Ok! I was just trying to make this as future-proof as possible, and provide useful example code for the next cut/paste developer to use. > > + cm->cmsg_len = CMSG_SPACE(sizeof(cookie)); > CMSG_LEN : > Instead of indenting all this existing code please add a helper > do_recv_completion_zcookie, call that if SO_EE_ORIGIN_ZCOOKIE > and fall through to existing code otherwise. : > Verify ncookies <= MAX_.. > Verify ret == ncookies * sizeof(uint32_t) > > + zerocopied = 1; > Unused in this path Ok, will fix all these. Also, coalescing some related comments for patch 6/7: > + 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.
