On 12/15/25 2:57 PM, David Marchand wrote:
> Define macros to limit copy/paste (those will be reused later).
>
> To ease test debugging, store composed and expected good/bad packets
> in the unit test working directory, and pass the file names.
>
> Example:
> $ make -C build check TESTSUITEFLAGS="1098 -d -v"
> ...
> ../../tests/dpif-netdev.at:808:
> ovs-vsctl set interface p1 options:ol_ip_rx_csum_set_good=true
> ../../tests/dpif-netdev.at:808:
> ovs-appctl netdev-dummy/receive p1 $(cat good_frame)
> ../../tests/dpif-netdev.at:808:
> ovs-pcap p2.pcap > p2.pcap.txt 2>&1
> ../../tests/dpif-netdev.at:808:
> tail -n 1 p2.pcap.txt
> ../../tests/dpif-netdev.at:808:
> ovs-vsctl set interface p1 options:ol_ip_rx_csum_set_good=false
> ...
>
> Signed-off-by: David Marchand <[email protected]>
> Acked-by: Kevin Traynor <[email protected]>
> ---
> tests/dpif-netdev.at | 970 ++++++++-----------------------------------
> 1 file changed, 175 insertions(+), 795 deletions(-)
>
> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> index 90d7497664..bbce1ccee4 100644
> --- a/tests/dpif-netdev.at
> +++ b/tests/dpif-netdev.at
> @@ -736,6 +736,43 @@ AT_CHECK([test `ovs-vsctl get Interface p2
> statistics:tx_q0_packets` -gt 0 -a dn
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> +m4_define([CHECK_FWD_PACKET],
> + [test -z "$3" || AT_CHECK([ovs-vsctl set interface $1 options:$3=true])
> + AT_CHECK([ovs-appctl netdev-dummy/receive $1 $(cat $4)])
> + test "$5" = 'none' || {
> + AT_CHECK([ovs-pcap $2.pcap > $2.pcap.txt 2>&1])
> + AT_CHECK_UNQUOTED([tail -n 1 $2.pcap.txt], [0], [$(cat $5)
> +])
> + }
> + test -z "$3" || AT_CHECK([ovs-vsctl remove interface $1 options $3])
> +])
We shouldnt mix the shell and macro checks here. It makes autotest generate
unnecessary code and makes it harder to userstand which variables are checked
at runtime and which at compile time. All the arguments here are macro
arguments that are known at compilation time and so the m4_if macro should
be used to check them. And we should probbaly use expout instead of the
unquoted check, i.e.:
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index c41c1e18d..955b0a51c 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -737,14 +737,14 @@ OVS_VSWITCHD_STOP
AT_CLEANUP
m4_define([CHECK_FWD_PACKET],
- [test -z "$3" || AT_CHECK([ovs-vsctl set interface $1 options:$3=true])
+ [m4_if([$3], [], [], [AT_CHECK([ovs-vsctl set interface $1
options:$3=true])])
AT_CHECK([ovs-appctl netdev-dummy/receive $1 $(cat $4)])
- test "$5" = 'none' || {
- AT_CHECK([ovs-pcap $2.pcap > $2.pcap.txt 2>&1])
- AT_CHECK_UNQUOTED([tail -n 1 $2.pcap.txt], [0], [$(cat $5)
-])
- }
- test -z "$3" || AT_CHECK([ovs-vsctl remove interface $1 options $3])
+ m4_if([$5], [none], [], [
+ AT_CHECK([ovs-pcap $2.pcap > $2.pcap.txt 2>&1])
+ AT_CHECK([awk 1 $5 > expout]) dnl Also normalizes the line endings.
+ AT_CHECK([tail -n 1 $2.pcap.txt], [0], [expout])
+ ])
+ m4_if([$3], [], [], [AT_CHECK([ovs-vsctl remove interface $1 options $3])])
])
dnl CHECK_IP_CHECKSUMS rx_port tx_port good_pkt bad_pkt good_exp bad_exp
---
> +
> +dnl CHECK_IP_CHECKSUMS rx_port tx_port good_pkt bad_pkt good_exp bad_exp
> +dnl
> +dnl Test combinations of Rx IP checksum flags for a good or bad packet
> +dnl received on rx_port, and sent over tx_port.
> +m4_define([CHECK_IP_CHECKSUMS],
> + [dnl Checks for good packet.
> + dnl No Rx flag
> + CHECK_FWD_PACKET($1, $2, , $3, $5)
> + dnl Rx good
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_good, $3, $5)
> + dnl Rx bad
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_bad, $3, $5)
> + dnl Rx partial
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_partial, $3, $5)
> +
> + dnl Checks for bad packet.
> + dnl No Rx flag
> + CHECK_FWD_PACKET($1, $2, , $4, $6)
> + dnl Rx good
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_good, $4, $5)
> + dnl Rx bad
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_bad, $4, $6)
> + dnl Rx partial
> + CHECK_FWD_PACKET($1, $2, ol_ip_rx_csum_set_partial, $4, $5)
> +])
> +
> AT_SETUP([dpif-netdev - ip csum offload])
> AT_KEYWORDS([userspace offload])
> OVS_VSWITCHD_START(
> @@ -747,6 +784,8 @@ OVS_VSWITCHD_START(
>
> AT_CHECK([ovs-appctl vlog/set netdev_dummy:file:dbg])
>
> +AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
> +
> dnl Modify the ip_dst addr to force changing the IP csum.
> AT_CHECK([ovs-ofctl add-flow br1
> in_port=p1,actions=mod_nw_dst:192.168.1.1,output:p2])
>
> @@ -755,160 +794,33 @@ flow_s="\
> tcp,ip_src=192.168.123.2,ip_dst=192.168.123.1,ip_frag=no,\
> tcp_src=54392,tcp_dst=5201,tcp_flags=ack"
>
> -good_frame=$(ovs-ofctl compose-packet --bare "${flow_s}")
> +ovs-ofctl compose-packet --bare "${flow_s}" > good_frame
It wasn't possible to check the command before, but now it should be
wrapped into AT_CHECK, so we have a trace of it in the logs and we're
sure that it doesn't fail silently.
> -bad_frame=$(echo $good_frame | sed -e "s/6b72/dead/")
> +cat good_frame | sed -e "s/6b72/dead/" > bad_frame
> dnl 0x6b72 + (5201-2222) == 0x7715
> dnl 0xdead + (5201-2222) == 0xea50
> -bad_expected=$(echo $good_expected | sed -e "s/7715/ea50/")
> +cat good_expected | sed -e "s/7715/ea50/" > bad_expected
These cat / sed commands should also be checked. We can't check all of
them easily, but we should check ones that we can.
Both checks can be easily added with a simple substitution in vim:
:%s/^ovs-ofctl.*/AT_CHECK([\0])/gc
:%s/^cat .*/AT_CHECK([\0])/gc
Since all that mentioned above are just minor test changes, If you agree,
I can make them on commit. WDYT?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
