From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30BDAC282DD for ; Thu, 23 May 2019 21:46:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E938D2168B for ; Thu, 23 May 2019 21:46:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="vF7wzIaf" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388397AbfEWVq3 (ORCPT ); Thu, 23 May 2019 17:46:29 -0400 Received: from mail-ed1-f67.google.com ([209.85.208.67]:35050 "EHLO mail-ed1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388232AbfEWVq2 (ORCPT ); Thu, 23 May 2019 17:46:28 -0400 Received: by mail-ed1-f67.google.com with SMTP id p26so11282000edr.2; Thu, 23 May 2019 14:46:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hS1J+L8InX8t+S5g40fUDrEZZWAutsMmjF58rVNfY4A=; b=vF7wzIafOwzPH1p6LsShpR37Mg3sBS7pnLiT2O9iV6X1EtYSjTwLFwI1Br/cLiMXt2 RwhRLJ6sJ8W8XSai7YLJK5m3TJbd46SuWj7BgeZG4WUd4KKSpPrpTbI+ff62ThALF71C odj8iC1OSSKR/yrb1wuok+xjLsOJO0qivUJqCPfh9PBISpU6oulbKPK4Vf1ZdTq4f//q 3N4a0kdGoEHNkmU/wZx5V5PaWeMeebWVmHcI783Ik9w5f+UmCmSyThzR/d02njmGhDq+ XzHD//Fm/PwpHXQf1ljwBKIyq9nnSNpVikR1A3hDfePxb8GaqeIJCk4D8XaeNI1moqnl 5qCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hS1J+L8InX8t+S5g40fUDrEZZWAutsMmjF58rVNfY4A=; b=d1AEp2ysj2JLc09oB3E281wUGCkk8dSHQr50i0HT73+zlJeJ0HIJPrdthuh55zALl3 wP79v8SfzlWt8+lnXpbUHu5OpoX+aOfKbi+Kibd/PbbcLzyK8Anb9QQ3DRL82edUpsUk Q2eCpGgogHh2fGbSu5gQSV0ygokG7FFYVC5I4PvNBrpcfINJxptvtc1JIPaBI4M6E45D /nWr2Ma0LZsHyMI1FFvOkNsHlZ361LpQwJGJXAsO4wJBHCiKYXOPYG/zudObp8xQ777h LM8xjaa9kNnnPSOjmF9+K2uHyShs+nEvSrlmKxto+8b3VYHwfZUz0ow4yA2sszcvj9Pu ge1g== X-Gm-Message-State: APjAAAXF3YXgeRx22mGBGVM9aUwu41zPAZ9GJNPq/hhiJ9sn1R706iSh DI7g73mqhy/scJdvOLUGqVdEw1QoGvIMboUVrLA= X-Google-Smtp-Source: APXvYqwRs1XOL8neFbNGaC+e+ESBojE/AQYVeCKksTEz4shDZdYlC7mr4NocdxwmtEAjUnfC/5JJpyz2nvSkbDYoeDo= X-Received: by 2002:a50:b665:: with SMTP id c34mr99149662ede.148.1558647986054; Thu, 23 May 2019 14:46:26 -0700 (PDT) MIME-Version: 1.0 References: <20190523210651.80902-1-fklassen@appneta.com> <20190523210651.80902-3-fklassen@appneta.com> In-Reply-To: <20190523210651.80902-3-fklassen@appneta.com> From: Willem de Bruijn Date: Thu, 23 May 2019 17:45:49 -0400 Message-ID: Subject: Re: [PATCH net 2/4] net/udpgso_bench_tx: options to exercise TX CMSG To: Fred Klassen Cc: "David S. Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , Shuah Khan , Network Development , LKML , linux-kselftest@vger.kernel.org, Willem de Bruijn Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 23, 2019 at 5:11 PM Fred Klassen wrote: > > This enhancement adds options that facilitate load testing with > additional TX CMSG options, and to optionally print results of > various send CMSG operations. > > These options are especially useful in isolating situations > where error-queue messages are lost when combined with other > CMSG operations (e.g. SO_ZEROCOPY). > > New options: > > -T - add TX CMSG that requests TX software timestamps > -H - similar to -T except request TX hardware timestamps > -q - add IP_TOS/IPV6_TCLASS TX CMSG > -P - call poll() before reading error queue > -v - print detailed results > > Fixes: 3a687bef148d ("selftests: udp gso benchmark") This is not a fix, but an extension. Fixes tags help with backporting to stable kernels. There is something to be said to backport the main change and support SO_TIMESTAMPING + UDP_GSO on older kernels, especially since it is very concise. But the tests should probably be in a separate patch set targeting net-next. > Signed-off-by: Fred Klassen > --- > tools/testing/selftests/net/udpgso_bench_tx.c | 290 ++++++++++++++++++++++++-- > 1 file changed, 273 insertions(+), 17 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c > index 4074538b5df5..a900f016b9e7 100644 > --- a/tools/testing/selftests/net/udpgso_bench_tx.c > +++ b/tools/testing/selftests/net/udpgso_bench_tx.c > @@ -5,6 +5,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -19,6 +21,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -34,6 +37,10 @@ > #define SO_ZEROCOPY 60 > #endif > > +#ifndef SO_EE_ORIGIN_ZEROCOPY > +#define SO_EE_ORIGIN_ZEROCOPY 5 > +#endif > + > #ifndef MSG_ZEROCOPY > #define MSG_ZEROCOPY 0x4000000 > #endif > @@ -48,9 +55,14 @@ static uint16_t cfg_mss; > static int cfg_payload_len = (1472 * 42); > static int cfg_port = 8000; > static int cfg_runtime_ms = -1; > +static bool cfg_poll; > static bool cfg_segment; > static bool cfg_sendmmsg; > static bool cfg_tcp; > +static uint32_t cfg_tx_ts = SOF_TIMESTAMPING_TX_SOFTWARE; > +static bool cfg_tx_tstamp; > +static uint32_t cfg_tos; > +static bool cfg_verbose; > static bool cfg_zerocopy; > static int cfg_msg_nr; > static uint16_t cfg_gso_size; > @@ -58,6 +70,10 @@ static uint16_t cfg_gso_size; > static socklen_t cfg_alen; > static struct sockaddr_storage cfg_dst_addr; > > +struct my_scm_timestamping { > + struct timespec ts[3]; > +}; > + This and the above should not be needed if including It may be absent if relying on the host header files, but the kselftest build system should correctly use the files from the kernel source tree. > static bool interrupted; > static char buf[NUM_PKT][ETH_MAX_MTU]; > > @@ -89,20 +105,20 @@ static int set_cpu(int cpu) > > static void setup_sockaddr(int domain, const char *str_addr, void *sockaddr) > { > - struct sockaddr_in6 *addr6 = (void *) sockaddr; > - struct sockaddr_in *addr4 = (void *) sockaddr; > + struct sockaddr_in6 *addr6 = (void *)sockaddr; > + struct sockaddr_in *addr4 = (void *)sockaddr; > > switch (domain) { > case PF_INET: > addr4->sin_family = AF_INET; > addr4->sin_port = htons(cfg_port); > - if (inet_pton(AF_INET, str_addr, &(addr4->sin_addr)) != 1) > + if (inet_pton(AF_INET, str_addr, &addr4->sin_addr) != 1) > error(1, 0, "ipv4 parse error: %s", str_addr); > break; > case PF_INET6: > addr6->sin6_family = AF_INET6; > addr6->sin6_port = htons(cfg_port); > - if (inet_pton(AF_INET6, str_addr, &(addr6->sin6_addr)) != 1) > + if (inet_pton(AF_INET6, str_addr, &addr6->sin6_addr) != 1) Please do not include style changes like these. Try to minimize changes required to add the new feature. From mboxrd@z Thu Jan 1 00:00:00 1970 From: willemdebruijn.kernel at gmail.com (Willem de Bruijn) Date: Thu, 23 May 2019 17:45:49 -0400 Subject: [PATCH net 2/4] net/udpgso_bench_tx: options to exercise TX CMSG In-Reply-To: <20190523210651.80902-3-fklassen@appneta.com> References: <20190523210651.80902-1-fklassen@appneta.com> <20190523210651.80902-3-fklassen@appneta.com> Message-ID: On Thu, May 23, 2019 at 5:11 PM Fred Klassen wrote: > > This enhancement adds options that facilitate load testing with > additional TX CMSG options, and to optionally print results of > various send CMSG operations. > > These options are especially useful in isolating situations > where error-queue messages are lost when combined with other > CMSG operations (e.g. SO_ZEROCOPY). > > New options: > > -T - add TX CMSG that requests TX software timestamps > -H - similar to -T except request TX hardware timestamps > -q - add IP_TOS/IPV6_TCLASS TX CMSG > -P - call poll() before reading error queue > -v - print detailed results > > Fixes: 3a687bef148d ("selftests: udp gso benchmark") This is not a fix, but an extension. Fixes tags help with backporting to stable kernels. There is something to be said to backport the main change and support SO_TIMESTAMPING + UDP_GSO on older kernels, especially since it is very concise. But the tests should probably be in a separate patch set targeting net-next. > Signed-off-by: Fred Klassen > --- > tools/testing/selftests/net/udpgso_bench_tx.c | 290 ++++++++++++++++++++++++-- > 1 file changed, 273 insertions(+), 17 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c > index 4074538b5df5..a900f016b9e7 100644 > --- a/tools/testing/selftests/net/udpgso_bench_tx.c > +++ b/tools/testing/selftests/net/udpgso_bench_tx.c > @@ -5,6 +5,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -19,6 +21,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -34,6 +37,10 @@ > #define SO_ZEROCOPY 60 > #endif > > +#ifndef SO_EE_ORIGIN_ZEROCOPY > +#define SO_EE_ORIGIN_ZEROCOPY 5 > +#endif > + > #ifndef MSG_ZEROCOPY > #define MSG_ZEROCOPY 0x4000000 > #endif > @@ -48,9 +55,14 @@ static uint16_t cfg_mss; > static int cfg_payload_len = (1472 * 42); > static int cfg_port = 8000; > static int cfg_runtime_ms = -1; > +static bool cfg_poll; > static bool cfg_segment; > static bool cfg_sendmmsg; > static bool cfg_tcp; > +static uint32_t cfg_tx_ts = SOF_TIMESTAMPING_TX_SOFTWARE; > +static bool cfg_tx_tstamp; > +static uint32_t cfg_tos; > +static bool cfg_verbose; > static bool cfg_zerocopy; > static int cfg_msg_nr; > static uint16_t cfg_gso_size; > @@ -58,6 +70,10 @@ static uint16_t cfg_gso_size; > static socklen_t cfg_alen; > static struct sockaddr_storage cfg_dst_addr; > > +struct my_scm_timestamping { > + struct timespec ts[3]; > +}; > + This and the above should not be needed if including It may be absent if relying on the host header files, but the kselftest build system should correctly use the files from the kernel source tree. > static bool interrupted; > static char buf[NUM_PKT][ETH_MAX_MTU]; > > @@ -89,20 +105,20 @@ static int set_cpu(int cpu) > > static void setup_sockaddr(int domain, const char *str_addr, void *sockaddr) > { > - struct sockaddr_in6 *addr6 = (void *) sockaddr; > - struct sockaddr_in *addr4 = (void *) sockaddr; > + struct sockaddr_in6 *addr6 = (void *)sockaddr; > + struct sockaddr_in *addr4 = (void *)sockaddr; > > switch (domain) { > case PF_INET: > addr4->sin_family = AF_INET; > addr4->sin_port = htons(cfg_port); > - if (inet_pton(AF_INET, str_addr, &(addr4->sin_addr)) != 1) > + if (inet_pton(AF_INET, str_addr, &addr4->sin_addr) != 1) > error(1, 0, "ipv4 parse error: %s", str_addr); > break; > case PF_INET6: > addr6->sin6_family = AF_INET6; > addr6->sin6_port = htons(cfg_port); > - if (inet_pton(AF_INET6, str_addr, &(addr6->sin6_addr)) != 1) > + if (inet_pton(AF_INET6, str_addr, &addr6->sin6_addr) != 1) Please do not include style changes like these. Try to minimize changes required to add the new feature. From mboxrd@z Thu Jan 1 00:00:00 1970 From: willemdebruijn.kernel@gmail.com (Willem de Bruijn) Date: Thu, 23 May 2019 17:45:49 -0400 Subject: [PATCH net 2/4] net/udpgso_bench_tx: options to exercise TX CMSG In-Reply-To: <20190523210651.80902-3-fklassen@appneta.com> References: <20190523210651.80902-1-fklassen@appneta.com> <20190523210651.80902-3-fklassen@appneta.com> Message-ID: Content-Type: text/plain; charset="UTF-8" Message-ID: <20190523214549.vD_2FOSswvJ1_el8XPX4QvyJ0TnJjQU6LNirsyPqmK4@z> On Thu, May 23, 2019@5:11 PM Fred Klassen wrote: > > This enhancement adds options that facilitate load testing with > additional TX CMSG options, and to optionally print results of > various send CMSG operations. > > These options are especially useful in isolating situations > where error-queue messages are lost when combined with other > CMSG operations (e.g. SO_ZEROCOPY). > > New options: > > -T - add TX CMSG that requests TX software timestamps > -H - similar to -T except request TX hardware timestamps > -q - add IP_TOS/IPV6_TCLASS TX CMSG > -P - call poll() before reading error queue > -v - print detailed results > > Fixes: 3a687bef148d ("selftests: udp gso benchmark") This is not a fix, but an extension. Fixes tags help with backporting to stable kernels. There is something to be said to backport the main change and support SO_TIMESTAMPING + UDP_GSO on older kernels, especially since it is very concise. But the tests should probably be in a separate patch set targeting net-next. > Signed-off-by: Fred Klassen > --- > tools/testing/selftests/net/udpgso_bench_tx.c | 290 ++++++++++++++++++++++++-- > 1 file changed, 273 insertions(+), 17 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c > index 4074538b5df5..a900f016b9e7 100644 > --- a/tools/testing/selftests/net/udpgso_bench_tx.c > +++ b/tools/testing/selftests/net/udpgso_bench_tx.c > @@ -5,6 +5,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -19,6 +21,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -34,6 +37,10 @@ > #define SO_ZEROCOPY 60 > #endif > > +#ifndef SO_EE_ORIGIN_ZEROCOPY > +#define SO_EE_ORIGIN_ZEROCOPY 5 > +#endif > + > #ifndef MSG_ZEROCOPY > #define MSG_ZEROCOPY 0x4000000 > #endif > @@ -48,9 +55,14 @@ static uint16_t cfg_mss; > static int cfg_payload_len = (1472 * 42); > static int cfg_port = 8000; > static int cfg_runtime_ms = -1; > +static bool cfg_poll; > static bool cfg_segment; > static bool cfg_sendmmsg; > static bool cfg_tcp; > +static uint32_t cfg_tx_ts = SOF_TIMESTAMPING_TX_SOFTWARE; > +static bool cfg_tx_tstamp; > +static uint32_t cfg_tos; > +static bool cfg_verbose; > static bool cfg_zerocopy; > static int cfg_msg_nr; > static uint16_t cfg_gso_size; > @@ -58,6 +70,10 @@ static uint16_t cfg_gso_size; > static socklen_t cfg_alen; > static struct sockaddr_storage cfg_dst_addr; > > +struct my_scm_timestamping { > + struct timespec ts[3]; > +}; > + This and the above should not be needed if including It may be absent if relying on the host header files, but the kselftest build system should correctly use the files from the kernel source tree. > static bool interrupted; > static char buf[NUM_PKT][ETH_MAX_MTU]; > > @@ -89,20 +105,20 @@ static int set_cpu(int cpu) > > static void setup_sockaddr(int domain, const char *str_addr, void *sockaddr) > { > - struct sockaddr_in6 *addr6 = (void *) sockaddr; > - struct sockaddr_in *addr4 = (void *) sockaddr; > + struct sockaddr_in6 *addr6 = (void *)sockaddr; > + struct sockaddr_in *addr4 = (void *)sockaddr; > > switch (domain) { > case PF_INET: > addr4->sin_family = AF_INET; > addr4->sin_port = htons(cfg_port); > - if (inet_pton(AF_INET, str_addr, &(addr4->sin_addr)) != 1) > + if (inet_pton(AF_INET, str_addr, &addr4->sin_addr) != 1) > error(1, 0, "ipv4 parse error: %s", str_addr); > break; > case PF_INET6: > addr6->sin6_family = AF_INET6; > addr6->sin6_port = htons(cfg_port); > - if (inet_pton(AF_INET6, str_addr, &(addr6->sin6_addr)) != 1) > + if (inet_pton(AF_INET6, str_addr, &addr6->sin6_addr) != 1) Please do not include style changes like these. Try to minimize changes required to add the new feature.