| Message ID | 20191101005127.1355-1-jakub.kicinski@netronome.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] Revert "selftests: bpf: Don't try to read files without read permission" | expand |
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.
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 :/
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!
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!
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):
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(-)