Re: [PATCH net-next v4 2/2] selftests: fib_tests: Add a test case for IPv6 garbage collection

[Kui-Feng Lee <sinquersw@gmail.com>]

On 7/31/23 11:14, Kui-Feng Lee wrote:
> Sorry for replying late! I am just back from a vocation.
> 
> On 7/25/23 03:27, Paolo Abeni wrote:
>> On Fri, 2023-07-21 at 17:38 -0700, kuifeng@meta.com wrote:
>>> From: Kui-Feng Lee <kuifeng@meta.com>
>>>
>>> Add 10 IPv6 routes with expiration time.  Wait for a few seconds
>>> to make sure they are removed correctly.
>>>
>>> Signed-off-by: Kui-Feng Lee <kuifeng@meta.com>
>>> ---
>>>   tools/testing/selftests/net/fib_tests.sh | 90 +++++++++++++++++++++++-
>>>   1 file changed, 87 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/net/fib_tests.sh 
>>> b/tools/testing/selftests/net/fib_tests.sh
>>> index 35d89dfa6f11..4c92fb3c3844 100755
>>> --- a/tools/testing/selftests/net/fib_tests.sh
>>> +++ b/tools/testing/selftests/net/fib_tests.sh
>>> @@ -9,13 +9,16 @@ ret=0
>>>   ksft_skip=4
>>>   # all tests in this script. Can be overridden with -t option
>>> -TESTS="unregister down carrier nexthop suppress ipv6_notify 
>>> ipv4_notify ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric 
>>> ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter 
>>> ipv4_del_addr ipv4_mangle ipv6_mangle ipv4_bcast_neigh"
>>> +TESTS="unregister down carrier nexthop suppress ipv6_notify 
>>> ipv4_notify \
>>> +       ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric 
>>> ipv6_route_metrics \
>>> +       ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr \
>>> +       ipv4_mangle ipv6_mangle ipv4_bcast_neigh fib6_gc_test"
>>>   VERBOSE=0
>>>   PAUSE_ON_FAIL=no
>>>   PAUSE=no
>>> -IP="ip -netns ns1"
>>> -NS_EXEC="ip netns exec ns1"
>>> +IP="$(which ip) -netns ns1"
>>> +NS_EXEC="$(which ip) netns exec ns1"
>>>   which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || 
>>> ping6=$(which ping)
>>> @@ -747,6 +750,86 @@ fib_notify_test()
>>>       cleanup &> /dev/null
>>>   }
>>> +fib6_gc_test()
>>> +{
>>> +    setup
>>> +
>>> +    echo
>>> +    echo "Fib6 garbage collection test"
>>> +    set -e
>>> +
>>> +    # Check expiration of routes every 3 seconds (GC)
>>> +    $NS_EXEC sysctl -wq net.ipv6.route.gc_interval=300
>>> +
>>> +    $IP link add dummy_10 type dummy
>>> +    $IP link set dev dummy_10 up
>>> +    $IP -6 address add 2001:10::1/64 dev dummy_10
>>> +
>>> +    $NS_EXEC sysctl -wq net.ipv6.route.flush=1
>>> +
>>> +    # Temporary routes
>>> +    for i in $(seq 1 1000); do
>>> +        # Expire route after 4 seconds
>>> +        $IP -6 route add 2001:20::$i \
>>> +        via 2001:10::2 dev dummy_10 expires 4
>>> +    done
>>> +    N_EXP=$($IP -6 route list |grep expires|wc -l)
>>> +    if [ $N_EXP -ne 1000 ]; then
>>> +        echo "FAIL: expected 1000 routes with expires, got $N_EXP"
>>> +        ret=1
>>> +    else
>>> +        sleep 5
>>> +        REALTM_P=$($NS_EXEC strace -T sysctl \
>>> +               -wq net.ipv6.route.flush=1 2>&1 | \
>>> +               awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", 
>>> ""); print $0;}')
>>
>> I guess the above works somehow ?!?
>>
>> But I think something alike:
>>
>>     # just after printing the banner
>>     TIME=$(which time)
>>     if [ -z "$TIME" ]; then
>>         echo "command 'time' is missing, skipping test"
>>         return
>>     fi
>>     # ...
>>
>>         # replacing the strace command
>>         REALTM_P=$(time -f %e $NS_EXEC sysctl \
>>                -wq net.ipv6.route.flush=1 2>&1)
>>
>> would be better.
>>
>> In any case you should check explicitly for the additionally needed
>> command ('strace' in your code, 'time' here).
>>
>> And you could include the expected output in the commit message (just a
>> line, right?)
>>
>> Cheers
>>
>> Paolo
> 
> 
> The availability of 'time' is much higher than 'strace', however
> 'time -f %e' measures runtime in mini-seconds. That means I have to
> add more (x100 perhaps) temporary routes to make it reliably visible to
> 'time -f %e'. I will try it and check the existence of the command.

It is really slow adding 100000 routes. I turned to 'strace' and
detecting its existence.

> 
>>
>>
>>> +        N_EXP_s5=$($IP -6 route list |grep expires|wc -l)
>>> +
>>> +        if [ $N_EXP_s5 -ne 0 ]; then
>>> +        echo "FAIL: expected 0 routes with expires, got $N_EXP_s5"
>>> +        ret=1
>>> +        else
>>> +        ret=0
>>> +        fi
>>> +    fi
>>> +
>>> +    # Permanent routes
>>> +    for i in $(seq 1 5000); do
>>> +        $IP -6 route add 2001:30::$i \
>>> +        via 2001:10::2 dev dummy_10
>>> +    done
>>> +    # Temporary routes
>>> +    for i in $(seq 1 1000); do
>>> +        # Expire route after 4 seconds
>>> +        $IP -6 route add 2001:20::$i \
>>> +        via 2001:10::2 dev dummy_10 expires 4
>>> +    done
>>> +    N_EXP=$($IP -6 route list |grep expires|wc -l)
>>> +    if [ $N_EXP -ne 1000 ]; then
>>> +        echo
>>> +        "FAIL: expected 1000 routes with expires, got $N_EXP (5000 
>>> permanent routes)"
>>> +        ret=1
>>> +    else
>>> +        sleep 5
>>> +        REALTM_T=$($NS_EXEC strace -T sysctl \
>>> +               -wq net.ipv6.route.flush=1 2>&1 | \
>>> +               awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", 
>>> ""); print $0;}')
>>> +        N_EXP_s5=$($IP -6 route list |grep expires|wc -l)
>>> +
>>> +        if [ $N_EXP_s5 -ne 0 ]; then
>>> +        echo "FAIL: expected 0 routes with expires, got $N_EXP_s5 
>>> (5000 permanent routes)"
>>> +        ret=1
>>> +        else
>>> +        ret=0
>>> +        fi
>>> +    fi
>>> +
>>> +    set +e
>>> +
>>> +    log_test $ret 0 "ipv6 route garbage collection (${REALTM_P}s, 
>>> ${REALTM_T}s)"
>>> +
>>> +    cleanup &> /dev/null
>>> +}
>>> +
>>>   fib_suppress_test()
>>>   {
>>>       echo
>>> @@ -2217,6 +2300,7 @@ do
>>>       ipv4_mangle)            ipv4_mangle_test;;
>>>       ipv6_mangle)            ipv6_mangle_test;;
>>>       ipv4_bcast_neigh)        ipv4_bcast_neigh_test;;
>>> +    fib6_gc_test|ipv6_gc)        fib6_gc_test;;
>>>       help) echo "Test names: $TESTS"; exit 0;;
>>>       esac
>>
>>