2022-09-07 05:53:33

by Zhou, Jie2X

[permalink] [raw]
Subject: test ./tools/testing/selftests/bpf/test_offload.py failed

hi,

I try to know why output following error?
I found that "disable_ifindex" file do not set read function, so return -EINVAL when do read.
Is it a bug in test_offload.py?

test output:
selftests: bpf: test_offload.py
Test destruction of generic XDP...
......
raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex

cat: /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex: Invalid argument
not ok 20 selftests: bpf: test_offload.py # exit=1

source code:
In drivers/net/netdevsim/hwstats.c:
define NSIM_DEV_HWSTATS_FOPS(ACTION, TYPE) \
{ \
.fops = { \
.open = simple_open, \
.write = nsim_dev_hwstats_do_write, \
.llseek = generic_file_llseek, \
.owner = THIS_MODULE, \
}, \
.action = ACTION, \
.type = TYPE, \
}

static const struct nsim_dev_hwstats_fops nsim_dev_hwstats_l3_disable_fops =
NSIM_DEV_HWSTATS_FOPS(NSIM_DEV_HWSTATS_DO_DISABLE,
NETDEV_OFFLOAD_XSTATS_TYPE_L3);

debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_disable_fops.fops);

In fs/read_write.c:
ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
{
......
if (file->f_op->read)
ret = file->f_op->read(file, buf, count, pos);
else if (file->f_op->read_iter)
ret = new_sync_read(file, buf, count, pos);
else
ret = -EINVAL;
......
}

best regards,


2022-09-07 07:24:19

by Ido Schimmel

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Wed, Sep 07, 2022 at 01:16:57PM +0800, Jie2x Zhou wrote:
> I found that "disable_ifindex" file do not set read function, so return -EINVAL when do read.
> Is it a bug in test_offload.py?

Most likely a bug in netdevsim itself as it sets the mode of this file
as "rw" instead of "w". The test actually knows to skip such files:

p = os.path.join(path, f)
if not os.stat(p).st_mode & stat.S_IRUSR:
continue

Can you test the following patch?

diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
index 605a38e16db0..0e58aa7f0374 100644
--- a/drivers/net/netdevsim/hwstats.c
+++ b/drivers/net/netdevsim/hwstats.c
@@ -433,11 +433,11 @@ int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
goto err_remove_hwstats_recursive;
}

- debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("enable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_enable_fops.fops);
- debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("disable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_disable_fops.fops);
- debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("fail_next_enable", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_fail_fops.fops);

INIT_DELAYED_WORK(&hwstats->traffic_dw,

>
> test output:
> selftests: bpf: test_offload.py
> Test destruction of generic XDP...
> ......
> raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
> Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex
>
> cat: /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex: Invalid argument
> not ok 20 selftests: bpf: test_offload.py # exit=1

2022-09-07 09:38:51

by Zhou, Jie2X

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

hi,

>Most likely a bug in netdevsim itself as it sets the mode of this file
>as "rw" instead of "w". The test actually knows to skip such files:
>Can you test the following patch?
Sorry, I want to confirm that have you ever run test_offload.py test?
What is the output of test_offload.py?

best regards,

________________________________________
From: Ido Schimmel <[email protected]>
Sent: Wednesday, September 7, 2022 2:43 PM
To: Zhou, Jie2X
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Li, Philip; [email protected]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Wed, Sep 07, 2022 at 01:16:57PM +0800, Jie2x Zhou wrote:
> I found that "disable_ifindex" file do not set read function, so return -EINVAL when do read.
> Is it a bug in test_offload.py?

Most likely a bug in netdevsim itself as it sets the mode of this file
as "rw" instead of "w". The test actually knows to skip such files:

p = os.path.join(path, f)
if not os.stat(p).st_mode & stat.S_IRUSR:
continue

Can you test the following patch?

diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
index 605a38e16db0..0e58aa7f0374 100644
--- a/drivers/net/netdevsim/hwstats.c
+++ b/drivers/net/netdevsim/hwstats.c
@@ -433,11 +433,11 @@ int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
goto err_remove_hwstats_recursive;
}

- debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("enable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_enable_fops.fops);
- debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("disable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_disable_fops.fops);
- debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("fail_next_enable", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_fail_fops.fops);

INIT_DELAYED_WORK(&hwstats->traffic_dw,

>
> test output:
> selftests: bpf: test_offload.py
> Test destruction of generic XDP...
> ......
> raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
> Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex
>
> cat: /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex: Invalid argument
> not ok 20 selftests: bpf: test_offload.py # exit=1

2022-09-07 16:42:07

by Ido Schimmel

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Wed, Sep 07, 2022 at 08:51:56AM +0000, Zhou, Jie2X wrote:
> What is the output of test_offload.py?

This output [1], but requires this [2] additional fix on top of the one
I already posted for netdevsim. Hopefully someone more familiar with
this test can comment if this is the right fix or not.

Without it, bpftool refuses to load the program [3].

[1]
# ./test_offload.py
Test destruction of generic XDP...
Test TC non-offloaded...
Test TC non-offloaded isn't getting bound...
Test TC offloads are off by default...
[...]
test_offload.py: OK
# echo $?
0

[2]
diff --git a/tools/testing/selftests/bpf/progs/sample_map_ret0.c b/tools/testing/selftests/bpf/progs/sample_map_ret0.c
index 495990d355ef..91417aae6194 100644
--- a/tools/testing/selftests/bpf/progs/sample_map_ret0.c
+++ b/tools/testing/selftests/bpf/progs/sample_map_ret0.c
@@ -17,7 +17,8 @@ struct {
} array SEC(".maps");

/* Sample program which should always load for testing control paths. */
-SEC(".text") int func()
+SEC("xdp")
+int func()
{
__u64 key64 = 0;
__u32 key = 0;
diff --git a/tools/testing/selftests/bpf/progs/sample_ret0.c b/tools/testing/selftests/bpf/progs/sample_ret0.c
index fec99750d6ea..f51c63dd6f20 100644
--- a/tools/testing/selftests/bpf/progs/sample_ret0.c
+++ b/tools/testing/selftests/bpf/progs/sample_ret0.c
@@ -1,6 +1,9 @@
/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>

/* Sample program which should always load for testing control paths. */
+SEC("xdp")
int func()
{
return 0;
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 6cd6ef9fc20b..0381f48f45a6 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -235,7 +235,7 @@ def tc(args, JSON=True, ns="", fail=True, include_stderr=False):
def ethtool(dev, opt, args, fail=True):
return cmd("ethtool %s %s %s" % (opt, dev["ifname"], args), fail=fail)

-def bpf_obj(name, sec=".text", path=bpf_test_dir,):
+def bpf_obj(name, sec="xdp", path=bpf_test_dir,):
return "obj %s sec %s" % (os.path.join(path, name), sec)

def bpf_pinned(name):

[3]
# bpftool prog load /home/idosch/code/linux/tools/testing/selftests/bpf/sample_ret0.o /sys/fs/bpf/nooffload type xdp
Error: object file doesn't contain any bpf program
Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.
If it used to work for this object file but now doesn't, see --legacy option for more details.

2022-09-08 03:31:43

by Zhou, Jie2X

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

hi ido,

My error is "Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex"

Do you get [1]error, after patch [2]?

[1]
# bpftool prog load /home/idosch/code/linux/tools/testing/selftests/bpf/sample_ret0.o /sys/fs/bpf/nooffload type xdp
Error: object file doesn't contain any bpf program
Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.
If it used to work for this object file but now doesn't, see --legacy option for more details.

[2]
diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
index 605a38e16db0..0e58aa7f0374 100644
--- a/drivers/net/netdevsim/hwstats.c
+++ b/drivers/net/netdevsim/hwstats.c
@@ -433,11 +433,11 @@ int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
goto err_remove_hwstats_recursive;
}

- debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("enable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_enable_fops.fops);
- debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("disable_ifindex", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_disable_fops.fops);
- debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
+ debugfs_create_file("fail_next_enable", 0200, hwstats->l3_ddir, hwstats,
&nsim_dev_hwstats_l3_fail_fops.fops);

INIT_DELAYED_WORK(&hwstats->traffic_dw,

best regards,

________________________________________
From: Ido Schimmel <[email protected]>
Sent: Thursday, September 8, 2022 12:08 AM
To: Zhou, Jie2X; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Li, Philip; [email protected]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Wed, Sep 07, 2022 at 08:51:56AM +0000, Zhou, Jie2X wrote:
> What is the output of test_offload.py?

This output [1], but requires this [2] additional fix on top of the one
I already posted for netdevsim. Hopefully someone more familiar with
this test can comment if this is the right fix or not.

Without it, bpftool refuses to load the program [3].

[1]
# ./test_offload.py
Test destruction of generic XDP...
Test TC non-offloaded...
Test TC non-offloaded isn't getting bound...
Test TC offloads are off by default...
[...]
test_offload.py: OK
# echo $?
0

[2]
diff --git a/tools/testing/selftests/bpf/progs/sample_map_ret0.c b/tools/testing/selftests/bpf/progs/sample_map_ret0.c
index 495990d355ef..91417aae6194 100644
--- a/tools/testing/selftests/bpf/progs/sample_map_ret0.c
+++ b/tools/testing/selftests/bpf/progs/sample_map_ret0.c
@@ -17,7 +17,8 @@ struct {
} array SEC(".maps");

/* Sample program which should always load for testing control paths. */
-SEC(".text") int func()
+SEC("xdp")
+int func()
{
__u64 key64 = 0;
__u32 key = 0;
diff --git a/tools/testing/selftests/bpf/progs/sample_ret0.c b/tools/testing/selftests/bpf/progs/sample_ret0.c
index fec99750d6ea..f51c63dd6f20 100644
--- a/tools/testing/selftests/bpf/progs/sample_ret0.c
+++ b/tools/testing/selftests/bpf/progs/sample_ret0.c
@@ -1,6 +1,9 @@
/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>

/* Sample program which should always load for testing control paths. */
+SEC("xdp")
int func()
{
return 0;
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 6cd6ef9fc20b..0381f48f45a6 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -235,7 +235,7 @@ def tc(args, JSON=True, ns="", fail=True, include_stderr=False):
def ethtool(dev, opt, args, fail=True):
return cmd("ethtool %s %s %s" % (opt, dev["ifname"], args), fail=fail)

-def bpf_obj(name, sec=".text", path=bpf_test_dir,):
+def bpf_obj(name, sec="xdp", path=bpf_test_dir,):
return "obj %s sec %s" % (os.path.join(path, name), sec)

def bpf_pinned(name):

[3]
# bpftool prog load /home/idosch/code/linux/tools/testing/selftests/bpf/sample_ret0.o /sys/fs/bpf/nooffload type xdp
Error: object file doesn't contain any bpf program
Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.
If it used to work for this object file but now doesn't, see --legacy option for more details.

2022-09-08 06:37:35

by Ido Schimmel

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Thu, Sep 08, 2022 at 03:10:51AM +0000, Zhou, Jie2X wrote:
> My error is "Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex"

This one is solved by the netdevsim patch ([2]).

> Do you get [1]error, after patch [2]?

Yes. Maybe you do not see it because you have an older bpftool without
"libbpf_strict" feature:

$ bpftool --version
bpftool v6.8.0
using libbpf v0.8
features: libbfd, libbpf_strict, skeletons

> [1]
> # bpftool prog load /home/idosch/code/linux/tools/testing/selftests/bpf/sample_ret0.o /sys/fs/bpf/nooffload type xdp
> Error: object file doesn't contain any bpf program
> Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.
> If it used to work for this object file but now doesn't, see --legacy option for more details.
>
> [2]
> diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
> index 605a38e16db0..0e58aa7f0374 100644
> --- a/drivers/net/netdevsim/hwstats.c
> +++ b/drivers/net/netdevsim/hwstats.c
> @@ -433,11 +433,11 @@ int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
> goto err_remove_hwstats_recursive;
> }
>
> - debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("enable_ifindex", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_enable_fops.fops);
> - debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("disable_ifindex", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_disable_fops.fops);
> - debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("fail_next_enable", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_fail_fops.fops);
>
> INIT_DELAYED_WORK(&hwstats->traffic_dw,

2022-09-08 08:05:06

by Zhou, Jie2X

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

hi ido,

>> My error is "Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex"
>This one is solved by the netdevsim patch ([2]).
Thanks for your reply.
About netdevsim patch ([2]), do you commit it to kernel mail list?

best regards,

________________________________________
From: Ido Schimmel <[email protected]>
Sent: Thursday, September 8, 2022 2:23 PM
To: Zhou, Jie2X
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Li, Philip; [email protected]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Thu, Sep 08, 2022 at 03:10:51AM +0000, Zhou, Jie2X wrote:
> My error is "Exception: Command failed: cat /sys/kernel/debug/netdevsim/netdevsim0//ports/0/dev/hwstats/l3/disable_ifindex"

This one is solved by the netdevsim patch ([2]).

> Do you get [1]error, after patch [2]?

Yes. Maybe you do not see it because you have an older bpftool without
"libbpf_strict" feature:

$ bpftool --version
bpftool v6.8.0
using libbpf v0.8
features: libbfd, libbpf_strict, skeletons

> [1]
> # bpftool prog load /home/idosch/code/linux/tools/testing/selftests/bpf/sample_ret0.o /sys/fs/bpf/nooffload type xdp
> Error: object file doesn't contain any bpf program
> Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.
> If it used to work for this object file but now doesn't, see --legacy option for more details.
>
> [2]
> diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
> index 605a38e16db0..0e58aa7f0374 100644
> --- a/drivers/net/netdevsim/hwstats.c
> +++ b/drivers/net/netdevsim/hwstats.c
> @@ -433,11 +433,11 @@ int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
> goto err_remove_hwstats_recursive;
> }
>
> - debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("enable_ifindex", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_enable_fops.fops);
> - debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("disable_ifindex", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_disable_fops.fops);
> - debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
> + debugfs_create_file("fail_next_enable", 0200, hwstats->l3_ddir, hwstats,
> &nsim_dev_hwstats_l3_fail_fops.fops);
>
> INIT_DELAYED_WORK(&hwstats->traffic_dw,

2022-09-08 09:10:39

by Ido Schimmel

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Thu, Sep 08, 2022 at 07:44:06AM +0000, Zhou, Jie2X wrote:
> About netdevsim patch ([2]), do you commit it to kernel mail list?

Yes, I will send it to 'net'. Can I add your Tested-by tag?

2022-09-09 09:23:51

by Zhou, Jie2X

[permalink] [raw]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

hi ido,

After apply your kernel patch and bpf test patch.
The test result is OK.

Test destruction of generic XDP...
Test TC non-offloaded...
Test TC non-offloaded isn't getting bound...
Test TC offloads are off by default...
Test TC offload by default...
......
Test loading program with maps...
Test bpftool bound info reporting (own ns)...
Test bpftool bound info reporting (other ns)...
Test bpftool bound info reporting (remote ns)...
Test bpftool bound info reporting (back to own ns)...
Test bpftool bound info reporting (removed dev)...
Test map update (no flags)...
Test map update (exists)...
Test map update (noexist)...
Test map dump...
Test map getnext...
Test map delete (htab)...
Test map delete (array)...
Test map remove...
Test map creation fail path...
Test multi-dev ASIC program reuse...
Test multi-dev ASIC cross-dev replace...
Test multi-dev ASIC cross-dev install...
Test multi-dev ASIC cross-dev map reuse...
Test multi-dev ASIC cross-dev destruction...
Test multi-dev ASIC cross-dev destruction - move...
Test multi-dev ASIC cross-dev destruction - orphaned...
test_offload.py: OK

best regards,

________________________________________
From: Ido Schimmel <[email protected]>
Sent: Thursday, September 8, 2022 4:03 PM
To: Zhou, Jie2X
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Li, Philip; [email protected]
Subject: Re: test ./tools/testing/selftests/bpf/test_offload.py failed

On Thu, Sep 08, 2022 at 07:44:06AM +0000, Zhou, Jie2X wrote:
> About netdevsim patch ([2]), do you commit it to kernel mail list?

Yes, I will send it to 'net'. Can I add your Tested-by tag?