netdev - Re: [PATCH net-next v2 3/3] selftests: net: add new testcases for nexthop API compat mode sysctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJieiUiRdbPUWe0oKR2T_y+LgWeZfMqzhFMPjaw2KsqmrdeuxQ@mail.gmail.com>
Date:   Mon, 27 Apr 2020 10:15:05 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     David Ahern <dsahern@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        Benjamin Poirier <bpoirier@...ulusnetworks.com>
Subject: Re: [PATCH net-next v2 3/3] selftests: net: add new testcases for
 nexthop API compat mode sysctl

On Sun, Apr 26, 2020 at 11:38 AM Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
>
> On Sun, Apr 26, 2020 at 10:40 AM David Ahern <dsahern@...il.com> wrote:
> >
> > On 4/25/20 6:48 PM, Roopa Prabhu wrote:
> > > @@ -253,6 +253,33 @@ check_route6()
> > >       check_output "${out}" "${expected}"
> > >  }
> > >
> > > +start_ip_monitor()
> > > +{
> > > +     local mtype=$1
> > > +
> > > +     # start the monitor in the background
> > > +     tmpfile=`mktemp /var/run/nexthoptestXXX`
> > > +     mpid=`($IP monitor $1 > $tmpfile & echo $!) 2>/dev/null`
> >
> > && echo?
> >
> > also, that looks weird. shouldn't it be:
> >         mpid=($IP monitor ${mtype} > $tmpfile 2>&1 && echo $!)
> >
>
> no, the & is for background. I picked this from the rtnetlink selftest.
> It can be moved to a library if there is one, this version is
> specifically for route monitor.
>
> > you declare mtype but use $1.
>
> will fix , thats a leftover from last min cleanup
>
> >
> >
> > > +     sleep 0.2
> > > +     echo "$mpid $tmpfile"
> > > +}
> > > +
> > > +stop_ip_monitor()
> > > +{
> > > +     local mpid=$1
> > > +     local tmpfile=$2
> > > +     local el=$3
> > > +
> > > +     # check the monitor results
> > > +     kill $mpid
> > > +     lines=`wc -l $tmpfile | cut "-d " -f1`
> >
> > just for consistency with the rest of the script, use $(...) instead of
> > `...`
> >
> > > +     test $lines -eq $el
> > > +     rc=$?
> > > +     rm -rf $tmpfile
> > > +
> > > +     return $rc
> > > +}
> > > +
> > >  ################################################################################
> > >  # basic operations (add, delete, replace) on nexthops and nexthop groups
> > >  #
> >
> > ...
> >
> > > +ipv6_compat_mode()
> > > +{
> > > +     local rc
> > > +
> > > +     echo
> > > +     echo "IPv6 nexthop api compat mode test"
> > > +     echo "--------------------------------"
> > > +
> > > +     sysctl_nexthop_compat_mode_check "IPv6"
> > > +     if [ $? -eq $ksft_skip ]; then
> > > +             return $ksft_skip
> > > +     fi
> > > +
> > > +     run_cmd "$IP nexthop add id 62 via 2001:db8:91::2 dev veth1"
> > > +     run_cmd "$IP nexthop add id 63 via 2001:db8:91::3 dev veth1"
> > > +     run_cmd "$IP nexthop add id 122 group 62/63"
> > > +     ipmout=$(start_ip_monitor route)
> > > +
> > > +     run_cmd "$IP -6 ro add 2001:db8:101::1/128 nhid 122"
> > > +     # route add notification should contain expanded nexthops
> > > +     stop_ip_monitor $ipmout 3
> > > +     log_test $? 0 "IPv6 compat mode on - route add notification"
> > > +
> > > +     # route dump should contain expanded nexthops
> > > +     check_route6 "2001:db8:101::1" "2001:db8:101::1 nhid 122 metric 1024 pref medium nexthop via 2001:db8:91::2 dev veth1 weight 1 nexthop via 2001:db8:91::3 dev veth1 weight 1"
> > > +     log_test $? 0 "IPv6 compat mode on - route dump"
> > > +
> > > +     # change in nexthop group should generate route notification
> > > +     run_cmd "$IP nexthop add id 64 via 2001:db8:91::4 dev veth1"
> > > +     ipmout=$(start_ip_monitor route)
> > > +     run_cmd "$IP nexthop replace id 122 group 62/64"
> > > +     stop_ip_monitor $ipmout 3
> > > +
> > > +     log_test $? 0 "IPv6 compat mode on - nexthop change"
> > > +
> > > +     # set compat mode off
> > > +     sysctl_nexthop_compat_mode_set 0 "IPv6"
> > > +
> > > +     run_cmd "$IP -6 ro del 2001:db8:101::1/128 nhid 122"
> > > +
> > > +     run_cmd "$IP nexthop add id 62 via 2001:db8:91::2 dev veth1"
> > > +     run_cmd "$IP nexthop add id 63 via 2001:db8:91::3 dev veth1"
> > > +     run_cmd "$IP nexthop add id 122 group 62/63"
> > > +     ipmout=$(start_ip_monitor route)
> > > +
> > > +     run_cmd "$IP -6 ro add 2001:db8:101::1/128 nhid 122"
> > > +     # route add notification should not contain expanded nexthops
> > > +     stop_ip_monitor $ipmout 1
> > > +     log_test $? 0 "IPv6 compat mode off - route add notification"
> > > +
> > > +     # route dump should not contain expanded nexthops
> > > +     check_route6 "2001:db8:101::1" "2001:db8:101::1 nhid 122 metric 1024 pref medium"
> >
> > here and above, remove the 'pref medium' from the string; it was moved
> > by a recent iproute2 change (from Donald) so for compat with old and new
> > iproute2 just drop it. See 493f3cc7ee02.
> >
>
> ack, looks like my iproute2 is a little old

David, I still see pref with the latest iproute2-next. I have removed
it from v3. But there are other tests in fib_nexthops.sh that still
use 'pref'. Just fyi. I don't plan to respin the selftest patch,
unless you want me to. Just fyi