* [PATCH bpf v2 1/7] xdp: remove the xdp_attachment_flags_ok() callback
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 2/7] selftests/bpf/test_offload.py: Remove check for program load flags match Toke Høiland-Jørgensen
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
Since commit 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF
programs in net_device"), the XDP program attachment info is now maintained
in the core code. This interacts badly with the xdp_attachment_flags_ok()
check that prevents unloading an XDP program with different load flags than
it was loaded with. In practice, two kinds of failures are seen:
- An XDP program loaded without specifying a mode (and which then ends up
in driver mode) cannot be unloaded if the program mode is specified on
unload.
- The dev_xdp_uninstall() hook always calls the driver callback with the
mode set to the type of the program but an empty flags argument, which
means the flags_ok() check prevents the program from being removed,
leading to bpf prog reference leaks.
The original reason this check was added was to avoid ambiguity when
multiple programs were loaded. With the way the checks are done in the core
now, this is quite simple to enforce in the core code, so let's add a check
there and get rid of the xdp_attachment_flags_ok() callback entirely.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
.../net/ethernet/netronome/nfp/nfp_net_common.c | 6 -----
drivers/net/ethernet/ti/cpsw_priv.c | 3 ---
drivers/net/netdevsim/bpf.c | 3 ---
include/net/xdp.h | 2 --
net/core/dev.c | 22 ++++++++++++++++++--
net/core/xdp.c | 12 -----------
6 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index b150da43adb2..437226866ce8 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3562,9 +3562,6 @@ static int nfp_net_xdp_setup_drv(struct nfp_net *nn, struct netdev_bpf *bpf)
struct nfp_net_dp *dp;
int err;
- if (!xdp_attachment_flags_ok(&nn->xdp, bpf))
- return -EBUSY;
-
if (!prog == !nn->dp.xdp_prog) {
WRITE_ONCE(nn->dp.xdp_prog, prog);
xdp_attachment_setup(&nn->xdp, bpf);
@@ -3593,9 +3590,6 @@ static int nfp_net_xdp_setup_hw(struct nfp_net *nn, struct netdev_bpf *bpf)
{
int err;
- if (!xdp_attachment_flags_ok(&nn->xdp_hw, bpf))
- return -EBUSY;
-
err = nfp_app_xdp_offload(nn->app, nn, bpf->prog, bpf->extack);
if (err)
return err;
diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
index 31c5e36ff706..424e644724e4 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.c
+++ b/drivers/net/ethernet/ti/cpsw_priv.c
@@ -1265,9 +1265,6 @@ static int cpsw_xdp_prog_setup(struct cpsw_priv *priv, struct netdev_bpf *bpf)
if (!priv->xdpi.prog && !prog)
return 0;
- if (!xdp_attachment_flags_ok(&priv->xdpi, bpf))
- return -EBUSY;
-
WRITE_ONCE(priv->xdp_prog, prog);
xdp_attachment_setup(&priv->xdpi, bpf);
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 2e90512f3bbe..85546664bdd5 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -190,9 +190,6 @@ nsim_xdp_set_prog(struct netdevsim *ns, struct netdev_bpf *bpf,
{
int err;
- if (!xdp_attachment_flags_ok(xdp, bpf))
- return -EBUSY;
-
if (bpf->command == XDP_SETUP_PROG && !ns->bpf_xdpdrv_accept) {
NSIM_EA(bpf->extack, "driver XDP disabled in DebugFS");
return -EOPNOTSUPP;
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 3814fb631d52..9dab2bc6f187 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -240,8 +240,6 @@ struct xdp_attachment_info {
};
struct netdev_bpf;
-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
- struct netdev_bpf *bpf);
void xdp_attachment_setup(struct xdp_attachment_info *info,
struct netdev_bpf *bpf);
diff --git a/net/core/dev.c b/net/core/dev.c
index 8588ade790cb..38412e70f761 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8917,6 +8917,17 @@ static struct bpf_prog *dev_xdp_prog(struct net_device *dev,
return dev->xdp_state[mode].prog;
}
+static u8 dev_xdp_prog_count(struct net_device *dev)
+{
+ u8 count = 0;
+ int i;
+
+ for (i = 0; i < __MAX_XDP_MODE; i++)
+ if (dev->xdp_state[i].prog || dev->xdp_state[i].link)
+ count++;
+ return count;
+}
+
u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
{
struct bpf_prog *prog = dev_xdp_prog(dev, mode);
@@ -9007,6 +9018,7 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
struct bpf_xdp_link *link, struct bpf_prog *new_prog,
struct bpf_prog *old_prog, u32 flags)
{
+ unsigned int num_modes = hweight32(flags & XDP_FLAGS_MODES);
struct bpf_prog *cur_prog;
enum bpf_xdp_mode mode;
bpf_op_t bpf_op;
@@ -9022,11 +9034,17 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
NL_SET_ERR_MSG(extack, "Invalid XDP flags for BPF link attachment");
return -EINVAL;
}
- /* just one XDP mode bit should be set, zero defaults to SKB mode */
- if (hweight32(flags & XDP_FLAGS_MODES) > 1) {
+ /* just one XDP mode bit should be set, zero defaults to drv/skb mode */
+ if (num_modes > 1) {
NL_SET_ERR_MSG(extack, "Only one XDP mode flag can be set");
return -EINVAL;
}
+ /* avoid ambiguity if offload + drv/skb mode progs are both loaded */
+ if (!num_modes && dev_xdp_prog_count(dev) > 1) {
+ NL_SET_ERR_MSG(extack,
+ "More than one program loaded, unset mode is ambiguous");
+ return -EINVAL;
+ }
/* old_prog != NULL implies XDP_FLAGS_REPLACE is set */
if (old_prog && !(flags & XDP_FLAGS_REPLACE)) {
NL_SET_ERR_MSG(extack, "XDP_FLAGS_REPLACE is not specified");
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 491ad569a79c..d900cebc0acd 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -403,18 +403,6 @@ void __xdp_release_frame(void *data, struct xdp_mem_info *mem)
}
EXPORT_SYMBOL_GPL(__xdp_release_frame);
-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
- struct netdev_bpf *bpf)
-{
- if (info->prog && (bpf->flags ^ info->flags) & XDP_FLAGS_MODES) {
- NL_SET_ERR_MSG(bpf->extack,
- "program loaded with different flags");
- return false;
- }
- return true;
-}
-EXPORT_SYMBOL_GPL(xdp_attachment_flags_ok);
-
void xdp_attachment_setup(struct xdp_attachment_info *info,
struct netdev_bpf *bpf)
{
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 2/7] selftests/bpf/test_offload.py: Remove check for program load flags match
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 1/7] xdp: remove the xdp_attachment_flags_ok() callback Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 3/7] netdevsim: Add debugfs toggle to reject BPF programs in verifier Toke Høiland-Jørgensen
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
Since we just removed the xdp_attachment_flags_ok() callback, also remove
the check for it in test_offload.py, and replace it with a test for the new
ambiguity-avoid check when multiple programs are loaded.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/test_offload.py | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 43c9cda199b8..becd27b2f4ba 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -716,13 +716,11 @@ def test_multi_prog(simdev, sim, obj, modename, modeid):
fail(ret == 0, "Replaced one of programs without -force")
check_extack(err, "XDP program already attached.", args)
- if modename == "" or modename == "drv":
- othermode = "" if modename == "drv" else "drv"
- start_test("Test multi-attachment XDP - detach...")
- ret, _, err = sim.unset_xdp(othermode, force=True,
- fail=False, include_stderr=True)
- fail(ret == 0, "Removed program with a bad mode")
- check_extack(err, "program loaded with different flags.", args)
+ start_test("Test multi-attachment XDP - remove without mode...")
+ ret, _, err = sim.unset_xdp("", force=True,
+ fail=False, include_stderr=True)
+ fail(ret == 0, "Removed program without a mode flag")
+ check_extack(err, "More than one program loaded, unset mode is ambiguous.", args)
sim.unset_xdp("offload")
xdp = sim.ip_link_show(xdp=True)["xdp"]
@@ -1001,16 +999,6 @@ try:
check_extack(err,
"native and generic XDP can't be active at the same time.",
args)
- ret, _, err = sim.set_xdp(obj, "", force=True,
- fail=False, include_stderr=True)
- fail(ret == 0, "Replaced XDP program with a program in different mode")
- check_extack(err, "program loaded with different flags.", args)
-
- start_test("Test XDP prog remove with bad flags...")
- ret, _, err = sim.unset_xdp("", force=True,
- fail=False, include_stderr=True)
- fail(ret == 0, "Removed program with a bad mode")
- check_extack(err, "program loaded with different flags.", args)
start_test("Test MTU restrictions...")
ret, _ = sim.set_mtu(9000, fail=False)
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 3/7] netdevsim: Add debugfs toggle to reject BPF programs in verifier
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 1/7] xdp: remove the xdp_attachment_flags_ok() callback Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 2/7] selftests/bpf/test_offload.py: Remove check for program load flags match Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 4/7] selftests/bpf/test_offload.py: only check verifier log on verification fails Toke Høiland-Jørgensen
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
This adds a new debugfs toggle ('bpf_bind_verifier_accept') that can be
used to make netdevsim reject BPF programs from being accepted by the
verifier. If this toggle (which defaults to true) is set to false,
nsim_bpf_verify_insn() will return EOPNOTSUPP on the last
instruction (after outputting the 'Hello from netdevsim' verifier message).
This makes it possible to check the verification callback in the driver
from test_offload.py in selftests, since the verifier now clears the
verifier log on a successful load, hiding the message from the driver.
Acked-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
drivers/net/netdevsim/bpf.c | 12 ++++++++++--
drivers/net/netdevsim/netdevsim.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 85546664bdd5..90aafb56f140 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -63,15 +63,20 @@ static int
nsim_bpf_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn)
{
struct nsim_bpf_bound_prog *state;
+ int ret = 0;
state = env->prog->aux->offload->dev_priv;
if (state->nsim_dev->bpf_bind_verifier_delay && !insn_idx)
msleep(state->nsim_dev->bpf_bind_verifier_delay);
- if (insn_idx == env->prog->len - 1)
+ if (insn_idx == env->prog->len - 1) {
pr_vlog(env, "Hello from netdevsim!\n");
- return 0;
+ if (!state->nsim_dev->bpf_bind_verifier_accept)
+ ret = -EOPNOTSUPP;
+ }
+
+ return ret;
}
static int nsim_bpf_finalize(struct bpf_verifier_env *env)
@@ -595,6 +600,9 @@ int nsim_bpf_dev_init(struct nsim_dev *nsim_dev)
&nsim_dev->bpf_bind_accept);
debugfs_create_u32("bpf_bind_verifier_delay", 0600, nsim_dev->ddir,
&nsim_dev->bpf_bind_verifier_delay);
+ nsim_dev->bpf_bind_verifier_accept = true;
+ debugfs_create_bool("bpf_bind_verifier_accept", 0600, nsim_dev->ddir,
+ &nsim_dev->bpf_bind_verifier_accept);
return 0;
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 827fc80f50a0..c4e7ad2a1964 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -189,6 +189,7 @@ struct nsim_dev {
struct dentry *take_snapshot;
struct bpf_offload_dev *bpf_dev;
bool bpf_bind_accept;
+ bool bpf_bind_verifier_accept;
u32 bpf_bind_verifier_delay;
struct dentry *ddir_bpf_bound_progs;
u32 prog_id_gen;
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 4/7] selftests/bpf/test_offload.py: only check verifier log on verification fails
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2020-12-04 11:52 ` [PATCH bpf v2 3/7] netdevsim: Add debugfs toggle to reject BPF programs in verifier Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 5/7] selftests/bpf/test_offload.py: fix expected case of extack messages Toke Høiland-Jørgensen
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
Since 6f8a57ccf85 ("bpf: Make verifier log more relevant by default"), the
verifier discards log messages for successfully-verified programs. This
broke test_offload.py which is looking for a verification message from the
driver callback. Change test_offload.py to use the toggle in netdevsim to
make the verification fail before looking for the verification message.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/test_offload.py | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index becd27b2f4ba..61527b43f067 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -911,11 +911,18 @@ try:
sim.tc_flush_filters()
+ start_test("Test TC offloads failure...")
+ sim.dfs["dev/bpf_bind_verifier_accept"] = 0
+ ret, _, err = sim.cls_bpf_add_filter(obj, verbose=True, skip_sw=True,
+ fail=False, include_stderr=True)
+ fail(ret == 0, "TC filter did not reject with TC offloads enabled")
+ check_verifier_log(err, "[netdevsim] Hello from netdevsim!")
+ sim.dfs["dev/bpf_bind_verifier_accept"] = 1
+
start_test("Test TC offloads work...")
ret, _, err = sim.cls_bpf_add_filter(obj, verbose=True, skip_sw=True,
fail=False, include_stderr=True)
fail(ret != 0, "TC filter did not load with TC offloads enabled")
- check_verifier_log(err, "[netdevsim] Hello from netdevsim!")
start_test("Test TC offload basics...")
dfs = simdev.dfs_get_bound_progs(expected=1)
@@ -1032,6 +1039,15 @@ try:
rm("/sys/fs/bpf/offload")
sim.wait_for_flush()
+ start_test("Test XDP load failure...")
+ sim.dfs["dev/bpf_bind_verifier_accept"] = 0
+ ret, _, err = bpftool_prog_load("sample_ret0.o", "/sys/fs/bpf/offload",
+ dev=sim['ifname'], fail=False, include_stderr=True)
+ fail(ret == 0, "verifier should fail on load")
+ check_verifier_log(err, "[netdevsim] Hello from netdevsim!")
+ sim.dfs["dev/bpf_bind_verifier_accept"] = 1
+ sim.wait_for_flush()
+
start_test("Test XDP offload...")
_, _, err = sim.set_xdp(obj, "offload", verbose=True, include_stderr=True)
ipl = sim.ip_link_show(xdp=True)
@@ -1039,7 +1055,6 @@ try:
progs = bpftool_prog_list(expected=1)
prog = progs[0]
fail(link_xdp["id"] != prog["id"], "Loaded program has wrong ID")
- check_verifier_log(err, "[netdevsim] Hello from netdevsim!")
start_test("Test XDP offload is device bound...")
dfs = simdev.dfs_get_bound_progs(expected=1)
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 5/7] selftests/bpf/test_offload.py: fix expected case of extack messages
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
` (3 preceding siblings ...)
2020-12-04 11:52 ` [PATCH bpf v2 4/7] selftests/bpf/test_offload.py: only check verifier log on verification fails Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 6/7] selftests/bpf/test_offload.py: reset ethtool features after failed setting Toke Høiland-Jørgensen
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
Commit 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs
in net_device") changed the case of some of the extack messages being
returned when attaching of XDP programs failed. This broke test_offload.py,
so let's fix the test to reflect this.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/test_offload.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 61527b43f067..51a5e4d939cc 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -1004,7 +1004,7 @@ try:
fail=False, include_stderr=True)
fail(ret == 0, "Replaced XDP program with a program in different mode")
check_extack(err,
- "native and generic XDP can't be active at the same time.",
+ "Native and generic XDP can't be active at the same time.",
args)
start_test("Test MTU restrictions...")
@@ -1035,7 +1035,7 @@ try:
offload = bpf_pinned("/sys/fs/bpf/offload")
ret, _, err = sim.set_xdp(offload, "drv", fail=False, include_stderr=True)
fail(ret == 0, "attached offloaded XDP program to drv")
- check_extack(err, "using device-bound program without HW_MODE flag is not supported.", args)
+ check_extack(err, "Using device-bound program without HW_MODE flag is not supported.", args)
rm("/sys/fs/bpf/offload")
sim.wait_for_flush()
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 6/7] selftests/bpf/test_offload.py: reset ethtool features after failed setting
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
` (4 preceding siblings ...)
2020-12-04 11:52 ` [PATCH bpf v2 5/7] selftests/bpf/test_offload.py: fix expected case of extack messages Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-04 11:52 ` [PATCH bpf v2 7/7] selftests/bpf/test_offload.py: filter bpftool internal map when counting maps Toke Høiland-Jørgensen
2020-12-08 14:18 ` [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
When setting the ethtool feature flag fails (as expected for the test), the
kernel now tracks that the feature was requested to be 'off' and refuses to
subsequently disable it again. So reset it back to 'on' so a subsequent
disable (that's not supposed to fail) can succeed.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/test_offload.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 51a5e4d939cc..2128fbd8414b 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -946,6 +946,7 @@ try:
start_test("Test disabling TC offloads is rejected while filters installed...")
ret, _ = sim.set_ethtool_tc_offloads(False, fail=False)
fail(ret == 0, "Driver should refuse to disable TC offloads with filters installed...")
+ sim.set_ethtool_tc_offloads(True)
start_test("Test qdisc removal frees things...")
sim.tc_flush_filters()
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf v2 7/7] selftests/bpf/test_offload.py: filter bpftool internal map when counting maps
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
` (5 preceding siblings ...)
2020-12-04 11:52 ` [PATCH bpf v2 6/7] selftests/bpf/test_offload.py: reset ethtool features after failed setting Toke Høiland-Jørgensen
@ 2020-12-04 11:52 ` Toke Høiland-Jørgensen
2020-12-08 14:18 ` [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
7 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-04 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
From: Toke Høiland-Jørgensen <toke@redhat.com>
A few of the tests in test_offload.py expects to see a certain number of
maps created, and checks this by counting the number of maps returned by
bpftool. There is already a filter that will remove any maps already there
at the beginning of the test, but bpftool now creates a map for the PID
iterator rodata on each invocation, which makes the map count wrong. Fix
this by also filtering the pid_iter.rodata map by name when counting.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/test_offload.py | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 2128fbd8414b..b99bb8ed3ed4 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -184,9 +184,7 @@ def bpftool_prog_list(expected=None, ns=""):
def bpftool_map_list(expected=None, ns=""):
_, maps = bpftool("map show", JSON=True, ns=ns, fail=True)
# Remove the base maps
- for m in base_maps:
- if m in maps:
- maps.remove(m)
+ maps = [m for m in maps if m not in base_maps and m.get('name') not in base_map_names]
if expected is not None:
if len(maps) != expected:
fail(True, "%d BPF maps loaded, expected %d" %
@@ -770,6 +768,9 @@ ret, progs = bpftool("prog", fail=False)
skip(ret != 0, "bpftool not installed")
base_progs = progs
_, base_maps = bpftool("map")
+base_map_names = [
+ 'pid_iter.rodata' # created on each bpftool invocation
+]
# Check netdevsim
ret, out = cmd("modprobe netdevsim", fail=False)
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order
2020-12-04 11:52 [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
` (6 preceding siblings ...)
2020-12-04 11:52 ` [PATCH bpf v2 7/7] selftests/bpf/test_offload.py: filter bpftool internal map when counting maps Toke Høiland-Jørgensen
@ 2020-12-08 14:18 ` Toke Høiland-Jørgensen
2020-12-08 17:03 ` Jakub Kicinski
7 siblings, 1 reply; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-08 14:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
Toke Høiland-Jørgensen <toke@redhat.com> writes:
> This series restores the test_offload.py selftest to working order. It seems a
> number of subtle behavioural changes have crept into various subsystems which
> broke test_offload.py in a number of ways. Most of these are fairly benign
> changes where small adjustments to the test script seems to be the best fix, but
> one is an actual kernel bug that I've observed in the wild caused by a bad
> interaction between xdp_attachment_flags_ok() and the rework of XDP program
> handling in the core netdev code.
>
> Patch 1 fixes the bug by removing xdp_attachment_flags_ok(), and the reminder of
> the patches are adjustments to test_offload.py, including a new feature for
> netdevsim to force a BPF verification fail. Please see the individual patches
> for details.
>
> Changelog:
>
> v2:
> - Replace xdp_attachment_flags_ok() with a check in dev_xdp_attach()
> - Better packing of struct nsim_dev
Hi Jakub
Any feedback on v2? Would be great to get it merged before the final
5.10 release :)
-Toke
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order
2020-12-08 14:18 ` [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order Toke Høiland-Jørgensen
@ 2020-12-08 17:03 ` Jakub Kicinski
2020-12-08 22:00 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2020-12-08 17:03 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
On Tue, 08 Dec 2020 15:18:31 +0100 Toke Høiland-Jørgensen wrote:
> Toke Høiland-Jørgensen <toke@redhat.com> writes:
>
> > This series restores the test_offload.py selftest to working order. It seems a
> > number of subtle behavioural changes have crept into various subsystems which
> > broke test_offload.py in a number of ways. Most of these are fairly benign
> > changes where small adjustments to the test script seems to be the best fix, but
> > one is an actual kernel bug that I've observed in the wild caused by a bad
> > interaction between xdp_attachment_flags_ok() and the rework of XDP program
> > handling in the core netdev code.
> >
> > Patch 1 fixes the bug by removing xdp_attachment_flags_ok(), and the reminder of
> > the patches are adjustments to test_offload.py, including a new feature for
> > netdevsim to force a BPF verification fail. Please see the individual patches
> > for details.
> >
> > Changelog:
> >
> > v2:
> > - Replace xdp_attachment_flags_ok() with a check in dev_xdp_attach()
> > - Better packing of struct nsim_dev
>
> Any feedback on v2? Would be great to get it merged before the final
> 5.10 release :)
LGTM but if my opinion mattered this could would not have been changed
in the first place :)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf v2 0/7] selftests/bpf: Restore test_offload.py to working order
2020-12-08 17:03 ` Jakub Kicinski
@ 2020-12-08 22:00 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-08 22:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Jesper Dangaard Brouer,
Michael S. Tsirkin, Romain Perier, Allen Pais, Grygorii Strashko,
Simon Horman, Gustavo A. R. Silva, Lorenzo Bianconi, Wei Yongjun,
Jiri Benc, oss-drivers, linux-omap, netdev, bpf
Jakub Kicinski <kuba@kernel.org> writes:
> On Tue, 08 Dec 2020 15:18:31 +0100 Toke Høiland-Jørgensen wrote:
>> Toke Høiland-Jørgensen <toke@redhat.com> writes:
>>
>> > This series restores the test_offload.py selftest to working order. It seems a
>> > number of subtle behavioural changes have crept into various subsystems which
>> > broke test_offload.py in a number of ways. Most of these are fairly benign
>> > changes where small adjustments to the test script seems to be the best fix, but
>> > one is an actual kernel bug that I've observed in the wild caused by a bad
>> > interaction between xdp_attachment_flags_ok() and the rework of XDP program
>> > handling in the core netdev code.
>> >
>> > Patch 1 fixes the bug by removing xdp_attachment_flags_ok(), and the reminder of
>> > the patches are adjustments to test_offload.py, including a new feature for
>> > netdevsim to force a BPF verification fail. Please see the individual patches
>> > for details.
>> >
>> > Changelog:
>> >
>> > v2:
>> > - Replace xdp_attachment_flags_ok() with a check in dev_xdp_attach()
>> > - Better packing of struct nsim_dev
>>
>> Any feedback on v2? Would be great to get it merged before the final
>> 5.10 release :)
>
> LGTM but if my opinion mattered this could would not have been changed
> in the first place :)
Heh, right. Well I, for one, value your input, so thanks for taking a
look :)
-Toke
^ permalink raw reply [flat|nested] 11+ messages in thread