netdev - Re: [PATCH V2 net-next 3/3] selftests/net: reap zerocopy completions passed up as ancillary data.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 25 Feb 2018 10:56:14 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Santosh Shilimkar <santosh.shilimkar@...cle.com>
Subject: Re: [PATCH V2 net-next 3/3] selftests/net: reap zerocopy completions
 passed up as ancillary data.

On Fri, Feb 23, 2018 at 5:08 PM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> PF_RDS sockets pass up cookies for zerocopy completion as ancillary
> data. Update msg_zerocopy to reap this information.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@...cle.com>
> ---
> v2: receive zerocopy completion notification as POLLIN
>
>  tools/testing/selftests/net/msg_zerocopy.c |   60 ++++++++++++++++++++++++----
>  1 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
> index eff9cf2..8c466e8 100644
> --- a/tools/testing/selftests/net/msg_zerocopy.c
> +++ b/tools/testing/selftests/net/msg_zerocopy.c
> @@ -344,7 +344,48 @@ static int do_setup_tx(int domain, int type, int protocol)
>         return fd;
>  }
>
> -static bool do_recv_completion(int fd)
> +static int do_process_zerocopy_cookies(struct rds_zcopy_cookies *ck)
> +{
> +       int ncookies, i;

Since this is example code, let's be pedantic about type: uint32_t.
Or don't use a local variable.

> +
> +       ncookies = ck->num;
> +       if (ncookies > RDS_MAX_ZCOOKIES)
> +               error(1, 0, "Returned %d cookies, max expected %d\n",
> +                     ncookies, RDS_MAX_ZCOOKIES);
> +       for (i = 0; i < ncookies; i++)
> +               if (cfg_verbose >= 2)
> +                       fprintf(stderr, "%d\n", ck->cookies[i]);
> +       return ncookies;
> +}
> +
> +static int do_recvmsg_completion(int fd)
> +{
> +       struct msghdr msg;
> +       char cmsgbuf[256];

more precise: CMSG_SPACE(sizeof(*ck));

> +       struct cmsghdr *cmsg;
> +       bool ret = false;
> +       struct rds_zcopy_cookies *ck;

then this must move above. Reverse christmas tree is preferred, anyway.

> +
> +       memset(&msg, 0, sizeof(msg));
> +       msg.msg_control = cmsgbuf;
> +       msg.msg_controllen = sizeof(cmsgbuf);
> +
> +       if (recvmsg(fd, &msg, MSG_DONTWAIT))
> +               return ret;

check msg_flags & MSG_CTRUNC

> +       for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> +               if (cmsg->cmsg_level == SOL_RDS &&
> +                   cmsg->cmsg_type == RDS_CMSG_ZCOPY_COMPLETION) {
> +
> +                       ck = (struct rds_zcopy_cookies *)CMSG_DATA(cmsg);
> +                       completions += do_process_zerocopy_cookies(ck);
> +                       ret = true;
> +                       break;
> +               }

maybe warn on unexpected other type or level

> +       }
> +       return ret;
> +}
> +
> +static bool do_recv_completion(int fd, int domain)
>  {
>         struct sock_extended_err *serr;
>         struct msghdr msg = {};
> @@ -353,6 +394,9 @@ static bool do_recv_completion(int fd)
>         int ret, zerocopy;
>         char control[100];
>
> +       if (domain == PF_RDS)
> +               return do_recvmsg_completion(fd);
> +
>         msg.msg_control = control;
>         msg.msg_controllen = sizeof(control);
>
> @@ -409,20 +453,20 @@ static bool do_recv_completion(int fd)
>  }
>
>  /* Read all outstanding messages on the errqueue */
> -static void do_recv_completions(int fd)
> +static void do_recv_completions(int fd, int domain)
>  {
> -       while (do_recv_completion(fd)) {}
> +       while (do_recv_completion(fd, domain)) {}
>  }
>
>  /* Wait for all remaining completions on the errqueue */
> -static void do_recv_remaining_completions(int fd)
> +static void do_recv_remaining_completions(int fd, int domain)
>  {
>         int64_t tstop = gettimeofday_ms() + cfg_waittime_ms;
>
>         while (completions < expected_completions &&
>                gettimeofday_ms() < tstop) {
> -               if (do_poll(fd, POLLERR))
> -                       do_recv_completions(fd);
> +               if (do_poll(fd, domain == PF_RDS ? POLLIN : POLLERR))
> +                       do_recv_completions(fd, domain);
>         }
>
>         if (completions < expected_completions)
> @@ -503,13 +547,13 @@ static void do_tx(int domain, int type, int protocol)
>
>                 while (!do_poll(fd, POLLOUT)) {
>                         if (cfg_zerocopy)
> -                               do_recv_completions(fd);
> +                               do_recv_completions(fd, domain);
>                 }
>
>         } while (gettimeofday_ms() < tstop);
>
>         if (cfg_zerocopy)
> -               do_recv_remaining_completions(fd);
> +               do_recv_remaining_completions(fd, domain);
>
>         if (close(fd))
>                 error(1, errno, "close");
> --
> 1.7.1
>