All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.