Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 17 Apr 2023 11:27:51 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To:     Yang Yang <yang.yang29@zte.com.cn>, davem@davemloft.net,
        edumazet@google.com, willemdebruijn.kernel@gmail.com
Cc:     yang.yang29@zte.com.cn, kuba@kernel.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        netdev@vger.kernel.org, pabeni@redhat.com, shuah@kernel.org,
        zhang.yunkai@zte.com.cn, xu.xin16@zte.com.cn,
        Xuexin Jiang <jiang.xuexin@zte.com.cn>
Message-ID: <643d6577e81f1_29b64929414@willemb.c.googlers.com.notmuch>
In-Reply-To: <20230417122504.193350-1-yang.yang29@zte.com.cn>
References: <202304172017351308785@zte.com.cn>
 <20230417122504.193350-1-yang.yang29@zte.com.cn>
Subject: RE: [PATCH linux-next 3/3] selftests: net: udpgso_bench_rx: Fix
 packet number exceptions
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Yang Yang wrote:
> From: Zhang Yunkai (CGEL ZTE) <zhang.yunkai@zte.com.cn>
> 
> The -n parameter is confusing and seems to only affect the frequency of
> determining whether the time reaches 1s.

This statement seems irrelevant to this patch.

Is the point that cfg_expected_pkt_nr is tested in do_flush_udp to
stop reading, but not in the caller of that function, do_recv, to
break out of the loop? That's a fair point and may deserve a fix.

> However, the final print of the
> program is the number of messages expected to be received, which is always
> 0.
> 
> bash# udpgso_bench_rx -4 -n 100
> bash# udpgso_bench_tx -l 1 -4 -D "$DST"
> udpgso_bench_rx: wrong packet number! got 0, expected 100
> 
> This is because the packets are always cleared after print.

This looks good to me. Would be a fix to the commit that introduced
that wrong packet number branch, commit 3327a9c46352
("selftests: add functionals test for UDP GRO").
> 
> Signed-off-by: Zhang Yunkai (CGEL ZTE) <zhang.yunkai@zte.com.cn>
> Reviewed-by: xu xin (CGEL ZTE) <xu.xin16@zte.com.cn>
> Reviewed-by: Yang Yang (CGEL ZTE) <yang.yang29@zte.com.cn>
> Cc: Xuexin Jiang (CGEL ZTE) <jiang.xuexin@zte.com.cn>
> ---
>  tools/testing/selftests/net/udpgso_bench_rx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c
> index 784e88b31f7d..b66bb53af19f 100644
> --- a/tools/testing/selftests/net/udpgso_bench_rx.c
> +++ b/tools/testing/selftests/net/udpgso_bench_rx.c
> @@ -50,7 +50,7 @@ static int  cfg_rcv_timeout_ms;
>  static struct sockaddr_storage cfg_bind_addr;
>  
>  static bool interrupted;
> -static unsigned long packets, bytes;
> +static unsigned long packets, total_packets, bytes;
>  
>  static void sigint_handler(int signum)
>  {
> @@ -405,6 +405,7 @@ static void do_recv(void)
>  					"%s rx: %6lu MB/s %8lu calls/s\n",
>  					cfg_tcp ? "tcp" : "udp",
>  					bytes >> 20, packets);
> +			total_packets += packets;
>  			bytes = packets = 0;
>  			treport = tnow + 1000;
>  		}
> @@ -415,7 +416,7 @@ static void do_recv(void)
>  
>  	if (cfg_expected_pkt_nr && (packets != cfg_expected_pkt_nr))
>  		error(1, 0, "wrong packet number! got %ld, expected %d\n",
> -		      packets, cfg_expected_pkt_nr);
> +		      total_packets + packets, cfg_expected_pkt_nr);
>  
>  	if (close(fd))
>  		error(1, errno, "close");
> -- 
> 2.15.2