Re: [PATCH net-next 7/7] selftests/net: add zerocopy support for PF_RDS test case
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2018-01-28 14:00:47
On Wed, Jan 24, 2018 at 12:46 PM, Sowmini Varadhan [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Send a cookie with sendmsg() on PF_RDS sockets, and process the returned batched cookies in do_recv_completion() Signed-off-by: Sowmini Varadhan <redacted> --- 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
quoted hunk ↗ jump to hunk
@@ -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;
}