BPF List
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: Quentin Monnet <quentin@isovalent.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Jesper Dangaard Brouer" <jbrouer@redhat.com>,
	"Yauheni Kaliuta" <ykaliuta@redhat.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>
Subject: Re: How about adding a name for bpftool self created maps?
Date: Fri, 5 Aug 2022 17:10:04 +0800	[thread overview]
Message-ID: <YuzebJdw6GJ9CVLz@Laptop-X1> (raw)
In-Reply-To: <91477d69-5fe6-5815-3bba-f63764bd61e3@isovalent.com>

On Thu, Aug 04, 2022 at 11:01:48AM +0100, Quentin Monnet wrote:
> Hi! It would look much cleaner to have something specific to map names.
> It does not have to be a dedicated probe in my opinion, maybe we can
> just try loading with a name and retry if this fails with -EINVAL (a bit
> like we retry with another prog type in bpf_object__probe_loading(), if
> the first one fails). Something like this (not tested):
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 50d41815f431..abcafdf8ae7e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4430,7 +4430,10 @@ static int probe_kern_global_data(void)
>  	};
>  	int ret, map, insn_cnt = ARRAY_SIZE(insns);
>  
> -	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32,  1, NULL);
> +	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, "global_data", sizeof(int), 32, 1, NULL);
> +	if (map < 0 && errno == EINVAL)
> +		/* Retry without name */
> +		map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32, 1, NULL);
>  	if (map < 0) {
>  		ret = -errno;
>  		cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
> 
> (Maybe with a small wrapper, given that we'd also need this in
> probe_prog_bind_map() and probe_kern_array_mmap() as well.)

Ah, this looks more clean and easier.

> 
> > And I also have a question about function probe_kern_prog_name(). I only
> > saw it created a prog with name "test". But I didn't find the function check
> > if the prog are really has name "test". If a old kernel doesn't support prog
> > name, I think it will just ignore the name field. No?
> 
> No, "if (CHECK_ATTR(BPF_PROG_LOAD))" should fail in bpf_prog_load() in
> kernel/bpf/syscall.c, and the syscall should fail with -EINVAL.
> 
> If older kernels simply ignored the "name" field for programs and maps,
> we wouldn't have to probe or retry for the current case in the first
> place :).

Thanks for the explanation. I will try add a wrapper first.

Regards
Hangbin

      reply	other threads:[~2022-08-05  9:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-21  2:09 How about adding a name for bpftool self created maps? Hangbin Liu
2022-06-21 21:28 ` Quentin Monnet
2022-06-22 10:15   ` Hangbin Liu
2022-08-04  6:30   ` Hangbin Liu
2022-08-04 10:01     ` Quentin Monnet
2022-08-05  9:10       ` Hangbin Liu [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=YuzebJdw6GJ9CVLz@Laptop-X1 \
    --to=liuhangbin@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jbrouer@redhat.com \
    --cc=quentin@isovalent.com \
    --cc=toke@redhat.com \
    --cc=ykaliuta@redhat.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