----- On 13 Aug, 2020, at 19:13, David Ahern dsah...@gmail.com wrote:

...

>> +ipv6_ping()
>> +{
>> +    log_section "IPv6: VRF ICMP error route lookup ping"
>> +
>> +    setup
>> +
>> +    # verify connectivity
>> +    if ! check_connectivity6; then
>> +            echo "Error: Basic connectivity is broken"
>> +            ret=1
>> +            return
>> +    fi
>> +
>> +    if [ "$VERBOSE" = "1" ]; then
>> +            echo "Command to check for ICMP ttl exceeded:"
>> +            run_cmd ip netns exec h1 "${ping6}" -t1 -c1 -W2 ${H2_N2_IP6}
>> +    fi
>> +
>> +    ip netns exec h1 "${ping6}" -t1 -c1 -W2 ${H2_N2_IP6} | grep -q "Time 
>> exceeded:
>> Hop limit"
> 
> run_cmd runs the command and if VERBOSE is set to 1 shows the command to
> the user. Something is off with this script and passing the -v arg -- I
> do not get a command list. This applies to the whole script.

Hum, I have no issues here with '-v', you get no output at all from run_cmd?

> 
> Since you need to check for output, I suggest modifying run_cmd to
> search the output for the given string.

I took this pattern of executing commands twice when running with verbose and
grepping the output from icmp_redirect.sh. I'll see if I can come up with
something fancier in run_cmd.


>> +    log_test $? 0 "Ping received ICMP ttl exceeded"
>> +}
>> +################################################################################
> 
> missing newline between '}' and '####'

Ack

...

>> +while getopts :46pvh o
>> +do
>> +    case $o in
>> +            4) TESTS=ipv4;;
>> +            6) TESTS=ipv6;;
>> +                p) PAUSE_ON_FAIL=yes;;
>> +                v) VERBOSE=1;;
>> +            h) usage; exit 0;;
>> +                *) usage; exit 1;;
> 
> indentation issues; not using tabs
> 

Ack

>> +    esac
>> +done
>> +
>> +#
>> +# show user test config
>> +#
>> +if [ -z "$TESTS" ]; then
>> +        TESTS="$TESTS_IPV4 $TESTS_IPV6"
>> +elif [ "$TESTS" = "ipv4" ]; then
>> +        TESTS="$TESTS_IPV4"
>> +elif [ "$TESTS" = "ipv6" ]; then
>> +        TESTS="$TESTS_IPV6"
>> +fi
>> +
>> +for t in $TESTS
>> +do
>> +    case $t in
>> +    ipv4_ping|ping)             ipv4_ping;;
>> +    ipv4_traceroute|traceroute) ipv4_traceroute;;
>> +
>> +    ipv6_ping|ping)             ipv6_ping;;
>> +    ipv6_traceroute|traceroute) ipv6_traceroute;;
>> +
>> +    # setup namespaces and config, but do not run any tests
>> +    setup)                      setup; exit 0;;
> 
> you don't allow '-t setup' so you can remove this part

I'l add the missing '-t' option to getopts.

> 
>> +
>> +    help)                       echo "Test names: $TESTS"; exit 0;;
>> +    esac
>> +done
>> +
>> +cleanup
>> +
>> +printf "\nTests passed: %3d\n" ${nsuccess}
>> +printf "Tests failed: %3d\n"   ${nfail}
>> +
>> +exit $ret

Reply via email to