[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