f12a00d571
Author: David Marchand <[email protected]>
[PATCH v2 05/11] dpif-netdev.at: Add helpers for checksum tests.
This patch defines macros to reduce code duplication in checksum tests by
creating reusable helper functions for testing different checksum
scenarios.
> 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])
> +])
Does this code handle cleanup properly if any of the AT_CHECK calls fail? The
configuration option may be left set in the interface when tests fail, which
could affect subsequent tests.
> +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_l4_rx_csum_set_bad, $4, $6)
Is there a copy-paste error in this line? The macro name is
CHECK_IP_CHECKSUMS but this line uses "ol_l4_rx_csum_set_bad". Should
this be "ol_ip_rx_csum_set_bad"?
[ ... ]
> @@ -794,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
[ ... ]
> @@ -1334,7 +1314,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1
> ${icmp_frame}])
> AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
> AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${icmp_expected}
> ])
> -AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=false])
> +AT_CHECK([ovs-vsctl remove Interface p1 options ol_ip_rx_csum_set_good])
Is this direct use of "ovs-vsctl remove" consistent with the approach
used in the CHECK_FWD_PACKET macro? Should this special case also use
the macro pattern for consistency?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
