[selftest] udpgro test report fail but passed

[selftest] udpgro test report fail but passed

[Hangbin Liu]

Hi Paolo,

In our recently internal testing, the udpgro.sh test reports failed but it
still return 0 as passed. e.g.

```
ipv6
 no GRO                                  ok
 no GRO chk cmsg                         ok
 GRO                                     ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520

failed
 GRO chk cmsg                            ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520

failed
 GRO with custom segment size            ./udpgso_bench_rx: recv: bad packet len, got 500, expected 14520

failed
 GRO with custom segment size cmsg       ./udpgso_bench_rx: recv: bad packet len, got 500, expected 14520

failed
 bad GRO lookup                          ok
 multiple GRO socks                      ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520

./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520

failed
```

For run_one_2sock testing, I saw

```
        ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 &
        ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \
                echo "ok" || \
                echo "failed" &
        ...
        ./udpgso_bench_tx ${tx_args}
        ret=$?
        wait $(jobs -p)
        return $ret
```

So what's the effect if it echo "failed" while ret == 0?

Thanks
Hangbin

Re: [selftest] udpgro test report fail but passed

[Paolo Abeni]

On 8/13/24 05:29, Hangbin Liu wrote:
> In our recently internal testing, the udpgro.sh test reports failed but it
> still return 0 as passed. e.g.
> 
> ```
> ipv6
>   no GRO                                  ok
>   no GRO chk cmsg                         ok
>   GRO                                     ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
> 
> failed
>   GRO chk cmsg                            ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
> 
> failed
>   GRO with custom segment size            ./udpgso_bench_rx: recv: bad packet len, got 500, expected 14520
> 
> failed
>   GRO with custom segment size cmsg       ./udpgso_bench_rx: recv: bad packet len, got 500, expected 14520
> 
> failed
>   bad GRO lookup                          ok
>   multiple GRO socks                      ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
> 
> ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
> 
> failed
> ```
> 
> For run_one_2sock testing, I saw
> 
> ```
>          ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 &
>          ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \
>                  echo "ok" || \
>                  echo "failed" &
>          ...
>          ./udpgso_bench_tx ${tx_args}
>          ret=$?
>          wait $(jobs -p)
>          return $ret
> ```
> 
> So what's the effect if it echo "failed" while ret == 0?

It's just pour integration between the script and the selftests harness.

The script should capture the pid of the background UDP receiver, wait 
poll for a bit for such process termination after that the sender 
completes, then send a termination signal, capture the receiver exit 
code and use it to emit the success/fail message and update the script 
return code.

Could you please have a shot at the above?

BTW I sometimes observed similar failures in the past when the bpf 
program failed to load.

Cheers,

Paolo

Re: [selftest] udpgro test report fail but passed

[Hangbin Liu]

On Tue, Aug 13, 2024 at 10:24:34AM +0200, Paolo Abeni wrote:
> It's just pour integration between the script and the selftests harness.
> 
> The script should capture the pid of the background UDP receiver, wait poll
> for a bit for such process termination after that the sender completes, then
> send a termination signal, capture the receiver exit code and use it to emit
> the success/fail message and update the script return code.

If that's the case, we shouldn't echo the result as the return value will
always be 0. Is the following change you want? e.g.

@@ -115,16 +113,14 @@ run_one_2sock() {
 	cfg_veth
 
 	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 &
-	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \
-		echo "ok" || \
-		echo "failed" &
+	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} &
 
 	wait_local_port_listen "${PEER_NS}" 12345 udp
 	./udpgso_bench_tx ${tx_args} -p 12345
 	wait_local_port_listen "${PEER_NS}" 8000 udp
 	./udpgso_bench_tx ${tx_args}
-	ret=$?
 	wait $(jobs -p)
+	ret=$?
 	return $ret
 }

> 
> Could you please have a shot at the above?
> 
> BTW I sometimes observed similar failures in the past when the bpf program
> failed to load.

From my log I didn't see bpf load failure..

Thanks
Hangbin

Re: [selftest] udpgro test report fail but passed

[Paolo Abeni]

On 8/13/24 11:57, Hangbin Liu wrote:
> On Tue, Aug 13, 2024 at 10:24:34AM +0200, Paolo Abeni wrote:
>> It's just pour integration between the script and the selftests harness.
>>
>> The script should capture the pid of the background UDP receiver, wait poll
>> for a bit for such process termination after that the sender completes, then
>> send a termination signal, capture the receiver exit code and use it to emit
>> the success/fail message and update the script return code.
> 
> If that's the case, we shouldn't echo the result as the return value will
> always be 0. Is the following change you want? e.g.

The below example captures a slightly more complex case, where there 2 
sender/receiver pair.
> 
> @@ -115,16 +113,14 @@ run_one_2sock() {
>   	cfg_veth
>   
>   	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 &
> -	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \
> -		echo "ok" || \
> -		echo "failed" &
> +	ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} &
>   
>   	wait_local_port_listen "${PEER_NS}" 12345 udp
>   	./udpgso_bench_tx ${tx_args} -p 12345
>   	wait_local_port_listen "${PEER_NS}" 8000 udp
>   	./udpgso_bench_tx ${tx_args}
> -	ret=$?
>   	wait $(jobs -p)
> +	ret=$?
>   	return $ret
>   }
> 
>>
>> Could you please have a shot at the above?

I think it will not be enough. We need to check the exit code of all the 
involved processes (2 senders, 2 receivers)

Thanks!

Paolo