diff mbox series

[v3,bpf-next,3/4] btf: expose API to work with raw btf_ext data

Message ID 20190208025555.4027769-4-andriin@fb.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series tools/btf: extend libbpf APIs to work with btf w/o kernel | expand

Commit Message

Andrii Nakryiko Feb. 8, 2019, 2:55 a.m. UTC 
This patch changes struct btf_ext to retain original data in sequential
block of memory, which makes it possible to expose
btf_ext__get_raw_data() interface, that's similar to
btf__get_raw_data(), allowing users of libbpf to get access to raw
representation of .BTF.ext section.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.c      | 85 +++++++++++++++++++++-------------------
 tools/lib/bpf/btf.h      |  2 +
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 47 insertions(+), 41 deletions(-)

Comments

Yonghong Song Feb. 8, 2019, 5:12 p.m. UTC | #1 
On 2/7/19 6:55 PM, Andrii Nakryiko wrote:
> This patch changes struct btf_ext to retain original data in sequential
> block of memory, which makes it possible to expose
> btf_ext__get_raw_data() interface, that's similar to
> btf__get_raw_data(), allowing users of libbpf to get access to raw
> representation of .BTF.ext section.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   tools/lib/bpf/btf.c      | 85 +++++++++++++++++++++-------------------
>   tools/lib/bpf/btf.h      |  2 +
>   tools/lib/bpf/libbpf.map |  1 +
>   3 files changed, 47 insertions(+), 41 deletions(-)
> 
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 8730f6c3be9e..c87cc3d71b9f 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -41,9 +41,8 @@ struct btf {
>   
>   struct btf_ext_info {
>   	/*
> -	 * info points to a deep copy of the individual info section
> -	 * (e.g. func_info and line_info) from the .BTF.ext.
> -	 * It does not include the __u32 rec_size.
> +	 * info points to the individual info section (e.g. func_info and
> +	 * line_info) from the .BTF.ext. It does not include the __u32 rec_size.
>   	 */
>   	void *info;
>   	__u32 rec_size;
> @@ -51,8 +50,13 @@ struct btf_ext_info {
>   };
>   
>   struct btf_ext {
> +	union {
> +		struct btf_ext_header *hdr;
> +		void *data;
> +	};
>   	struct btf_ext_info func_info;
>   	struct btf_ext_info line_info;
> +	__u32 data_size;
>   };
>   
>   struct btf_ext_info_sec {
> @@ -603,19 +607,13 @@ struct btf_ext_sec_copy_param {
>   };
>   
>   static int btf_ext_copy_info(struct btf_ext *btf_ext,
> -			     __u8 *data, __u32 data_size,
>   			     struct btf_ext_sec_copy_param *ext_sec)

Overall looks good. Since we do not really "copy" info any more,
rather we try to "setup" info based on btf_ext. Maybe changing
all function and structure names with "_copy_" to "_setup_"?

>   {
> -	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
>   	const struct btf_ext_info_sec *sinfo;
>   	struct btf_ext_info *ext_info;
>   	__u32 info_left, record_size;
>   	/* The start of the info sec (including the __u32 record_size). */
> -	const void *info;
> -
> -	/* data and data_size do not include btf_ext_header from now on */
> -	data = data + hdr->hdr_len;
> -	data_size -= hdr->hdr_len;
> +	void *info;
[...]
Andrii Nakryiko Feb. 8, 2019, 5:15 p.m. UTC | #2 
On Fri, Feb 8, 2019 at 9:13 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 2/7/19 6:55 PM, Andrii Nakryiko wrote:
> > This patch changes struct btf_ext to retain original data in sequential
> > block of memory, which makes it possible to expose
> > btf_ext__get_raw_data() interface, that's similar to
> > btf__get_raw_data(), allowing users of libbpf to get access to raw
> > representation of .BTF.ext section.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >   tools/lib/bpf/btf.c      | 85 +++++++++++++++++++++-------------------
> >   tools/lib/bpf/btf.h      |  2 +
> >   tools/lib/bpf/libbpf.map |  1 +
> >   3 files changed, 47 insertions(+), 41 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 8730f6c3be9e..c87cc3d71b9f 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -41,9 +41,8 @@ struct btf {
> >
> >   struct btf_ext_info {
> >       /*
> > -      * info points to a deep copy of the individual info section
> > -      * (e.g. func_info and line_info) from the .BTF.ext.
> > -      * It does not include the __u32 rec_size.
> > +      * info points to the individual info section (e.g. func_info and
> > +      * line_info) from the .BTF.ext. It does not include the __u32 rec_size.
> >        */
> >       void *info;
> >       __u32 rec_size;
> > @@ -51,8 +50,13 @@ struct btf_ext_info {
> >   };
> >
> >   struct btf_ext {
> > +     union {
> > +             struct btf_ext_header *hdr;
> > +             void *data;
> > +     };
> >       struct btf_ext_info func_info;
> >       struct btf_ext_info line_info;
> > +     __u32 data_size;
> >   };
> >
> >   struct btf_ext_info_sec {
> > @@ -603,19 +607,13 @@ struct btf_ext_sec_copy_param {
> >   };
> >
> >   static int btf_ext_copy_info(struct btf_ext *btf_ext,
> > -                          __u8 *data, __u32 data_size,
> >                            struct btf_ext_sec_copy_param *ext_sec)
>
> Overall looks good. Since we do not really "copy" info any more,
> rather we try to "setup" info based on btf_ext. Maybe changing
> all function and structure names with "_copy_" to "_setup_"?

Makes sense, will update.

>
> >   {
> > -     const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
> >       const struct btf_ext_info_sec *sinfo;
> >       struct btf_ext_info *ext_info;
> >       __u32 info_left, record_size;
> >       /* The start of the info sec (including the __u32 record_size). */
> > -     const void *info;
> > -
> > -     /* data and data_size do not include btf_ext_header from now on */
> > -     data = data + hdr->hdr_len;
> > -     data_size -= hdr->hdr_len;
> > +     void *info;
> [...]
diff mbox series

Patch

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 8730f6c3be9e..c87cc3d71b9f 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -41,9 +41,8 @@  struct btf {
 
 struct btf_ext_info {
 	/*
-	 * info points to a deep copy of the individual info section
-	 * (e.g. func_info and line_info) from the .BTF.ext.
-	 * It does not include the __u32 rec_size.
+	 * info points to the individual info section (e.g. func_info and
+	 * line_info) from the .BTF.ext. It does not include the __u32 rec_size.
 	 */
 	void *info;
 	__u32 rec_size;
@@ -51,8 +50,13 @@  struct btf_ext_info {
 };
 
 struct btf_ext {
+	union {
+		struct btf_ext_header *hdr;
+		void *data;
+	};
 	struct btf_ext_info func_info;
 	struct btf_ext_info line_info;
+	__u32 data_size;
 };
 
 struct btf_ext_info_sec {
@@ -603,19 +607,13 @@  struct btf_ext_sec_copy_param {
 };
 
 static int btf_ext_copy_info(struct btf_ext *btf_ext,
-			     __u8 *data, __u32 data_size,
 			     struct btf_ext_sec_copy_param *ext_sec)
 {
-	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
 	const struct btf_ext_info_sec *sinfo;
 	struct btf_ext_info *ext_info;
 	__u32 info_left, record_size;
 	/* The start of the info sec (including the __u32 record_size). */
-	const void *info;
-
-	/* data and data_size do not include btf_ext_header from now on */
-	data = data + hdr->hdr_len;
-	data_size -= hdr->hdr_len;
+	void *info;
 
 	if (ext_sec->off & 0x03) {
 		pr_debug(".BTF.ext %s section is not aligned to 4 bytes\n",
@@ -623,16 +621,15 @@  static int btf_ext_copy_info(struct btf_ext *btf_ext,
 		return -EINVAL;
 	}
 
-	if (data_size < ext_sec->off ||
-	    ext_sec->len > data_size - ext_sec->off) {
+	info = btf_ext->data + btf_ext->hdr->hdr_len + ext_sec->off;
+	info_left = ext_sec->len;
+
+	if (btf_ext->data + btf_ext->data_size < info + ext_sec->len) {
 		pr_debug("%s section (off:%u len:%u) is beyond the end of the ELF section .BTF.ext\n",
-		     ext_sec->desc, ext_sec->off, ext_sec->len);
+			 ext_sec->desc, ext_sec->off, ext_sec->len);
 		return -EINVAL;
 	}
 
-	info = data + ext_sec->off;
-	info_left = ext_sec->len;
-
 	/* At least a record size */
 	if (info_left < sizeof(__u32)) {
 		pr_debug(".BTF.ext %s record size not found\n", ext_sec->desc);
@@ -644,7 +641,7 @@  static int btf_ext_copy_info(struct btf_ext *btf_ext,
 	if (record_size < ext_sec->min_rec_size ||
 	    record_size & 0x03) {
 		pr_debug("%s section in .BTF.ext has invalid record size %u\n",
-		     ext_sec->desc, record_size);
+			 ext_sec->desc, record_size);
 		return -EINVAL;
 	}
 
@@ -690,42 +687,35 @@  static int btf_ext_copy_info(struct btf_ext *btf_ext,
 	ext_info = ext_sec->ext_info;
 	ext_info->len = ext_sec->len - sizeof(__u32);
 	ext_info->rec_size = record_size;
-	ext_info->info = malloc(ext_info->len);
-	if (!ext_info->info)
-		return -ENOMEM;
-	memcpy(ext_info->info, info + sizeof(__u32), ext_info->len);
+	ext_info->info = info + sizeof(__u32);
 
 	return 0;
 }
 
-static int btf_ext_copy_func_info(struct btf_ext *btf_ext,
-				  __u8 *data, __u32 data_size)
+static int btf_ext_copy_func_info(struct btf_ext *btf_ext)
 {
-	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
 	struct btf_ext_sec_copy_param param = {
-		.off = hdr->func_info_off,
-		.len = hdr->func_info_len,
+		.off = btf_ext->hdr->func_info_off,
+		.len = btf_ext->hdr->func_info_len,
 		.min_rec_size = sizeof(struct bpf_func_info_min),
 		.ext_info = &btf_ext->func_info,
 		.desc = "func_info"
 	};
 
-	return btf_ext_copy_info(btf_ext, data, data_size, &param);
+	return btf_ext_copy_info(btf_ext, &param);
 }
 
-static int btf_ext_copy_line_info(struct btf_ext *btf_ext,
-				  __u8 *data, __u32 data_size)
+static int btf_ext_copy_line_info(struct btf_ext *btf_ext)
 {
-	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
 	struct btf_ext_sec_copy_param param = {
-		.off = hdr->line_info_off,
-		.len = hdr->line_info_len,
+		.off = btf_ext->hdr->line_info_off,
+		.len = btf_ext->hdr->line_info_len,
 		.min_rec_size = sizeof(struct bpf_line_info_min),
 		.ext_info = &btf_ext->line_info,
 		.desc = "line_info",
 	};
 
-	return btf_ext_copy_info(btf_ext, data, data_size, &param);
+	return btf_ext_copy_info(btf_ext, &param);
 }
 
 static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
@@ -765,9 +755,7 @@  void btf_ext__free(struct btf_ext *btf_ext)
 {
 	if (!btf_ext)
 		return;
-
-	free(btf_ext->func_info.info);
-	free(btf_ext->line_info.info);
+	free(btf_ext->data);
 	free(btf_ext);
 }
 
@@ -784,13 +772,23 @@  struct btf_ext *btf_ext__new(__u8 *data, __u32 size)
 	if (!btf_ext)
 		return ERR_PTR(-ENOMEM);
 
-	err = btf_ext_copy_func_info(btf_ext, data, size);
-	if (err) {
-		btf_ext__free(btf_ext);
-		return ERR_PTR(err);
+	btf_ext->data_size = size;
+	btf_ext->data = malloc(size);
+	if (!btf_ext->data) {
+		err = -ENOMEM;
+		goto done;
 	}
+	memcpy(btf_ext->data, data, size);
+
+	err = btf_ext_copy_func_info(btf_ext);
+	if (err)
+		goto done;
+
+	err = btf_ext_copy_line_info(btf_ext);
+	if (err)
+		goto done;
 
-	err = btf_ext_copy_line_info(btf_ext, data, size);
+done:
 	if (err) {
 		btf_ext__free(btf_ext);
 		return ERR_PTR(err);
@@ -799,6 +797,11 @@  struct btf_ext *btf_ext__new(__u8 *data, __u32 size)
 	return btf_ext;
 }
 
+const void *btf_ext__get_raw_data(const struct btf_ext* btf_ext, __u32 *size) {
+	*size = btf_ext->data_size;
+	return btf_ext->data;
+}
+
 static int btf_ext_reloc_info(const struct btf *btf,
 			      const struct btf_ext_info *ext_info,
 			      const char *sec_name, __u32 insns_cnt,
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index d6c0a5576fdc..ad9f648260c2 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -78,6 +78,8 @@  LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name,
 
 LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size);
 LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext);
+LIBBPF_API const void *btf_ext__get_raw_data(const struct btf_ext* btf_ext,
+					     __u32 *size);
 LIBBPF_API int btf_ext__reloc_func_info(const struct btf *btf,
 					const struct btf_ext *btf_ext,
 					const char *sec_name, __u32 insns_cnt,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 9e10467f8cbb..eb78c7c261d9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -141,6 +141,7 @@  LIBBPF_0.0.2 {
 		btf__load;
 		btf_ext__free;
 		btf_ext__func_info_rec_size;
+		btf_ext__get_raw_data;
 		btf_ext__line_info_rec_size;
 		btf_ext__new;
 		btf_ext__reloc_func_info;