[LTP] [PATCH v3 4/5] network/route: Rewrite route-change-gw into new API

[Alexey Kodanev]

Hi Petr,
On 8/6/19 10:55 PM, Petr Vorel wrote:
> Hi Alexey,
> 
>> Hi Petr,
>> On 7/25/19 2:10 PM, Petr Vorel wrote:
>>> * Drop route command (use just ip command), support both IPv4 and IPv6
>>> * Use unused network range to avoid clash with real network
>>> * Add verification with ping (previous version sent UDP datagram with
>>> ns-udpsender, but didn't verify receiving it on rhost and didn't setup
>>> rhost ip at all)
> 
>>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>>> ---
>>>  runtest/net_stress.route                      |   4 +-
>>>  .../network/stress/route/00_Descriptions.txt  |  18 +-
>>>  .../network/stress/route/route-change-gw      |  38 +++
>>>  .../network/stress/route/route4-change-gw     | 292 ------------------
>>>  .../network/stress/route/route6-change-gw     | 292 ------------------
>>>  5 files changed, 43 insertions(+), 601 deletions(-)
>>>  create mode 100755 testcases/network/stress/route/route-change-gw
>>>  delete mode 100644 testcases/network/stress/route/route4-change-gw
>>>  delete mode 100644 testcases/network/stress/route/route6-change-gw
> 
>>> ...
>>> +
>>> +# Change route gateway
>>> +# lhost: 10.23.x.2, gw (on rhost): 10.23.x.1, rhost: 10.23.0.1
>>> +
>>> +TST_TESTFUNC="test_gw"
>>> +. route-lib.sh
>>> +
>>> +setup()
>>> +{
>>> +	tst_res TINFO "change IPv$TST_IPVER route gateway $NS_TIMES times"
>>> +
>>> +	rt="$(tst_ipaddr_un -m 0 0)"
>>> +	lhost="$(tst_ipaddr_un 1 1)"
>>> +	rhost="$(tst_ipaddr_un 0 1)"
>>> +	tst_add_ipaddr -s -a $lhost
>>> +	tst_add_ipaddr -s -a $rhost rhost
>>> +}
>>> +
>>> +test_gw()
>>> +{
>>> +	local gw="$(tst_ipaddr_un 1 $(($1 + 1)))"
> 
>> We should keep $(($1 + 1)) within the valid range except already added IP address
>> ($lhost), i.e. for IPv4 the range is 2..254 for host id:
> 
>>     local gw="$(tst_ipaddr_un 1 $(($1 % 253 + 2)))"
> 
>> Either we could limit the value here or in the tst_ipaddr_un(). Looks like
>> route-change-if needs a similar fix for net id?
> Good point (sorry to keep octet/hextet overflow related errors).
> Although it'd be simpler to fix it in the code, I'd prefer to have this support
> in tst_ipaddr_un(). Diff below adds -l MIN_HOST_ID (I'll post it as a part of v3),
> do we want to lower also max host id?


It can only be "max host id" option since we already have this variable in the
function, and in the test setup, we could assign "lhost" variable with the max
value:

  lhost="$(tst_ipaddr_un 1 $max_host_id)"


> 
> 
>>> +	local iface="$(tst_iface)"
>>> +
>>> +	tst_res TINFO "testing route over gateway '$gw'"
>>> +
>>> +	tst_add_ipaddr -s -a $gw rhost
>>> +	ROD ip route replace $rt dev $iface via $gw
> 
>> May be it would be cleaner to use "add" instead of "replace" since we remove it
>> and it shouldn't exist before a test start.
> 
>>> +	EXPECT_PASS ping$TST_IPV6 -c1 -I $lhost $rhost
> 
>> It is better to redirect stdout to null:
> 
>> EXPECT_PASS ping$TST_IPV6 -c1 -I $lhost $rhost \>/dev/null
> 
>> The same for *-if and *-dst.
> 
> Agree with all you pointed out.
> 
>> The rest in the patch-set looks good to me.
> 
> 
> Kind regards,
> Petr
> 
> commit 3a3ed9bc93c18d899a81b0f592eec4d4402984b1
> Author: Petr Vorel <pvorel@suse.cz>
> Date:   2019-08-06 16:58:28 +0200
> 
>     net/tst_ipaddr_un: Add -l MIN_HOST_ID support
>     
>     Signed-off-by: Petr Vorel <pvorel@suse.cz>
> 
> diff --git testcases/lib/tst_net.sh testcases/lib/tst_net.sh
> index 714298797..50d64efca 100644
> --- testcases/lib/tst_net.sh
> +++ testcases/lib/tst_net.sh
> @@ -361,11 +361,13 @@ tst_ipaddr()
>  
>  # Get IP address of unused network, specified either by type and counter
>  # or by net and host.
> -# tst_ipaddr_un [-cCOUNTER] [-m] [TYPE]
> -# tst_ipaddr_un [-m] NET_ID [HOST_ID]
> +# tst_ipaddr_un [-c COUNTER] [-l MIN_HOST_ID] [-m] [TYPE]
> +# tst_ipaddr_un [-l MIN_HOST_ID] [-m] NET_ID [HOST_ID]
>  #
>  # OPTIONS
>  # -c COUNTER: integer value for counting HOST_ID and NET_ID (default: 1)
> +# -l MIN_HOST_ID: min HOST_ID allowed minimal HOST_ID (useful for loop which
> +# overflow max HOST_ID)
>  # -m: print also mask
>  # TYPE: { lhost | rhost } (default: 'lhost')
>  # NET_ID: integer or hex value of net (IPv4: 3rd octet, IPv6: 3rd hextet)
> @@ -373,30 +375,39 @@ tst_ipaddr()
>  # hextet, default: 0)
>  tst_ipaddr_un()
>  {
> -	local counter host_id lower mask max_host_id max_net_id net_id tmp type
> +	local counter host_id mask max_host_id max_net_id min_host_id net_id tmp type
>  	local OPTIND
>  
> -	while getopts "c:m" opt; do
> +	[ "$TST_IPV6" ] && max_net_id=65535 || max_net_id=255
> +	max_host_id=$((max_net_id - 1))
> +
> +	while getopts "c:l:m" opt; do
>  		case $opt in
>  			c) counter="$OPTARG";;
> -			l) lower="$OPTARG";;
> +			l)
> +				min_host_id="$OPTARG"
> +				if ! tst_is_int "$min_host_id"; then
> +					tst_brk TBROK "tst_ipaddr_un: -l must be integer ($min_host_id)"
> +				fi
> +				if [ $min_host_id -ge $max_host_id ]; then
> +					tst_brk TBROK "tst_ipaddr_un: -l must be >= $max_host_id ($min_host_id)"
> +				fi
> +				;;
>  			m) [ "$TST_IPV6" ] && mask="/64" || mask="/24";;
>  		esac
>  	done
>  	shift $(($OPTIND - 1))
>  
> -	[ "$TST_IPV6" ] && max_net_id=65535 || max_net_id=255
> -
>  	# counter
>  	if [ $# -eq 0 -o "$1" = "lhost" -o "$1" = "rhost" ]; then
>  		[ -z "$counter" ] && counter=1
>  		[ $counter -lt 1 ] && counter=1
>  		type="${1:-lhost}"
> -		max_host_id=$((max_net_id - 1))
>  		tmp=$((counter * 2))
>  		[ "$type" = "rhost" ] && tmp=$((tmp - 1))
>  
>  		host_id=$((tmp % max_host_id))
> +
>  		net_id=$((tmp / max_host_id))
>  
>  		if [ $host_id -eq 0 ]; then
> @@ -415,7 +426,11 @@ tst_ipaddr_un()
>  	fi
>  
>  	net_id=$((net_id % max_net_id))
> +	if [ "$min_host_id" ]; then
> +		host_id=$(( host_id % (max_host_id - min_host_id + 1) + min_host_id ))
> +	else
>  		host_id=$((host_id % max_net_id))
> +	fi
>  
>  	if [ -z "$TST_IPV6" ]; then
>  		echo "${IPV4_NET16_UNUSED}.${net_id}.${host_id}${mask}"
>