[v2,bpf-next,07/13] resolve_btfids: use .BTF.base ELF section as base BTF if -B option is used

Commit Message

Alan Maguire April 24, 2024, 3:48 p.m. UTC

When resolving BTF ids, use the BTF in the module .BTF.base section
when passed the -B option.  Both references to base BTF from split
BTF and BTF ids will be relocated for base vmlinux on module load.

Signed-off-by: Alan Maguire <[e-mail]>
---
 tools/bpf/resolve_btfids/main.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Andrii Nakryiko April 29, 2024, 11:45 p.m. UTC | #1

On Wed, Apr 24, 2024 at 8:49 AM Alan Maguire <[e-mail]> wrote:
>
> When resolving BTF ids, use the BTF in the module .BTF.base section
> when passed the -B option.  Both references to base BTF from split
> BTF and BTF ids will be relocated for base vmlinux on module load.
>
> Signed-off-by: Alan Maguire <[e-mail]>
> ---
>  tools/bpf/resolve_btfids/main.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index d9520cb826b3..c5b622a31f18 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -115,6 +115,7 @@ struct object {
>         const char *path;
>         const char *btf;
>         const char *base_btf_path;
> +       int base;
>
>         struct {
>                 int              fd;
> @@ -532,11 +533,26 @@ static int symbols_resolve(struct object *obj)
>         __u32 nr_types;
>
>         if (obj->base_btf_path) {
> -               base_btf = btf__parse(obj->base_btf_path, NULL);
> +               LIBBPF_OPTS(btf_parse_opts, optp);
> +               const char *path;
> +
> +               if (obj->base) {
> +                       optp.btf_sec = BTF_BASE_ELF_SEC;
> +                       path = obj->path;
> +                       base_btf = btf__parse_opts(path, &optp);
> +                       /* fall back to normal base parsing if no BTF_BASE_ELF_SEC */
> +                       if (libbpf_get_error(base_btf))

don't add new uses of libbpf_get_error(), it will be eventually
removed, as it's now quire error prone. Just check pointer and then
access errno, if necessary

> +                               base_btf = NULL;
> +               }
> +               if (!base_btf) {
> +                       optp.btf_sec = BTF_ELF_SEC;
> +                       path = obj->base_btf_path;
> +                       base_btf = btf__parse_opts(path, &optp);
> +               }
>                 err = libbpf_get_error(base_btf);
>                 if (err) {
>                         pr_err("FAILED: load base BTF from %s: %s\n",
> -                              obj->base_btf_path, strerror(-err));
> +                              path, strerror(-err));
>                         return -1;
>                 }
>         }
> @@ -781,6 +797,8 @@ int main(int argc, const char **argv)
>                            "BTF data"),
>                 OPT_STRING('b', "btf_base", &obj.base_btf_path, "file",
>                            "path of file providing base BTF"),
> +               OPT_INCR('B', "base", &obj.base,
> +                        "use " BTF_BASE_ELF_SEC " ELF section BTF as base"),
>                 OPT_END()
>         };
>         int err = -1;
> --
> 2.31.1
>

Eduard Zingerman May 1, 2024, 8:39 p.m. UTC | #2

On Wed, 2024-04-24 at 16:48 +0100, Alan Maguire wrote:

[...]

> @@ -532,11 +533,26 @@ static int symbols_resolve(struct object *obj)
>  	__u32 nr_types;
>  
>  	if (obj->base_btf_path) {
> -		base_btf = btf__parse(obj->base_btf_path, NULL);
> +		LIBBPF_OPTS(btf_parse_opts, optp);
> +		const char *path;
> +
> +		if (obj->base) {
> +			optp.btf_sec = BTF_BASE_ELF_SEC;
> +			path = obj->path;
> +			base_btf = btf__parse_opts(path, &optp);
> +			/* fall back to normal base parsing if no BTF_BASE_ELF_SEC */
> +			if (libbpf_get_error(base_btf))
> +				base_btf = NULL;

Should this be a fatal error?
Since user requested '-B' explicitly?

> +		}
> +		if (!base_btf) {
> +			optp.btf_sec = BTF_ELF_SEC;
> +			path = obj->base_btf_path;
> +			base_btf = btf__parse_opts(path, &optp);
> +		}
>  		err = libbpf_get_error(base_btf);
>  		if (err) {
>  			pr_err("FAILED: load base BTF from %s: %s\n",
> -			       obj->base_btf_path, strerror(-err));
> +			       path, strerror(-err));
>  			return -1;
>  		}
>  	}

[...]

Alan Maguire May 2, 2024, 2:53 p.m. UTC | #3

On 01/05/2024 21:39, Eduard Zingerman wrote:
> On Wed, 2024-04-24 at 16:48 +0100, Alan Maguire wrote:
> 
> [...]
> 
>> @@ -532,11 +533,26 @@ static int symbols_resolve(struct object *obj)
>>  	__u32 nr_types;
>>  
>>  	if (obj->base_btf_path) {
>> -		base_btf = btf__parse(obj->base_btf_path, NULL);
>> +		LIBBPF_OPTS(btf_parse_opts, optp);
>> +		const char *path;
>> +
>> +		if (obj->base) {
>> +			optp.btf_sec = BTF_BASE_ELF_SEC;
>> +			path = obj->path;
>> +			base_btf = btf__parse_opts(path, &optp);
>> +			/* fall back to normal base parsing if no BTF_BASE_ELF_SEC */
>> +			if (libbpf_get_error(base_btf))
>> +				base_btf = NULL;
> 
> Should this be a fatal error?
> Since user requested '-B' explicitly?
>

No, the fallback behaviour is intended. The reason is this; if the user
is using an older pahole that does not support the generation of
distilled base BTF, there will be no .BTF.base section in modules. We
will however have specified the -B option, so we want to fall back to
normal resolve_btfids behaviour for modules. This avoids the need to
check if the BTF feature really works; if it doesn't we drive on with
default resolve_btfids behaviour for modules. Thanks!

Alan

>> +		}
>> +		if (!base_btf) {
>> +			optp.btf_sec = BTF_ELF_SEC;
>> +			path = obj->base_btf_path;
>> +			base_btf = btf__parse_opts(path, &optp);
>> +		}
>>  		err = libbpf_get_error(base_btf);
>>  		if (err) {
>>  			pr_err("FAILED: load base BTF from %s: %s\n",
>> -			       obj->base_btf_path, strerror(-err));
>> +			       path, strerror(-err));
>>  			return -1;
>>  		}
>>  	}
> 
> [...]

Patch

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index d9520cb826b3..c5b622a31f18 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -115,6 +115,7 @@  struct object {
 	const char *path;
 	const char *btf;
 	const char *base_btf_path;
+	int base;
 
 	struct {
 		int		 fd;
@@ -532,11 +533,26 @@  static int symbols_resolve(struct object *obj)
 	__u32 nr_types;
 
 	if (obj->base_btf_path) {
-		base_btf = btf__parse(obj->base_btf_path, NULL);
+		LIBBPF_OPTS(btf_parse_opts, optp);
+		const char *path;
+
+		if (obj->base) {
+			optp.btf_sec = BTF_BASE_ELF_SEC;
+			path = obj->path;
+			base_btf = btf__parse_opts(path, &optp);
+			/* fall back to normal base parsing if no BTF_BASE_ELF_SEC */
+			if (libbpf_get_error(base_btf))
+				base_btf = NULL;
+		}
+		if (!base_btf) {
+			optp.btf_sec = BTF_ELF_SEC;
+			path = obj->base_btf_path;
+			base_btf = btf__parse_opts(path, &optp);
+		}
 		err = libbpf_get_error(base_btf);
 		if (err) {
 			pr_err("FAILED: load base BTF from %s: %s\n",
-			       obj->base_btf_path, strerror(-err));
+			       path, strerror(-err));
 			return -1;
 		}
 	}
@@ -781,6 +797,8 @@  int main(int argc, const char **argv)
 			   "BTF data"),
 		OPT_STRING('b', "btf_base", &obj.base_btf_path, "file",
 			   "path of file providing base BTF"),
+		OPT_INCR('B', "base", &obj.base,
+			 "use " BTF_BASE_ELF_SEC " ELF section BTF as base"),
 		OPT_END()
 	};
 	int err = -1;