netdev - Re: [PATCH RFC bpf-next 2/2] selftests: Update test_tc_neigh to use the modified bpf_redirect

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: <684a0bd5-b131-c620-ed5e-d1ea7d151ae1@iogearbox.net>
Date:   Mon, 19 Oct 2020 16:40:28 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH RFC bpf-next 2/2] selftests: Update test_tc_neigh to use
 the modified bpf_redirect_neigh()

On 10/15/20 5:46 PM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@...hat.com>
> 
> This updates the test_tc_neigh selftest to use the new syntax of
> bpf_redirect_neigh(). To exercise the helper both with and without the
> optional parameter, one forwarding direction is changed to do a
> bpf_fib_lookup() followed by a call to bpf_redirect_neigh(), while the
> other direction is using the map-based ifindex lookup letting the redirect
> helper resolve the nexthop from the FIB.
> 
> This also fixes the test_tc_redirect.sh script to work on systems that have
> a consolidated dual-stack 'ping' binary instead of separate ping/ping6
> versions.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>

I would prefer if you could not mix the two tests, meaning, one complete test
case is only with bpf_redirect_neigh(get_dev_ifindex(xxx), NULL, 0, 0) for both
directions, and another self-contained one is with fib lookup + bpf_redirect_neigh
with params, even if it means we duplicate test_tc_neigh.c slighly, but I think
that's fine for sake of test coverage.

Thanks,
Daniel