On Sun, Jan 28, 2018 at 5:18 PM, Sowmini Varadhan
<sowmini.varad...@oracle.com> wrote:
>
> 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.

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:
>
>> +       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.

Reply via email to