[LTP] [RFC PATCH v8 08/11] network/stress: Fix and cleanup route IPv4 tests
Petr Vorel
pvorel@suse.cz
Tue Aug 22 23:22:40 CEST 2017
Hi Alexey,
> > Tests using route_test_change() use background traffic instead of ns-udpsender
> > but I fixed it that it uses route. Is it too much overhead? The reason is I'd
> How do you use it to make a background traffic if you change route on
> every loop iteration?
> > like to get rid of all ugly scripts and binaries in
> > testcases/network/stress/ns-tools, including ns-udpsender.c.
> ns-udpsender can send a single UDP datagram over the new route without
> waiting for answer but unfortunately we don't know for sure whether it
> actually sent datagram and/or destination received it.
What would you suggest? I chose netstress as it's kind of client & server application. Do
we want to write something from scratch?
I don't like also the infinite loop in send_udp_datagram (I've caused one crash in local
network in previous tests by sending too many packets).
> > + if [ "$cmd_type" = 'ifconfig_cmd' ]; then
> > + cmd="$cmd_name $(tst_iface) down"
> > + cmd2="$cmd_name $(tst_iface) up"
> > + else
> > + cmd="$cmd_name link set down dev $(tst_iface)"
> > + cmd2="$cmd_name link set up dev $(tst_iface)"
> This test-case looks like duplicate of if-updown
OK, I'll delete testcases/network/stress/route/route4-ifdown and
testcases/network/stress/route/route6-ifdown. testcases/network/stress/interface/if-updown
looks cleaner anyway.
Anything else to delete? The more time I spend with route tests the more I doubt about
it's real value.
...
> > +# helper function for route4-change-{dst,gw,if} tests
> > +route_test_change()
> > +{
...
> > + while [ $cnt -lt $NS_TIMES ]; do
> > + lhost_ifname2="$lhost_ifname"
> > + gateway2="$gateway"
> > +
> > + pre_dst_network="$dst_network"
> > + if [ "$test_field" = 'dst' ]; then
> > + dst_addr="$(tst_ipaddr_un_ip $(($cnt % 255)) $DST_HOST)"
> > + dst_network="$(tst_ipaddr_un_ip $(($cnt % 255)))"
> tst_ipaddr_un() handles '$cnt' overrun already.
Thanks, silly error.
> > + netstress_cleanup
> cleanup in the test on every loop iteration?
That's used to kill netstress before calling make_background_tcp_traffic.
> > + elif [ "$test_field" = 'gw' ]; then
> > + gateway2="$(tst_ipaddr_un_host rhost $(($RHOST_COUNTER_START + $cnt)))"
> > + local cnt2=$(($cnt + 1))
> > + [ $cnt2 -eq $RHOST_COUNTER_COUNT ] && cnt2=$RHOST_COUNTER_START
> > + gateway="$(tst_ipaddr_un_host rhost $(($RHOST_COUNTER_START + $cnt2)))"
> Why do you need RHOST_COUNTER_COUNT and RHOST_COUNTER_START variables?
I create 10 unused addresses on rhost in route4-change-gw and then I'm using them in the
test. I use it also for RHOST_DST_ADDR (creating unique rhost address in route4-change-gw
used as destination address for check_connectivity_interval).
> > + elif [ "$test_field" = 'if' ]; then
> > + [ $link_num -ge $LINK_TOTAL ] && link_num=0
> > + lhost_ifname="$(tst_iface lhost $link_num)"
> > + gateway="$(tst_ipaddr_un_ip $link_num 1)"
> > + link_num=$(($link_num + 1))
> > + fi
> It's better to change 'if' structure to 'case' one
OK.
> > +
> > + if [ $cnt -gt 0 ]; then
> > + manipulate_route $cmd_name 'del' $pre_dst_network $IPV4_NETMASK_NUM $IPV4_NETMASK $gateway2 $lhost_ifname2
> > + fi
> > +
> > + manipulate_route $cmd_name 'add' $dst_network $IPV4_NETMASK_NUM $IPV4_NETMASK $gateway $lhost_ifname
> > + make_background_tcp_traffic $(tst_ipaddr_un_host)
> For generating traffic you're always using a single destination address,
> why it can't be $(tst_ipaddr)?
I thought it should be also on unused network, but as it's a simple check (not a new
route) I can change it to $(tst_ipaddr).
> > + check_connectivity_interval $cnt false $lhost_ifname $dst_addr || return
> Could you please clarify the traffic flow here? if I understand it
> correctly ICMP goes over the new route/gw to an IP addresses on remote
> host which was added in setup, correct?
Yes.
> I think ns-udpsender is the more appropriate choice for a one way traffic...
OK. But we both don't like it's limitations (no guarantee of delivery and infinite loop)
> > --- a/testcases/network/stress/route/route4-rmmod
> > +++ b/testcases/network/stress/route/route4-rmmod
> ...
> > test_body()
> > {
> > - test_type=$1
> > + local cmd_type=$1
> > + local cmd_name="$(get_cmd $cmd_type)"
> > + local dst_addr=${DST_NETWORK}.${DST_HOST}
> > + local dst_network=${DST_NETWORK}.0
Left unused variables, I remove them.
> > + local cnt=0
> > - TCID=route4-rmmod0${test_type}
> > - TST_COUNT=$test_type
> > + tst_resm TINFO "IPv4 route is added by '$cmd_name' command and then deleted by removing network driver $NS_TIMES times"
> This is not what the test does after the changes.
Yes, no route added, sorry. I removed it because it was already set by
tst_add_ipaddr_stress:
10.23.0.0/24 dev ltp_ns_veth2 proto kernel scope link src 10.23.0.2
> With this implementation it should be moved to 'interface' group.
I'll put it back to match the description and really test creating route, so I can keep it
here.
Kind regards,
Petr
More information about the ltp
mailing list