[PATCH bpf-next 0/5] net: xdp: allow offload to coexist with generic

[PATCH bpf-next 0/5] net: xdp: allow offload to coexist with generic

[Jakub Kicinski]

Hi!

Offloaded and native/driver XDP programs can already coexist.
Allow offload and generic hook to coexist as well, there seem
to be no reason why not to do so.

Jakub Kicinski (5):
  selftests/bpf: fix the expected messages
  net: xdp: allow generic and driver XDP on one interface
  selftests/bpf: print traceback when test fails
  selftests/bpf: add test for mixing generic and offload XDP
  selftests/bpf: test reading the offloaded program

 net/core/dev.c                              |  10 +-
 tools/testing/selftests/bpf/test_offload.py | 135 ++++++++++++--------
 2 files changed, 85 insertions(+), 60 deletions(-)

-- 
2.19.2

[PATCH bpf-next 1/5] selftests/bpf: fix the expected messages

[Jakub Kicinski]

Recent changes added extack to program replacement path,
expect extack instead of generic messages.

Fixes: 01dde20ce04b ("xdp: Provide extack messages when prog attachment failed")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index d59642e70f56..a564d1a5a1b8 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -842,7 +842,9 @@ netns = []
     ret, _, err = sim.set_xdp(obj, "generic", force=True,
                               fail=False, include_stderr=True)
     fail(ret == 0, "Replaced XDP program with a program in different mode")
-    fail(err.count("File exists") != 1, "Replaced driver XDP with generic")
+    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")
@@ -957,7 +959,8 @@ netns = []
 
     start_test("Test multi-attachment XDP - replace...")
     ret, _, err = sim.set_xdp(obj, "offload", fail=False, include_stderr=True)
-    fail(err.count("busy") != 1, "Replaced one of programs without -force")
+    fail(ret == 0, "Replaced one of programs without -force")
+    check_extack(err, "XDP program already attached.", args)
 
     start_test("Test multi-attachment XDP - detach...")
     ret, _, err = sim.unset_xdp("drv", force=True,
-- 
2.19.2

[PATCH bpf-next 2/5] net: xdp: allow generic and driver XDP on one interface

[Jakub Kicinski]

Since commit a25717d2b604 ("xdp: support simultaneous driver and
hw XDP attachment") users can load an XDP program for offload and
in native driver mode simultaneously.  Allow a similar mix of
offload and SKB mode/generic XDP.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 net/core/dev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index bfa4be42afff..78c3b48392e1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7976,11 +7976,13 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 	enum bpf_netdev_command query;
 	struct bpf_prog *prog = NULL;
 	bpf_op_t bpf_op, bpf_chk;
+	bool offload;
 	int err;
 
 	ASSERT_RTNL();
 
-	query = flags & XDP_FLAGS_HW_MODE ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+	offload = flags & XDP_FLAGS_HW_MODE;
+	query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
 
 	bpf_op = bpf_chk = ops->ndo_bpf;
 	if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
@@ -7993,8 +7995,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		bpf_chk = generic_xdp_install;
 
 	if (fd >= 0) {
-		if (__dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG) ||
-		    __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG_HW)) {
+		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
 			NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
 			return -EEXIST;
 		}
@@ -8009,8 +8010,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
-		if (!(flags & XDP_FLAGS_HW_MODE) &&
-		    bpf_prog_is_dev_bound(prog->aux)) {
+		if (!offload && bpf_prog_is_dev_bound(prog->aux)) {
 			NL_SET_ERR_MSG(extack, "using device-bound program without HW_MODE flag is not supported");
 			bpf_prog_put(prog);
 			return -EINVAL;
-- 
2.19.2

[PATCH bpf-next 3/5] selftests/bpf: print traceback when test fails

[Jakub Kicinski]

Figuring out which exact check in test_offload.py takes more
time than it should.  Print the traceback (to the screen and
the logs).

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index a564d1a5a1b8..c379b36a5443 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -23,6 +23,7 @@ import string
 import struct
 import subprocess
 import time
+import traceback
 
 logfile = None
 log_level = 1
@@ -78,7 +79,9 @@ netns = [] # net namespaces to be removed
     if not cond:
         return
     print("FAIL: " + msg)
-    log("FAIL: " + msg, "", level=1)
+    tb = "".join(traceback.extract_stack().format())
+    print(tb)
+    log("FAIL: " + msg, tb, level=1)
     os.sys.exit(1)
 
 def start_test(msg):
-- 
2.19.2

[PATCH bpf-next 4/5] selftests/bpf: add test for mixing generic and offload XDP

[Jakub Kicinski]

Add simple sanity check for enabling generic and offload
XDP, simply reuse the native and offload checks.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 116 +++++++++++---------
 1 file changed, 62 insertions(+), 54 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index c379b36a5443..13fe12d09704 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -603,6 +603,65 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
                             include_stderr=True)
     check_no_extack(res, needle)
 
+def test_multi_prog(sim, obj, modename, modeid):
+    start_test("Test multi-attachment XDP - %s + offload..." %
+               (modename or "default", ))
+    sim.set_xdp(obj, "offload")
+    xdp = sim.ip_link_show(xdp=True)["xdp"]
+    offloaded = sim.dfs_read("bpf_offloaded_id")
+    fail("prog" not in xdp, "Base program not reported in single program mode")
+    fail(len(xdp["attached"]) != 1,
+         "Wrong attached program count with one program")
+
+    sim.set_xdp(obj, modename)
+    two_xdps = sim.ip_link_show(xdp=True)["xdp"]
+    offloaded2 = sim.dfs_read("bpf_offloaded_id")
+
+    fail(two_xdps["mode"] != 4, "Bad mode reported with multiple programs")
+    fail("prog" in two_xdps, "Base program reported in multi program mode")
+    fail(xdp["attached"][0] not in two_xdps["attached"],
+         "Offload program not reported after other activated")
+    fail(len(two_xdps["attached"]) != 2,
+         "Wrong attached program count with two programs")
+    fail(two_xdps["attached"][0]["prog"]["id"] ==
+         two_xdps["attached"][1]["prog"]["id"],
+         "Offloaded and other programs have the same id")
+    fail(offloaded != offloaded2,
+         "Offload ID changed after loading other program")
+
+    start_test("Test multi-attachment XDP - replace...")
+    ret, _, err = sim.set_xdp(obj, "offload", fail=False, include_stderr=True)
+    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)
+
+    sim.unset_xdp("offload")
+    xdp = sim.ip_link_show(xdp=True)["xdp"]
+    offloaded = sim.dfs_read("bpf_offloaded_id")
+
+    fail(xdp["mode"] != modeid, "Bad mode reported after multiple programs")
+    fail("prog" not in xdp,
+         "Base program not reported after multi program mode")
+    fail(xdp["attached"][0] not in two_xdps["attached"],
+         "Offload program not reported after other activated")
+    fail(len(xdp["attached"]) != 1,
+         "Wrong attached program count with remaining programs")
+    fail(offloaded != "0", "Offload ID reported with only other program left")
+
+    start_test("Test multi-attachment XDP - device remove...")
+    sim.set_xdp(obj, "offload")
+    sim.remove()
+
+    sim = NetdevSim()
+    sim.set_ethtool_tc_offloads(True)
+    return sim
 
 # Parse command line
 parser = argparse.ArgumentParser()
@@ -936,60 +995,9 @@ netns = []
     rm(pin_file)
     bpftool_prog_list_wait(expected=0)
 
-    start_test("Test multi-attachment XDP - attach...")
-    sim.set_xdp(obj, "offload")
-    xdp = sim.ip_link_show(xdp=True)["xdp"]
-    offloaded = sim.dfs_read("bpf_offloaded_id")
-    fail("prog" not in xdp, "Base program not reported in single program mode")
-    fail(len(ipl["xdp"]["attached"]) != 1,
-         "Wrong attached program count with one program")
-
-    sim.set_xdp(obj, "")
-    two_xdps = sim.ip_link_show(xdp=True)["xdp"]
-    offloaded2 = sim.dfs_read("bpf_offloaded_id")
-
-    fail(two_xdps["mode"] != 4, "Bad mode reported with multiple programs")
-    fail("prog" in two_xdps, "Base program reported in multi program mode")
-    fail(xdp["attached"][0] not in two_xdps["attached"],
-         "Offload program not reported after driver activated")
-    fail(len(two_xdps["attached"]) != 2,
-         "Wrong attached program count with two programs")
-    fail(two_xdps["attached"][0]["prog"]["id"] ==
-         two_xdps["attached"][1]["prog"]["id"],
-         "offloaded and drv programs have the same id")
-    fail(offloaded != offloaded2,
-         "offload ID changed after loading driver program")
-
-    start_test("Test multi-attachment XDP - replace...")
-    ret, _, err = sim.set_xdp(obj, "offload", fail=False, include_stderr=True)
-    fail(ret == 0, "Replaced one of programs without -force")
-    check_extack(err, "XDP program already attached.", args)
-
-    start_test("Test multi-attachment XDP - detach...")
-    ret, _, err = sim.unset_xdp("drv", 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)
-
-    sim.unset_xdp("offload")
-    xdp = sim.ip_link_show(xdp=True)["xdp"]
-    offloaded = sim.dfs_read("bpf_offloaded_id")
-
-    fail(xdp["mode"] != 1, "Bad mode reported after multiple programs")
-    fail("prog" not in xdp,
-         "Base program not reported after multi program mode")
-    fail(xdp["attached"][0] not in two_xdps["attached"],
-         "Offload program not reported after driver activated")
-    fail(len(ipl["xdp"]["attached"]) != 1,
-         "Wrong attached program count with remaining programs")
-    fail(offloaded != "0", "offload ID reported with only driver program left")
-
-    start_test("Test multi-attachment XDP - device remove...")
-    sim.set_xdp(obj, "offload")
-    sim.remove()
-
-    sim = NetdevSim()
-    sim.set_ethtool_tc_offloads(True)
+    sim = test_multi_prog(sim, obj, "", 1)
+    sim = test_multi_prog(sim, obj, "drv", 1)
+    sim = test_multi_prog(sim, obj, "generic", 2)
 
     start_test("Test mixing of TC and XDP...")
     sim.tc_add_ingress()
-- 
2.19.2

[PATCH bpf-next 5/5] selftests/bpf: test reading the offloaded program

[Jakub Kicinski]

Test adding the offloaded program after the other program
is already installed.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 29 ++++++++++++++-------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 13fe12d09704..84bea3985d64 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -592,6 +592,15 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
             return
     fail(True, "Missing or incorrect message from netdevsim in verifier log")
 
+def check_multi_basic(two_xdps):
+    fail(two_xdps["mode"] != 4, "Bad mode reported with multiple programs")
+    fail("prog" in two_xdps, "Base program reported in multi program mode")
+    fail(len(two_xdps["attached"]) != 2,
+         "Wrong attached program count with two programs")
+    fail(two_xdps["attached"][0]["prog"]["id"] ==
+         two_xdps["attached"][1]["prog"]["id"],
+         "Offloaded and other programs have the same id")
+
 def test_spurios_extack(sim, obj, skip_hw, needle):
     res = sim.cls_bpf_add_filter(obj, prio=1, handle=1, skip_hw=skip_hw,
                                  include_stderr=True)
@@ -615,17 +624,12 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
 
     sim.set_xdp(obj, modename)
     two_xdps = sim.ip_link_show(xdp=True)["xdp"]
-    offloaded2 = sim.dfs_read("bpf_offloaded_id")
 
-    fail(two_xdps["mode"] != 4, "Bad mode reported with multiple programs")
-    fail("prog" in two_xdps, "Base program reported in multi program mode")
     fail(xdp["attached"][0] not in two_xdps["attached"],
          "Offload program not reported after other activated")
-    fail(len(two_xdps["attached"]) != 2,
-         "Wrong attached program count with two programs")
-    fail(two_xdps["attached"][0]["prog"]["id"] ==
-         two_xdps["attached"][1]["prog"]["id"],
-         "Offloaded and other programs have the same id")
+    check_multi_basic(two_xdps)
+
+    offloaded2 = sim.dfs_read("bpf_offloaded_id")
     fail(offloaded != offloaded2,
          "Offload ID changed after loading other program")
 
@@ -655,8 +659,15 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
          "Wrong attached program count with remaining programs")
     fail(offloaded != "0", "Offload ID reported with only other program left")
 
-    start_test("Test multi-attachment XDP - device remove...")
+    start_test("Test multi-attachment XDP - reattach...")
     sim.set_xdp(obj, "offload")
+    two_xdps = sim.ip_link_show(xdp=True)["xdp"]
+
+    fail(xdp["attached"][0] not in two_xdps["attached"],
+         "Other program not reported after offload activated")
+    check_multi_basic(two_xdps)
+
+    start_test("Test multi-attachment XDP - device remove...")
     sim.remove()
 
     sim = NetdevSim()
-- 
2.19.2

Re: [PATCH bpf-next 0/5] net: xdp: allow offload to coexist with generic

[Daniel Borkmann]

On 02/06/2019 05:03 AM, Jakub Kicinski wrote:
> Hi!
> 
> Offloaded and native/driver XDP programs can already coexist.
> Allow offload and generic hook to coexist as well, there seem
> to be no reason why not to do so.
> 
> Jakub Kicinski (5):
>   selftests/bpf: fix the expected messages
>   net: xdp: allow generic and driver XDP on one interface
>   selftests/bpf: print traceback when test fails
>   selftests/bpf: add test for mixing generic and offload XDP
>   selftests/bpf: test reading the offloaded program
> 
>  net/core/dev.c                              |  10 +-
>  tools/testing/selftests/bpf/test_offload.py | 135 ++++++++++++--------
>  2 files changed, 85 insertions(+), 60 deletions(-)
> 

Applied, thanks!