LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Song Liu <liu.song.a23@gmail.com>
To: Michal Rostecki <mrostecki@opensuse.org>
Cc: Andrii Nakryiko <andriin@fb.com>,
	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 v4] libbpf: Return btf_fd for load_sk_storage_btf
Date: Thu, 30 May 2019 14:33:56 -0700	[thread overview]
Message-ID: <CAPhsuW7KhR1XXDb6Sv54xb1OiLQUC7NH4+uf8_b3tRje7O-YUQ@mail.gmail.com> (raw)
In-Reply-To: <20190529183109.17317-1-mrostecki@opensuse.org>

On Wed, May 29, 2019 at 11:30 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 renames the libbpf__probe_raw_btf function to
> 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.
>
> v4:
> - Fix typo in the commit message.
>
> Fixes: d7c4b3980c18 ("libbpf: detect supported kernel BTF features and sanitize BTF")
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
> Acked-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

Thanks for the fix!

> ---
>  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
>

  reply	other threads:[~2019-05-30 22:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-29 18:31 Michal Rostecki
2019-05-30 21:33 ` Song Liu [this message]
2019-06-01  0:05   ` Alexei Starovoitov

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=CAPhsuW7KhR1XXDb6Sv54xb1OiLQUC7NH4+uf8_b3tRje7O-YUQ@mail.gmail.com \
    --to=liu.song.a23@gmail.com \
    --cc=andriin@fb.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 \
    --subject='Re: [PATCH bpf v4] libbpf: Return btf_fd for load_sk_storage_btf' \
    /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

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).