netdev - Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding

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: <CADvbK_c6pdnHuKd_aDG3REWvGRx5AtGH2YO90tyRvcWesBDzPw@mail.gmail.com>
Date:   Thu, 5 Jul 2018 01:56:23 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     network dev <netdev@...r.kernel.org>, davem <davem@...emloft.net>,
        Davide Caratti <dcaratti@...hat.com>,
        Ido Schimmel <idosch@...sch.org>
Subject: Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed
 broadcast forwarding

On Wed, Jul 4, 2018 at 3:23 AM, David Ahern <dsahern@...il.com> wrote:
> On 7/3/18 5:36 AM, Xin Long wrote:
>> On Mon, Jul 2, 2018 at 11:12 PM, David Ahern <dsahern@...il.com> wrote:
>>> On 7/2/18 12:30 AM, Xin Long wrote:
>>>> +ping_ipv4()
>>>> +{
>>>> +     sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>>>> +     bc_forwarding_disable
>>>> +     ping_test $h1 198.51.100.255
>>>> +
>>>> +     iptables -A INPUT -i vrf-r1 -p icmp -j DROP
>>>> +     bc_forwarding_restore
>>>> +     bc_forwarding_enable
>>>> +     ping_test $h1 198.51.100.255
>>>> +
>>>> +     bc_forwarding_restore
>>>> +     iptables -D INPUT -i vrf-r1 -p icmp -j DROP
>>>> +     sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>>>> +}
>>>
>>> Both tests fail for me:
>>> TEST: ping                                              [FAIL]
>>> TEST: ping                                              [FAIL]
>> I think 'ip vrf exec ...' is not working in your env, while
>> the testing is using "ip vrf exec vrf-h1 ping ..."
>>
>> You can test it by:
>> # ip link add dev vrf-test type vrf table 1111
>> # ip vrf exec vrf-test ls
>
> well, that's embarrassing. yes, I updated ip and forgot to apply the bpf
> workaround to define the syscall number (not defined in jessie).
>
>>
>>>
>>> Why the need for the iptables rule?
>> This iptables rule is to block the echo_request packet going to
>> route's local_in.
>> When bc_forwarding is NOT doing forwarding well but the packet
>> goes to the route's local_in, it will fail.
>>
>> Without this rule, the 2nd ping will always succeed, we can't tell the
>> echo_reply is from route or h2.
>>
>> Or you have a better way to test this?
>
> your commands are not a proper test. The test should succeed and fail
> based on the routing lookup, not iptables rules.
A proper test can be done easily with netns, as vrf can't isolate much.
I don't want to bother forwarding/ directory with netns, so I will probably
just drop this selftest, and let the feature patch go first.

What do you think?

>
>>
>>>
>>> And, PAUSE_ON_FAIL is not working to take a look at why tests are
>>> failing. e.g.,
>>>
>>> PAUSE_ON_FAIL=yes ./router_broadcast.sh
>>>
>>> just continues on. Might be something with the infrastructure scripts.
>> Yes, in ./router_broadcast.sh, it loads lib.sh where it loads forwarding.config
>> where it has "PAUSE_ON_FAIL=no", which would override your
>> "PAUSE_ON_FAIL=yes".
>>
>
> ack. bit by that as well.