From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Michal Rostecki <mrostecki@opensuse.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
"open list:BPF (Safe dynamic programs and tools)"
<netdev@vger.kernel.org>,
"open list:BPF (Safe dynamic programs and tools)"
<bpf@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf v3] libbpf: Return btf_fd for load_sk_storage_btf
Date: Wed, 29 May 2019 10:18:53 -0700 [thread overview]
Message-ID: <CAEf4BzbajeCeWbS8=Y+E-TxY5m5DnHkco7UriU96mNn-knqc4Q@mail.gmail.com> (raw)
In-Reply-To: <20190529163616.8418-1-mrostecki@opensuse.org>
On Wed, May 29, 2019 at 9:35 AM Michal Rostecki <mrostecki@opensuse.org> wrote:
>
> Before this change, function load_sk_storage_btf expected that
> libbpf__probe_raw_btf was returning a BTF descriptor, but in fact it was
> returning an information about whether the probe was successful (0 or
> 1). load_sk_storage_btf was using that value as an argument of the close
> function, which was resulting in closing stdout and thus terminating the
> process which called that function.
>
> That bug was visible in bpftool. `bpftool feature` subcommand was always
> exiting too early (because of closed stdout) and it didn't display all
> requested probes. `bpftool -j feature` or `bpftool -p feature` were not
> returning a valid json object.
>
> This change remnames the libbpf__probe_raw_btf function to
typo: remnames -> renames
> libbpf__load_raw_btf, which now returns a BTF descriptor, as expected in
> load_sk_storage_btf.
>
> v2:
> - Fix typo in the commit message.
>
> v3:
> - Simplify BTF descriptor handling in bpf_object__probe_btf_* functions.
> - Rename libbpf__probe_raw_btf function to libbpf__load_raw_btf and
> return a BTF descriptor.
>
> Fixes: d7c4b3980c18 ("libbpf: detect supported kernel BTF features and sanitize BTF")
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
> ---
Thanks!
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/lib/bpf/libbpf.c | 28 ++++++++++++++++------------
> tools/lib/bpf/libbpf_internal.h | 4 ++--
> tools/lib/bpf/libbpf_probes.c | 13 ++++---------
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 197b574406b3..5d046cc7b207 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1645,14 +1645,16 @@ static int bpf_object__probe_btf_func(struct bpf_object *obj)
> /* FUNC x */ /* [3] */
> BTF_TYPE_ENC(5, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0), 2),
> };
> - int res;
> + int btf_fd;
>
> - res = libbpf__probe_raw_btf((char *)types, sizeof(types),
> - strs, sizeof(strs));
> - if (res < 0)
> - return res;
> - if (res > 0)
> + btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs));
> + if (btf_fd >= 0) {
> obj->caps.btf_func = 1;
> + close(btf_fd);
> + return 1;
> + }
> +
> return 0;
> }
>
> @@ -1670,14 +1672,16 @@ static int bpf_object__probe_btf_datasec(struct bpf_object *obj)
> BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
> BTF_VAR_SECINFO_ENC(2, 0, 4),
> };
> - int res;
> + int btf_fd;
>
> - res = libbpf__probe_raw_btf((char *)types, sizeof(types),
> - strs, sizeof(strs));
> - if (res < 0)
> - return res;
> - if (res > 0)
> + btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs));
> + if (btf_fd >= 0) {
> obj->caps.btf_datasec = 1;
> + close(btf_fd);
> + return 1;
> + }
> +
> return 0;
> }
>
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index f3025b4d90e1..dfab8012185c 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -34,7 +34,7 @@ do { \
> #define pr_info(fmt, ...) __pr(LIBBPF_INFO, fmt, ##__VA_ARGS__)
> #define pr_debug(fmt, ...) __pr(LIBBPF_DEBUG, fmt, ##__VA_ARGS__)
>
> -int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> - const char *str_sec, size_t str_len);
> +int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
> + const char *str_sec, size_t str_len);
>
> #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 5e2aa83f637a..6635a31a7a16 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -133,8 +133,8 @@ bool bpf_probe_prog_type(enum bpf_prog_type prog_type, __u32 ifindex)
> return errno != EINVAL && errno != EOPNOTSUPP;
> }
>
> -int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> - const char *str_sec, size_t str_len)
> +int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
> + const char *str_sec, size_t str_len)
> {
> struct btf_header hdr = {
> .magic = BTF_MAGIC,
> @@ -157,14 +157,9 @@ int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> memcpy(raw_btf + hdr.hdr_len + hdr.type_len, str_sec, hdr.str_len);
>
> btf_fd = bpf_load_btf(raw_btf, btf_len, NULL, 0, false);
> - if (btf_fd < 0) {
> - free(raw_btf);
> - return 0;
> - }
>
> - close(btf_fd);
> free(raw_btf);
> - return 1;
> + return btf_fd;
> }
>
> static int load_sk_storage_btf(void)
> @@ -190,7 +185,7 @@ static int load_sk_storage_btf(void)
> BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
> };
>
> - return libbpf__probe_raw_btf((char *)types, sizeof(types),
> + return libbpf__load_raw_btf((char *)types, sizeof(types),
> strs, sizeof(strs));
> }
>
> --
> 2.21.0
>
prev parent reply other threads:[~2019-05-29 17:19 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-29 16:36 [PATCH bpf v3] libbpf: Return btf_fd for load_sk_storage_btf Michal Rostecki
2019-05-29 17:18 ` Andrii Nakryiko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4BzbajeCeWbS8=Y+E-TxY5m5DnHkco7UriU96mNn-knqc4Q@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mrostecki@opensuse.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).