On Wed, Jan 24, 2018 at 12:46 PM, Sowmini Varadhan
<sowmini.varad...@oracle.com> wrote:
> Send a cookie with sendmsg() on PF_RDS sockets, and process the
> returned batched cookies in do_recv_completion()
>
> Signed-off-by: Sowmini Varadhan <sowmini.varad...@oracle.com>
> ---
> tools/testing/selftests/net/msg_zerocopy.c | 119 ++++++++++++++++++++-------
> 1 files changed, 88 insertions(+), 31 deletions(-)
>
> diff --git a/tools/testing/selftests/net/msg_zerocopy.c
> b/tools/testing/selftests/net/msg_zerocopy.c
> index 7a5b353..19d2b1a 100644
> --- a/tools/testing/selftests/net/msg_zerocopy.c
> +++ b/tools/testing/selftests/net/msg_zerocopy.c
> @@ -168,7 +168,26 @@ static int do_accept(int fd)
> return fd;
> }
>
> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy)
> +static void add_zcopy_cookie(struct msghdr *msg)
> +{
> + int olen = msg->msg_controllen;
> + struct cmsghdr *cm;
> + static uint32_t cookie;
> +
> + msg->msg_controllen += CMSG_SPACE(sizeof(cookie));
> + msg->msg_control = (struct cmsghdr *)realloc(msg->msg_control,
> + msg->msg_controllen);
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.
char control[CMSG_SPACE(sizeof(uint32_t)];
> + if (!msg->msg_control)
> + error(1, errno, "cannot allocate cmsghdr for cookie");
> + cm = (void *)msg->msg_control + olen;
> + cm->cmsg_len = CMSG_SPACE(sizeof(cookie));
CMSG_LEN
> + cm->cmsg_level = SOL_RDS;
> + cm->cmsg_type = RDS_CMSG_ZCOPY_COOKIE;
> + ++cookie;
> + memcpy(CMSG_DATA(cm), &cookie, sizeof(cookie));
cookie is not initialized
> @@ -346,36 +382,57 @@ static bool do_recv_completion(int fd)
> cm->cmsg_level, cm->cmsg_type);
>
> serr = (void *) CMSG_DATA(cm);
> - if (serr->ee_origin != SO_EE_ORIGIN_ZEROCOPY)
> - error(1, 0, "serr: wrong origin: %u", serr->ee_origin);
> - if (serr->ee_errno != 0)
> - error(1, 0, "serr: wrong error code: %u", serr->ee_errno);
>
> - hi = serr->ee_data;
> - lo = serr->ee_info;
> - range = hi - lo + 1;
> + switch (serr->ee_origin) {
> + case SO_EE_ORIGIN_ZEROCOPY: {
> + if (serr->ee_errno != 0)
> + error(1, 0, "serr: wrong error code: %u",
> + serr->ee_errno);
> + hi = serr->ee_data;
> + lo = serr->ee_info;
> + range = hi - lo + 1;
> +
> + /* Detect notification gaps. These should not happen often,
> + * if at all. Gaps can occur due to drops, reordering and
> + * retransmissions.
> + */
> + if (lo != next_completion)
> + fprintf(stderr, "gap: %u..%u does not append to %u\n",
> + lo, hi, next_completion);
> + next_completion = hi + 1;
> +
> + zerocopy = !(serr->ee_code & SO_EE_CODE_ZEROCOPY_COPIED);
> + if (zerocopied == -1)
> + zerocopied = zerocopy;
> + else if (zerocopied != zerocopy) {
> + fprintf(stderr, "serr: inconsistent\n");
> + zerocopied = zerocopy;
> + }
> + if (cfg_verbose >= 2)
> + fprintf(stderr, "completed: %u (h=%u l=%u)\n",
> + range, hi, lo);
>
> - /* Detect notification gaps. These should not happen often, if at all.
> - * Gaps can occur due to drops, reordering and retransmissions.
> - */
> - if (lo != next_completion)
> - fprintf(stderr, "gap: %u..%u does not append to %u\n",
> - lo, hi, next_completion);
> - next_completion = hi + 1;
> -
> - zerocopy = !(serr->ee_code & SO_EE_CODE_ZEROCOPY_COPIED);
> - if (zerocopied == -1)
> - zerocopied = zerocopy;
> - else if (zerocopied != zerocopy) {
> - fprintf(stderr, "serr: inconsistent\n");
> - zerocopied = zerocopy;
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.
> + completions += range;
> + break;
> + }
> + case SO_EE_ORIGIN_ZCOOKIE: {
> + int ncookies, i;
> +
> + if (serr->ee_errno != 0)
> + error(1, 0, "serr: wrong error code: %u",
> + serr->ee_errno);
> + ncookies = serr->ee_data;
Verify ncookies <= MAX_..
Verify ret == ncookies * sizeof(uint32_t)
> + for (i = 0; i < ncookies; i++)
> + if (cfg_verbose >= 2)
> + fprintf(stderr, "%d\n", ckbuf[i]);
> + completions += ncookies;
> + zerocopied = 1;
Unused in this path
> + break;
> + }
> + default:
> + error(1, 0, "serr: wrong origin: %u", serr->ee_origin);
> }
>
> - if (cfg_verbose >= 2)
> - fprintf(stderr, "completed: %u (h=%u l=%u)\n",
> - range, hi, lo);
> -
> - completions += range;
> return true;
> }
