[ovs-dev] [PATCH ovn 01/12] tests: Improve "dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS".
This change makes the test much more debuggable:
- Make it easier to find the particular failing test.
- Use correct checksums so that the packets can be compared for
equality instead of omitting the checksum field.
- Factor out packet comparison and produce better error messages on
failure.
Signed-off-by: Ben Pfaff
---
tests/network-functions.at | 18
tests/ovn.at | 184 -
2 files changed, 75 insertions(+), 127 deletions(-)
diff --git a/tests/network-functions.at b/tests/network-functions.at
index a149e9da4c58..c583bc31e881 100644
--- a/tests/network-functions.at
+++ b/tests/network-functions.at
@@ -52,6 +52,24 @@ test_csum 45764001c3d90a03aca80003
AT_CLEANUP
OVS_START_SHELL_HELPERS
+# ip4_csum_inplace IP4_HEADER
+#
+# Outputs IP4_HEADER with the checksum filled in properly.
+# The checksum must initially be . IP4_HEADER must be
+# 40 hex digits.
+ip4_csum_inplace() {
+local csum=$(ip_csum $1)
+echo "$1" | sed "s/^\(\)/\1$csum/"
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([ip4_csum_inplace])
+AT_CHECK([ip4_csum_inplace 457340004011c0a80001c0a800c7], [0],
+[457340004011b861c0a80001c0a800c7
+])
+AT_CLEANUP
+
+OVS_START_SHELL_HELPERS
# ip6_pseudoheader IP6_HEADER NEXT_HEADER PAYLOAD_LEN
#
# where:
diff --git a/tests/ovn.at b/tests/ovn.at
index 4be484051966..5666058d99ca 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5294,7 +5294,12 @@ sleep 2
as hv1 ovs-vsctl show
# This shell function sends a DHCP request packet
-# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP REQUEST_IP
ETH_BOOT USE_IP ...
+#
+# The first argument is just the number of calls to this function so
+# far (1, 2, ...). This is redundant, but it makes it easier to find
+# the failures by searching for the number.
+#
+# test_dhcp PACKET_NUM INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP
REQUEST_IP ETH_BOOT USE_IP ...
packet_num=0
test_dhcp() {
local expect_resume=:
@@ -5308,6 +5313,10 @@ test_dhcp() {
esac
done
+packet_num=$(expr $packet_num + 1)
+AT_FAIL_IF([test $packet_num != $1])
+shift
+
local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 ciaddr=$5 offer_ip=$6
request_ip=$7 eth_boot=$8 use_ip=$9
shift; shift; shift; shift; shift; shift; shift; shift; shift;
@@ -5319,7 +5328,6 @@ test_dhcp() {
src_ip=`ip_to_hex 0 0 0 0`
dst_ip=`ip_to_hex 255 255 255 255`
fi
-packet_num=$(expr $packet_num + 1)
AS_BOX([dhcp test packet $packet_num])
@@ -5415,7 +5423,9 @@ test_dhcp() {
ip_len=$(printf "%x" $ip_len)
udp_len=$(printf "%x" $udp_len)
# $ip_len var will be in 3 digits i.e 134. So adding a '0' before
$ip_len
-local
reply=${src_mac}${srv_mac}080045100${ip_len}8011${srv_ip}${reply_dst_ip}
+local reply=${src_mac}${srv_mac}0800
+local ip_header=45100${ip_len}8011${srv_ip}${reply_dst_ip}
+reply=${reply}$(ip4_csum_inplace $ip_header)
# udp header and dhcp header.
# $udp_len var will be in 3 digits. So adding a '0' before $udp_len
reply=${reply}004300440${udp_len}020106006359aa76${flags}${ciaddr}
@@ -5460,6 +5470,16 @@ test_dhcp() {
fi
}
+compare_dhcp_packets() {
+received=$($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif$1-tx.pcap)
+expected=$(cat $1.expected)
+
+if test "$received" != "$expected"; then
+AT_CHECK_UNQUOTED([echo "$received"; tcpdump_hex "$received"], [0],
+[$(echo "$expected"; tcpdump_hex "$expected")])
+fi
+}
+
reset_pcap_file() {
local iface=$1
local pcap_file=$2
@@ -5481,14 +5501,8 @@ server_ip=`ip_to_hex 10 0 0 1`
ciaddr=`ip_to_hex 0 0 0 0`
request_ip=0
expected_dhcp_opts=33040e100104ff0003040a0136040a01
-test_dhcp 1 f001 01 0 $ciaddr $offer_ip $request_ip 0 0 ff11
$server_ip 02 $expected_dhcp_opts
-
-$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
-cat 1.expected | cut -c -48 > expout
-AT_CHECK([cat 1.packets | cut -c -48], [0], [expout])
-# Skipping the IPv4 checksum.
-cat 1.expected | cut -c 53- > expout
-AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout])
+test_dhcp 1 1 f001 01 0 $ciaddr $offer_ip $request_ip 0 0 ff11
$server_ip 02 $expected_dhcp_opts
+compare_dhcp_packets 1
# --
@@ -5506,14 +5520,8 @@ server_ip=`ip_to_hex 10 0 0 1`
ciaddr=`ip_to_hex 0 0 0 0`
request_ip=$offer_ip
expected_dhcp_opts=33040e100104ff0003040a0136040a01
-test_dhcp 2 f002 03 0 $ciaddr $offer_ip $request_ip 0 0 ff11
$server_ip 05 $expected_dhcp_opts
-
-$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
-cat 2.expected | cut -c -48 > expout
-AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
-# Skipping the IPv4
Re: [ovs-dev] [PATCH ovn 01/12] tests: Improve "dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS".
On Wed, Nov 4, 2020 at 12:33 PM Ben Pfaff wrote:
>
> This change makes the test much more debuggable:
>
> - Make it easier to find the particular failing test.
>
> - Use correct checksums so that the packets can be compared for
> equality instead of omitting the checksum field.
>
> - Factor out packet comparison and produce better error messages on
> failure.
>
> Signed-off-by: Ben Pfaff
Thanks for the series. Patches 1 to 6 of this series were
straightforward. So I applied to the main branch.
Thanks
Numan
> ---
> tests/network-functions.at | 18
> tests/ovn.at | 184 -
> 2 files changed, 75 insertions(+), 127 deletions(-)
>
> diff --git a/tests/network-functions.at b/tests/network-functions.at
> index a149e9da4c58..c583bc31e881 100644
> --- a/tests/network-functions.at
> +++ b/tests/network-functions.at
> @@ -52,6 +52,24 @@ test_csum 45764001c3d90a03aca80003
> AT_CLEANUP
>
> OVS_START_SHELL_HELPERS
> +# ip4_csum_inplace IP4_HEADER
> +#
> +# Outputs IP4_HEADER with the checksum filled in properly.
> +# The checksum must initially be . IP4_HEADER must be
> +# 40 hex digits.
> +ip4_csum_inplace() {
> +local csum=$(ip_csum $1)
> +echo "$1" | sed "s/^\(\)/\1$csum/"
> +}
> +OVS_END_SHELL_HELPERS
> +
> +AT_SETUP([ip4_csum_inplace])
> +AT_CHECK([ip4_csum_inplace 457340004011c0a80001c0a800c7], [0],
> +[457340004011b861c0a80001c0a800c7
> +])
> +AT_CLEANUP
> +
> +OVS_START_SHELL_HELPERS
> # ip6_pseudoheader IP6_HEADER NEXT_HEADER PAYLOAD_LEN
> #
> # where:
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4be484051966..5666058d99ca 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5294,7 +5294,12 @@ sleep 2
> as hv1 ovs-vsctl show
>
> # This shell function sends a DHCP request packet
> -# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP REQUEST_IP
> ETH_BOOT USE_IP ...
> +#
> +# The first argument is just the number of calls to this function so
> +# far (1, 2, ...). This is redundant, but it makes it easier to find
> +# the failures by searching for the number.
> +#
> +# test_dhcp PACKET_NUM INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP
> REQUEST_IP ETH_BOOT USE_IP ...
> packet_num=0
> test_dhcp() {
> local expect_resume=:
> @@ -5308,6 +5313,10 @@ test_dhcp() {
> esac
> done
>
> +packet_num=$(expr $packet_num + 1)
> +AT_FAIL_IF([test $packet_num != $1])
> +shift
> +
> local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 ciaddr=$5
> offer_ip=$6 request_ip=$7 eth_boot=$8 use_ip=$9
> shift; shift; shift; shift; shift; shift; shift; shift; shift;
>
> @@ -5319,7 +5328,6 @@ test_dhcp() {
> src_ip=`ip_to_hex 0 0 0 0`
> dst_ip=`ip_to_hex 255 255 255 255`
> fi
> -packet_num=$(expr $packet_num + 1)
>
> AS_BOX([dhcp test packet $packet_num])
>
> @@ -5415,7 +5423,9 @@ test_dhcp() {
> ip_len=$(printf "%x" $ip_len)
> udp_len=$(printf "%x" $udp_len)
> # $ip_len var will be in 3 digits i.e 134. So adding a '0' before
> $ip_len
> -local
> reply=${src_mac}${srv_mac}080045100${ip_len}8011${srv_ip}${reply_dst_ip}
> +local reply=${src_mac}${srv_mac}0800
> +local
> ip_header=45100${ip_len}8011${srv_ip}${reply_dst_ip}
> +reply=${reply}$(ip4_csum_inplace $ip_header)
> # udp header and dhcp header.
> # $udp_len var will be in 3 digits. So adding a '0' before $udp_len
>
> reply=${reply}004300440${udp_len}020106006359aa76${flags}${ciaddr}
> @@ -5460,6 +5470,16 @@ test_dhcp() {
> fi
> }
>
> +compare_dhcp_packets() {
> +received=$($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif$1-tx.pcap)
> +expected=$(cat $1.expected)
> +
> +if test "$received" != "$expected"; then
> +AT_CHECK_UNQUOTED([echo "$received"; tcpdump_hex "$received"], [0],
> +[$(echo "$expected"; tcpdump_hex "$expected")])
> +fi
> +}
> +
> reset_pcap_file() {
> local iface=$1
> local pcap_file=$2
> @@ -5481,14 +5501,8 @@ server_ip=`ip_to_hex 10 0 0 1`
> ciaddr=`ip_to_hex 0 0 0 0`
> request_ip=0
> expected_dhcp_opts=33040e100104ff0003040a0136040a01
> -test_dhcp 1 f001 01 0 $ciaddr $offer_ip $request_ip 0 0 ff11
> $server_ip 02 $expected_dhcp_opts
> -
> -$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
> -cat 1.expected | cut -c -48 > expout
> -AT_CHECK([cat 1.packets | cut -c -48], [0], [expout])
> -# Skipping the IPv4 checksum.
> -cat 1.expected | cut -c 53- > expout
> -AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout])
> +test_dhcp 1 1 f001 01 0 $ciaddr $offer_ip $request_ip 0 0
> ff11 $server_ip 02 $expected_dhcp_opts
> +compare_dhcp_packets 1
>
> # --
>
> @@ -5506,14 +5520,8 @@ server_ip=`ip_to_hex 10 0 0
Re: [ovs-dev] [PATCH ovn 01/12] tests: Improve "dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS".
On Wed, Nov 04, 2020 at 03:36:20PM +0530, Numan Siddique wrote: > On Wed, Nov 4, 2020 at 12:33 PM Ben Pfaff wrote: > > > > This change makes the test much more debuggable: > > > > - Make it easier to find the particular failing test. > > > > - Use correct checksums so that the packets can be compared for > > equality instead of omitting the checksum field. > > > > - Factor out packet comparison and produce better error messages on > > failure. > > > > Signed-off-by: Ben Pfaff > > Thanks for the series. Patches 1 to 6 of this series were > straightforward. So I applied to the main branch. Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
