| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZGs/wOvSaM3Ackzx@lincoln>
Date: Mon, 22 May 2023 12:11:12 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Quentin Monnet <quentin@...valent.com>
CC: Stanislav Fomichev <sdf@...gle.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>, "Yonghong Song" <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, <bpf@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Niklas Söderlund
<niklas.soderlund@...igine.com>
Subject: Re: [PATCH bpf-next v2] bpftool: specify XDP Hints ifname when
loading program
bpf/vmtest-bpf-next-VM_Test-16 [0] has failed without any log.
Maybe it would make sense to rerun it?
[0] https://github.com/kernel-patches/bpf/actions/runs/5030713342/jobs/9023468565
On Wed, May 17, 2023 at 06:01:04PM +0200, Larysa Zaremba wrote:
> Add ability to specify a network interface used to resolve
> XDP Hints kfuncs when loading program through bpftool.
>
> Usage:
> bpftool prog load [...] xdpmeta_dev <ifname>
>
> Writing just 'dev <ifname>' instead of 'xdpmeta_dev' is a very probable
> mistake that results in not very descriptive errors,
> so 'bpftool prog load [...] dev <ifname>' syntax becomes deprecated,
> followed by 'bpftool map create [...] dev <ifname>' for consistency.
>
> Now, to offload program, execute:
> bpftool prog load [...] offload_dev <ifname>
>
> To offload map:
> bpftool map create [...] offload_dev <ifname>
>
> 'dev <ifname>' still performs offloading in the commands above, but now
> triggers a warning and is excluded from bash completion.
>
> 'xdpmeta_dev' and 'offload_dev' are mutually exclusive options, because
> 'xdpmeta_dev' basically makes a program device-bound without loading it
> onto the said device. For now, offloaded programs cannot use XDP hints [0],
> but if this changes, using 'offload_dev <ifname>' should cover this case.
>
> [0] https://lore.kernel.org/bpf/a5a636cc-5b03-686f-4be0-000383b05cfc@linux.dev
>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> ---
> Changes since v1:
> - changed syntax from 'dev xdpmeta <ifname>' to 'xdpmeta_dev <ifname>'
> - added 'offload_dev'
> - deprecated 'bpftool prog load [...] dev <ifname>' syntax
> - deprecated 'bpftool map create [...] dev <ifname>' syntax
> - updated usage message in prog.c
> - updated usage message in map.c
> - updated documentation
> - updated bash completion
> - mutual exclusivity of xdpmeta_dev and offload_dev [0] is now indicated
> by errors and syntax documentation
> ---
> .../bpf/bpftool/Documentation/bpftool-map.rst | 8 +--
> .../bpftool/Documentation/bpftool-prog.rst | 11 ++--
> tools/bpf/bpftool/bash-completion/bpftool | 7 +--
> tools/bpf/bpftool/map.c | 7 ++-
> tools/bpf/bpftool/prog.c | 51 ++++++++++++++++---
> 5 files changed, 64 insertions(+), 20 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> index 11250c4734fe..3b7ba037af95 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> @@ -28,7 +28,7 @@ MAP COMMANDS
> | **bpftool** **map** { **show** | **list** } [*MAP*]
> | **bpftool** **map create** *FILE* **type** *TYPE* **key** *KEY_SIZE* **value** *VALUE_SIZE* \
> | **entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**inner_map** *MAP*] \
> -| [**dev** *NAME*]
> +| [**offload_dev** *NAME*]
> | **bpftool** **map dump** *MAP*
> | **bpftool** **map update** *MAP* [**key** *DATA*] [**value** *VALUE*] [*UPDATE_FLAGS*]
> | **bpftool** **map lookup** *MAP* [**key** *DATA*]
> @@ -73,7 +73,7 @@ DESCRIPTION
> maps. On such kernels bpftool will automatically emit this
> information as well.
>
> - **bpftool map create** *FILE* **type** *TYPE* **key** *KEY_SIZE* **value** *VALUE_SIZE* **entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**inner_map** *MAP*] [**dev** *NAME*]
> + **bpftool map create** *FILE* **type** *TYPE* **key** *KEY_SIZE* **value** *VALUE_SIZE* **entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**inner_map** *MAP*] [**offload_dev** *NAME*]
> Create a new map with given parameters and pin it to *bpffs*
> as *FILE*.
>
> @@ -86,8 +86,8 @@ DESCRIPTION
> kernel needs it to collect metadata related to the inner maps
> that the new map will work with.
>
> - Keyword **dev** expects a network interface name, and is used
> - to request hardware offload for the map.
> + Keyword **offload_dev** expects a network interface name,
> + and is used to request hardware offload for the map.
>
> **bpftool map dump** *MAP*
> Dump all entries in a given *MAP*. In case of **name**,
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 9443c524bb76..dcae81bd27ed 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -31,7 +31,7 @@ PROG COMMANDS
> | **bpftool** **prog dump xlated** *PROG* [{ **file** *FILE* | [**opcodes**] [**linum**] [**visual**] }]
> | **bpftool** **prog dump jited** *PROG* [{ **file** *FILE* | [**opcodes**] [**linum**] }]
> | **bpftool** **prog pin** *PROG* *FILE*
> -| **bpftool** **prog** { **load** | **loadall** } *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**]
> +| **bpftool** **prog** { **load** | **loadall** } *OBJ* *PATH* [**type** *TYPE*] [**map** { **idx** *IDX* | **name** *NAME* } *MAP*] [{ **offload_dev** | **xdpmeta_dev** } *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**]
> | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> | **bpftool** **prog tracelog**
> @@ -129,7 +129,7 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**]
> + **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** { **idx** *IDX* | **name** *NAME* } *MAP*] [{ **offload_dev** | **xdpmeta_dev** } *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**]
> Load bpf program(s) from binary *OBJ* and pin as *PATH*.
> **bpftool prog load** pins only the first program from the
> *OBJ* as *PATH*. **bpftool prog loadall** pins all programs
> @@ -143,8 +143,11 @@ DESCRIPTION
> to be replaced in the ELF file counting from 0, while *NAME*
> allows to replace a map by name. *MAP* specifies the map to
> use, referring to it by **id** or through a **pinned** file.
> - If **dev** *NAME* is specified program will be loaded onto
> - given networking device (offload).
> + If **offload_dev** *NAME* is specified program will be loaded
> + onto given networking device (offload).
> + If **xdpmeta_dev** *NAME* is specified program will become
> + device-bound without offloading, this facilitates access
> + to XDP metadata.
> Optional **pinmaps** argument can be provided to pin all
> maps under *MAP_DIR* directory.
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index e7234d1a5306..085bf18f3659 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -278,7 +278,7 @@ _bpftool()
> _bpftool_get_prog_tags
> return 0
> ;;
> - dev)
> + dev|offload_dev|xdpmeta_dev)
> _sysfs_get_netdevs
> return 0
> ;;
> @@ -508,7 +508,8 @@ _bpftool()
> ;;
> *)
> COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
> - _bpftool_once_attr 'type dev pinmaps autoattach'
> + _bpftool_once_attr 'type pinmaps autoattach'
> + _bpftool_one_of_list 'offload_dev xdpmeta_dev'
> return 0
> ;;
> esac
> @@ -733,7 +734,7 @@ _bpftool()
> esac
> ;;
> *)
> - _bpftool_once_attr 'type key value entries name flags dev'
> + _bpftool_once_attr 'type key value entries name flags offload_dev'
> if _bpftool_search_list 'array_of_maps' 'hash_of_maps'; then
> _bpftool_once_attr 'inner_map'
> fi
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index ae9e822aa3fe..f98f7bbea2b1 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -1287,6 +1287,11 @@ static int do_create(int argc, char **argv)
> "flags"))
> goto exit;
> } else if (is_prefix(*argv, "dev")) {
> + p_info("Warning: 'bpftool map create [...] dev <ifname>' syntax is deprecated.\n"
> + "Going further, please use 'offload_dev <ifname>' to request hardware offload for the map.");
> + goto offload_dev;
> + } else if (is_prefix(*argv, "offload_dev")) {
> +offload_dev:
> NEXT_ARG();
>
> if (attr.map_ifindex) {
> @@ -1431,7 +1436,7 @@ static int do_help(int argc, char **argv)
> "Usage: %1$s %2$s { show | list } [MAP]\n"
> " %1$s %2$s create FILE type TYPE key KEY_SIZE value VALUE_SIZE \\\n"
> " entries MAX_ENTRIES name NAME [flags FLAGS] \\\n"
> - " [inner_map MAP] [dev NAME]\n"
> + " [inner_map MAP] [offload_dev NAME]\n"
> " %1$s %2$s dump MAP\n"
> " %1$s %2$s update MAP [key DATA] [value VALUE] [UPDATE_FLAGS]\n"
> " %1$s %2$s lookup MAP [key DATA]\n"
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 91b6075b2db3..3b77986f0fb7 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1517,12 +1517,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> struct bpf_program *prog = NULL, *pos;
> unsigned int old_map_fds = 0;
> const char *pinmaps = NULL;
> + __u32 xdpmeta_ifindex = 0;
> + __u32 offload_ifindex = 0;
> bool auto_attach = false;
> struct bpf_object *obj;
> struct bpf_map *map;
> const char *pinfile;
> unsigned int i, j;
> - __u32 ifindex = 0;
> const char *file;
> int idx, err;
>
> @@ -1614,17 +1615,46 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> map_replace[old_map_fds].fd = fd;
> old_map_fds++;
> } else if (is_prefix(*argv, "dev")) {
> + p_info("Warning: 'bpftool prog load [...] dev <ifname>' syntax is deprecated.\n"
> + "Going further, please use 'offload_dev <ifname>' to offload program to device.\n"
> + "For applications using XDP hints only, use 'xdpmeta_dev <ifname>'.");
> + goto offload_dev;
> + } else if (is_prefix(*argv, "offload_dev")) {
> +offload_dev:
> NEXT_ARG();
>
> - if (ifindex) {
> - p_err("offload device already specified");
> + if (offload_ifindex) {
> + p_err("offload_dev already specified");
> + goto err_free_reuse_maps;
> + } else if (xdpmeta_ifindex) {
> + p_err("xdpmeta_dev and offload_dev are mutually exclusive");
> + goto err_free_reuse_maps;
> + }
> + if (!REQ_ARGS(1))
> + goto err_free_reuse_maps;
> +
> + offload_ifindex = if_nametoindex(*argv);
> + if (!offload_ifindex) {
> + p_err("unrecognized netdevice '%s': %s",
> + *argv, strerror(errno));
> + goto err_free_reuse_maps;
> + }
> + NEXT_ARG();
> + } else if (is_prefix(*argv, "xdpmeta_dev")) {
> + NEXT_ARG();
> +
> + if (xdpmeta_ifindex) {
> + p_err("xdpmeta_dev already specified");
> + goto err_free_reuse_maps;
> + } else if (offload_ifindex) {
> + p_err("xdpmeta_dev and offload_dev are mutually exclusive");
> goto err_free_reuse_maps;
> }
> if (!REQ_ARGS(1))
> goto err_free_reuse_maps;
>
> - ifindex = if_nametoindex(*argv);
> - if (!ifindex) {
> + xdpmeta_ifindex = if_nametoindex(*argv);
> + if (!xdpmeta_ifindex) {
> p_err("unrecognized netdevice '%s': %s",
> *argv, strerror(errno));
> goto err_free_reuse_maps;
> @@ -1671,7 +1701,12 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> goto err_close_obj;
> }
>
> - bpf_program__set_ifindex(pos, ifindex);
> + if (prog_type == BPF_PROG_TYPE_XDP && xdpmeta_ifindex) {
> + bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY);
> + bpf_program__set_ifindex(pos, xdpmeta_ifindex);
> + } else {
> + bpf_program__set_ifindex(pos, offload_ifindex);
> + }
> if (bpf_program__type(pos) != prog_type)
> bpf_program__set_type(pos, prog_type);
> bpf_program__set_expected_attach_type(pos, expected_attach_type);
> @@ -1709,7 +1744,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> idx = 0;
> bpf_object__for_each_map(map, obj) {
> if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
> - bpf_map__set_ifindex(map, ifindex);
> + bpf_map__set_ifindex(map, offload_ifindex);
>
> if (j < old_map_fds && idx == map_replace[j].idx) {
> err = bpf_map__reuse_fd(map, map_replace[j++].fd);
> @@ -2416,7 +2451,7 @@ static int do_help(int argc, char **argv)
> " %1$s %2$s dump jited PROG [{ file FILE | [opcodes] [linum] }]\n"
> " %1$s %2$s pin PROG FILE\n"
> " %1$s %2$s { load | loadall } OBJ PATH \\\n"
> - " [type TYPE] [dev NAME] \\\n"
> + " [type TYPE] [{ offload_dev | xdpmeta_dev } NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\\\n"
> " [pinmaps MAP_DIR]\n"
> " [autoattach]\n"
> --
> 2.35.3
>
Powered by blists - more mailing lists