samples/bpf: Attach XDP programs in driver mode by default

When attaching XDP programs, userspace can set flags to request the attach
mode (generic/SKB mode, driver mode or hw offloaded mode). If no such flags
are requested, the kernel will attempt to attach in driver mode, and then
silently fall back to SKB mode if this fails.

The silent fallback is a major source of user confusion, as users will try
to load a program on a device without XDP support, and instead of an error
they will get the silent fallback behaviour, not notice, and then wonder
why performance is not what they were expecting.

In an attempt to combat this, let's switch all the samples to default to
explicitly requesting driver-mode attach. As part of this, ensure that all
the userspace utilities have a switch to enable SKB mode. For those that
have a switch to request driver mode, keep it but turn it into a no-op.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: David Ahern <dsahern@gmail.com>
Link: https://lore.kernel.org/bpf/20191216110742.364456-1-toke@redhat.com

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 3e553eed95a77aadbd588c9372b0bb4e4e315d65..38a8852cb57f51688f3b404d7d294ec907f0f04d 100644 (file)
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -98,7 +98,7 @@ int main(int argc, char **argv)
                        xdp_flags |= XDP_FLAGS_SKB_MODE;
                        break;
                case 'N':
-                       xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -109,6 +109,9 @@ int main(int argc, char **argv)
                }
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == argc) {
                usage(basename(argv[0]));
                return 1;

diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index d86e9ad0356bd60131ab49359968412d603a5a25..008789eb6adad060c8330adb030d7d399a4460cf 100644 (file)
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -120,7 +120,7 @@ int main(int argc, char **argv)
                        xdp_flags |= XDP_FLAGS_SKB_MODE;
                        break;
                case 'N':
-                       xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -132,6 +132,9 @@ int main(int argc, char **argv)
                opt_flags[opt] = 0;
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        for (i = 0; i < strlen(optstr); i++) {
                if (opt_flags[(unsigned int)optstr[i]]) {
                        fprintf(stderr, "Missing argument -%c\n", optstr[i]);

diff --git a/samples/bpf/xdp_fwd_user.c b/samples/bpf/xdp_fwd_user.c
index 97ff1dad766978601eaaf2da9a5a6752b86cfd98..c30f9acfdb84ddcda784ad57be0d662f9e34c574 100644 (file)
--- a/samples/bpf/xdp_fwd_user.c
+++ b/samples/bpf/xdp_fwd_user.c
@@ -27,11 +27,13 @@
 #include "libbpf.h"
 #include <bpf/bpf.h>
 
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+
 static int do_attach(int idx, int prog_fd, int map_fd, const char *name)
 {
        int err;
 
-       err = bpf_set_link_xdp_fd(idx, prog_fd, 0);
+       err = bpf_set_link_xdp_fd(idx, prog_fd, xdp_flags);
        if (err < 0) {
                printf("ERROR: failed to attach program to %s\n", name);
                return err;
@@ -49,7 +51,7 @@ static int do_detach(int idx, const char *name)
 {
        int err;
 
-       err = bpf_set_link_xdp_fd(idx, -1, 0);
+       err = bpf_set_link_xdp_fd(idx, -1, xdp_flags);
        if (err < 0)
                printf("ERROR: failed to detach program from %s\n", name);
 
@@ -83,11 +85,17 @@ int main(int argc, char **argv)
        int attach = 1;
        int ret = 0;
 
-       while ((opt = getopt(argc, argv, ":dD")) != -1) {
+       while ((opt = getopt(argc, argv, ":dDSF")) != -1) {
                switch (opt) {
                case 'd':
                        attach = 0;
                        break;
+               case 'S':
+                       xdp_flags |= XDP_FLAGS_SKB_MODE;
+                       break;
+               case 'F':
+                       xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+                       break;
                case 'D':
                        prog_name = "xdp_fwd_direct";
                        break;
@@ -97,6 +105,9 @@ int main(int argc, char **argv)
                }
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == argc) {
                usage(basename(argv[0]));
                return 1;

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 0da6e9e7132ecdc5a01b3d4ea10f33de5f5b5b83..72af628529b51f171c26c746561b6aa2d526d66f 100644 (file)
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -728,6 +728,10 @@ int main(int argc, char **argv)
                        return EXIT_FAIL_OPTION;
                }
        }
+
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        /* Required option */
        if (ifindex == -1) {
                fprintf(stderr, "ERR: required option --dev missing\n");

diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index f70ee33907fde0e845437d5403c7b8acc995b859..cc840661faabc909d41f9dd679db903129ad3467 100644 (file)
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -116,7 +116,7 @@ int main(int argc, char **argv)
                        xdp_flags |= XDP_FLAGS_SKB_MODE;
                        break;
                case 'N':
-                       xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -127,6 +127,9 @@ int main(int argc, char **argv)
                }
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == argc) {
                printf("usage: %s <IFNAME|IFINDEX>_IN <IFNAME|IFINDEX>_OUT\n", argv[0]);
                return 1;

diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index 5440cd620607fbee2e9c81b5640424d48e09b658..71dff8e3382a884f0097b776ed6322191f2003a2 100644 (file)
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -117,7 +117,7 @@ int main(int argc, char **argv)
                        xdp_flags |= XDP_FLAGS_SKB_MODE;
                        break;
                case 'N':
-                       xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -128,6 +128,9 @@ int main(int argc, char **argv)
                }
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == argc) {
                printf("usage: %s <IFNAME|IFINDEX>_IN <IFNAME|IFINDEX>_OUT\n", argv[0]);
                return 1;

diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index 1469b66ebad1ec13588e3069748c8e4d13124e80..fef286c5add2d8b62c88fdb5c5048e06c93ec8e3 100644 (file)
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -662,6 +662,9 @@ int main(int ac, char **argv)
                }
        }
 
+       if (!(flags & XDP_FLAGS_SKB_MODE))
+               flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == ac) {
                usage(basename(argv[0]));
                return 1;

diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 8fc3ad01de72c7bb8cf1c4cec0a4cbf17634758f..fc4983fd69590af0c45d1db9087f01aa99c8c56d 100644 (file)
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -551,6 +551,10 @@ int main(int argc, char **argv)
                        return EXIT_FAIL_OPTION;
                }
        }
+
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        /* Required option */
        if (ifindex == -1) {
                fprintf(stderr, "ERR: required option --dev missing\n");

diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index a5760e8bf2c44aa4869c4d01b2adce6c341fa166..8c1af1b7372d4c733c9d39865456a5969e7fb4d8 100644 (file)
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -52,13 +52,13 @@ static int do_detach(int idx, const char *name)
        __u32 curr_prog_id = 0;
        int err = 0;
 
-       err = bpf_get_link_xdp_id(idx, &curr_prog_id, 0);
+       err = bpf_get_link_xdp_id(idx, &curr_prog_id, xdp_flags);
        if (err) {
                printf("bpf_get_link_xdp_id failed\n");
                return err;
        }
        if (prog_id == curr_prog_id) {
-               err = bpf_set_link_xdp_fd(idx, -1, 0);
+               err = bpf_set_link_xdp_fd(idx, -1, xdp_flags);
                if (err < 0)
                        printf("ERROR: failed to detach prog from %s\n", name);
        } else if (!curr_prog_id) {
@@ -115,7 +115,7 @@ int main(int argc, char **argv)
                .prog_type      = BPF_PROG_TYPE_XDP,
        };
        struct perf_buffer_opts pb_opts = {};
-       const char *optstr = "F";
+       const char *optstr = "FS";
        int prog_fd, map_fd, opt;
        struct bpf_object *obj;
        struct bpf_map *map;
@@ -127,12 +127,18 @@ int main(int argc, char **argv)
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
                        break;
+               case 'S':
+                       xdp_flags |= XDP_FLAGS_SKB_MODE;
+                       break;
                default:
                        usage(basename(argv[0]));
                        return 1;
                }
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        if (optind == argc) {
                usage(basename(argv[0]));
                return 1;

diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index 2fe4c7f5ffe5e80b0e548f3051412683811af6ea..5f33b553003238259f85ece33521d103f6387426 100644 (file)
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -231,7 +231,7 @@ int main(int argc, char **argv)
                        xdp_flags |= XDP_FLAGS_SKB_MODE;
                        break;
                case 'N':
-                       xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'F':
                        xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -243,6 +243,9 @@ int main(int argc, char **argv)
                opt_flags[opt] = 0;
        }
 
+       if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+               xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        for (i = 0; i < strlen(optstr); i++) {
                if (opt_flags[(unsigned int)optstr[i]]) {
                        fprintf(stderr, "Missing argument -%c\n", optstr[i]);

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index a15480010828fbe53c71692af71d4c958166979c..e7829e5baaffdcd33ec15f3c62b7664c0506a669 100644 (file)
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -440,7 +440,7 @@ static void parse_command_line(int argc, char **argv)
                        opt_xdp_bind_flags |= XDP_COPY;
                        break;
                case 'N':
-                       opt_xdp_flags |= XDP_FLAGS_DRV_MODE;
+                       /* default, set below */
                        break;
                case 'n':
                        opt_interval = atoi(optarg);
@@ -474,6 +474,9 @@ static void parse_command_line(int argc, char **argv)
                }
        }
 
+       if (!(opt_xdp_flags & XDP_FLAGS_SKB_MODE))
+               opt_xdp_flags |= XDP_FLAGS_DRV_MODE;
+
        opt_ifindex = if_nametoindex(opt_if);
        if (!opt_ifindex) {
                fprintf(stderr, "ERROR: interface \"%s\" does not exist\n",