All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <ast@fb.com>
To: Andrii Nakryiko <andrii@kernel.org>, <bpf@vger.kernel.org>,
	<netdev@vger.kernel.org>, <daniel@iogearbox.net>
Cc: <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 12/17] libbpf: support extern resolution for BTF-defined maps in .maps section
Date: Wed, 14 Apr 2021 15:00:27 -0700	[thread overview]
Message-ID: <f3f3bcc5-be1a-6d11-0c6e-081fc30367c4@fb.com> (raw)
In-Reply-To: <20210414200146.2663044-13-andrii@kernel.org>

On 4/14/21 1:01 PM, Andrii Nakryiko wrote:
> Add extra logic to handle map externs (only BTF-defined maps are supported for
> linking). Re-use the map parsing logic used during bpf_object__open(). Map
> externs are currently restricted to always and only specify map type, key
> type and/or size, and value type and/or size. Nothing extra is allowed. If any
> of those attributes are mismatched between extern and actual map definition,
> linker will report an error.

I don't get the motivation for this.
It seems cumbersome to force users to do:
+extern struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__type(key, key_type);
+	__type(value, value_type);
+	/* no max_entries on extern map definitions */
+} map1 SEC(".maps");

> The original intent was to allow for extern to specify attributes that matters
> (to user) to enforce. E.g., if you specify just key information and omit
> value, then any value fits. Similarly, it should have been possible to enforce
> map_flags, pinning, and any other possible map attribute. Unfortunately, that
> means that multiple externs can be only partially overlapping with each other,
> which means linker would need to combine their type definitions to end up with
> the most restrictive and fullest map definition.

but there is only one such full map definition.
Can all externs to be:
extern struct {} map1 SEC(".maps");

They can be in multiple .o files, but one true global map def
should have all the fields and will take the precedence during
the linking.

The map type, key size, value size, max entries are all irrelevant
during compilation. They're relevant during loading, but libbpf is
not going to load every .o individually. So "extern map" can
have any fields it wouldn't change the end result after linking.
May be enforce that 'extern struct {} map' doesn't have
any fields defined instead?
It seems asking users to copy-paste map defs in one file and in all
of extern is just extra hassle.
The users wouldn't want to copy-paste them for production code,
but will put map def into .h and include in multiple .c,
but adding "extern " in many .c-s and not
adding that "extern " is the main .c is another macro hassle.
Actually forcing "no max_entries in extern" is killing this idea.
So it's mandatory copy-paste or even more macro magic with partial
defs of maps?
What am I missing?

  reply	other threads:[~2021-04-14 22:00 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14 20:01 [PATCH bpf-next 00/17] BPF static linker: support externs Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 01/17] bpftool: support dumping BTF VAR's "extern" linkage Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 02/17] bpftool: dump more info about DATASEC members Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 03/17] libbpf: suppress compiler warning when using SEC() macro with externs Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 04/17] libbpf: mark BPF subprogs with hidden visibility as static for BPF verifier Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 05/17] libbpf: allow gaps in BPF program sections to support overriden weak functions Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 06/17] libbpf: refactor BTF map definition parsing Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 07/17] libbpf: factor out symtab and relos sanity checks Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 08/17] libbpf: make few internal helpers available outside of libbpf.c Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 09/17] libbpf: extend sanity checking ELF symbols with externs validation Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 10/17] libbpf: tighten BTF type ID rewriting with error checking Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 11/17] libbpf: add linker extern resolution support for functions and global variables Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 12/17] libbpf: support extern resolution for BTF-defined maps in .maps section Andrii Nakryiko
2021-04-14 22:00   ` Alexei Starovoitov [this message]
2021-04-14 23:48     ` Andrii Nakryiko
2021-04-15  2:01       ` Alexei Starovoitov
2021-04-15 20:35         ` Andrii Nakryiko
2021-04-15 20:57           ` Alexei Starovoitov
2021-04-14 20:01 ` [PATCH bpf-next 13/17] selftests/bpf: use -O0 instead of -Og in selftests builds Andrii Nakryiko
2021-04-14 22:02   ` Alexei Starovoitov
2021-04-14 22:15   ` David Laight
2021-04-15  0:03     ` Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 14/17] selftests/bpf: omit skeleton generation for multi-linked BPF object files Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 15/17] selftests/bpf: add function linking selftest Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 16/17] selftests/bpf: add global variables " Andrii Nakryiko
2021-04-14 20:01 ` [PATCH bpf-next 17/17] sleftests/bpf: add map " Andrii Nakryiko
2021-04-15  0:21 ` [PATCH bpf-next 00/17] BPF static linker: support externs Andrii Nakryiko

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=f3f3bcc5-be1a-6d11-0c6e-081fc30367c4@fb.com \
    --to=ast@fb.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    /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.