[PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[Jakub Kicinski]

This reverts commit 5bc60de50dfe ("selftests: bpf: Don't try to read
files without read permission").

Quoted commit does not work at all, and was never tested.
Script requires root permissions (and tests for them)
and os.access() will always return true for root.

The correct fix is needed in the bpf tree, so let's just
revert and save ourselves the merge conflict.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index c44c650bde3a..15a666329a34 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -312,7 +312,7 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
             if f == "ports":
                 continue
             p = os.path.join(path, f)
-            if os.path.isfile(p) and os.access(p, os.R_OK):
+            if os.path.isfile(p):
                 _, out = cmd('cat %s/%s' % (path, f))
                 dfs[f] = out.strip()
             elif os.path.isdir(p):
-- 
2.23.0

Re: [PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 5:51 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> This reverts commit 5bc60de50dfe ("selftests: bpf: Don't try to read
> files without read permission").
>
> Quoted commit does not work at all, and was never tested.
> Script requires root permissions (and tests for them)
> and os.access() will always return true for root.
>
> The correct fix is needed in the bpf tree, so let's just
> revert and save ourselves the merge conflict.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>
Since original commit is broken may be apply directly to net-next ?
I'm fine whichever way.
I would wait for Jiri to reply first though.

Re: [oss-drivers] Re: [PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[Jakub Kicinski]

On Thu, 31 Oct 2019 17:56:46 -0700, Alexei Starovoitov wrote:
> On Thu, Oct 31, 2019 at 5:51 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> >
> > This reverts commit 5bc60de50dfe ("selftests: bpf: Don't try to read
> > files without read permission").
> >
> > Quoted commit does not work at all, and was never tested.
> > Script requires root permissions (and tests for them)
> > and os.access() will always return true for root.
> >
> > The correct fix is needed in the bpf tree, so let's just
> > revert and save ourselves the merge conflict.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>  
> 
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> Since original commit is broken may be apply directly to net-next ?
> I'm fine whichever way.

I'm 3 fixes down to get test_offloads.py to work again. One for
cls_bpf, one for the test itself and one for net/core/dev.c logic.
Should I target all those at net?

Are you and Daniel running test_offloads.py?  It looks like it lots of
things slipped in since I last run it :(

> I would wait for Jiri to reply first though.

Not sure what he can contribute at this point but sure :/

Re: [oss-drivers] Re: [PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[Jiri Pirko]

Fri, Nov 01, 2019 at 02:28:35AM CET, jakub.kicinski@netronome.com wrote:
>On Thu, 31 Oct 2019 17:56:46 -0700, Alexei Starovoitov wrote:
>> On Thu, Oct 31, 2019 at 5:51 PM Jakub Kicinski
>> <jakub.kicinski@netronome.com> wrote:
>> >
>> > This reverts commit 5bc60de50dfe ("selftests: bpf: Don't try to read
>> > files without read permission").
>> >
>> > Quoted commit does not work at all, and was never tested.
>> > Script requires root permissions (and tests for them)
>> > and os.access() will always return true for root.
>> >
>> > The correct fix is needed in the bpf tree, so let's just
>> > revert and save ourselves the merge conflict.
>> >
>> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>  
>> 
>> Acked-by: Alexei Starovoitov <ast@kernel.org>
>> Since original commit is broken may be apply directly to net-next ?
>> I'm fine whichever way.
>
>I'm 3 fixes down to get test_offloads.py to work again. One for
>cls_bpf, one for the test itself and one for net/core/dev.c logic.
>Should I target all those at net?
>
>Are you and Daniel running test_offloads.py?  It looks like it lots of
>things slipped in since I last run it :(
>
>> I would wait for Jiri to reply first though.
>
>Not sure what he can contribute at this point but sure :/

I'm okay with Jakub taking care of the fix. Thanks!

Re: [PATCH bpf-next] Revert "selftests: bpf: Don't try to read files without read permission"

[Daniel Borkmann]

On 11/1/19 1:51 AM, Jakub Kicinski wrote:
> This reverts commit 5bc60de50dfe ("selftests: bpf: Don't try to read
> files without read permission").
> 
> Quoted commit does not work at all, and was never tested.
> Script requires root permissions (and tests for them)
> and os.access() will always return true for root.
> 
> The correct fix is needed in the bpf tree, so let's just
> revert and save ourselves the merge conflict.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied, thanks!