From mboxrd@z Thu Jan 1 00:00:00 1970 From: Willem de Bruijn Subject: Re: [PATCH net-next 7/7] selftests/net: add zerocopy support for PF_RDS test case Date: Sun, 28 Jan 2018 15:00:05 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Network Development , David Miller , rds-devel@oss.oracle.com, santosh.shilimkar@oracle.com To: Sowmini Varadhan Return-path: Received: from mail-oi0-f66.google.com ([209.85.218.66]:45659 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751643AbeA1OAr (ORCPT ); Sun, 28 Jan 2018 09:00:47 -0500 Received: by mail-oi0-f66.google.com with SMTP id c189so2566635oib.12 for ; Sun, 28 Jan 2018 06:00:46 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Jan 24, 2018 at 12:46 PM, Sowmini Varadhan 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 > --- > 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; > }