netdev - Re: [PATCH v3 bpf-next 2/4] libbpf: add bpf_object__open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191006012416.5lq4xhhmdtgcoemc@ast-mbp>
Date:   Sat, 5 Oct 2019 18:24:18 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
        daniel@...earbox.net, andrii.nakryiko@...il.com, kernel-team@...com
Subject: Re: [PATCH v3 bpf-next 2/4] libbpf: add bpf_object__open_{file,mem}
 w/ extensible opts

On Fri, Oct 04, 2019 at 03:40:35PM -0700, Andrii Nakryiko wrote:
> Add new set of bpf_object__open APIs using new approach to optional
> parameters extensibility allowing simpler ABI compatibility approach.
> 
> This patch demonstrates an approach to implementing libbpf APIs that
> makes it easy to extend existing APIs with extra optional parameters in
> such a way, that ABI compatibility is preserved without having to do
> symbol versioning and generating lots of boilerplate code to handle it.
> To facilitate succinct code for working with options, add OPTS_VALID,
> OPTS_HAS, and OPTS_GET macros that hide all the NULL, size, and zero
> checks.
> 
> Additionally, newly added libbpf APIs are encouraged to follow similar
> pattern of having all mandatory parameters as formal function parameters
> and always have optional (NULL-able) xxx_opts struct, which should
> always have real struct size as a first field and the rest would be
> optional parameters added over time, which tune the behavior of existing
> API, if specified by user.
> 
> Signed-off-by: Andrii Nakryiko <andriin@...com>
...
> +/* Helper macro to declare and initialize libbpf options struct
> + *
> + * This dance with uninitialized declaration, followed by memset to zero,
> + * followed by assignment using compound literal syntax is done to preserve
> + * ability to use a nice struct field initialization syntax and **hopefully**
> + * have all the padding bytes initialized to zero. It's not guaranteed though,
> + * when copying literal, that compiler won't copy garbage in literal's padding
> + * bytes, but that's the best way I've found and it seems to work in practice.
> + */
> +#define LIBBPF_OPTS(TYPE, NAME, ...)					    \
> +	struct TYPE NAME;						    \
> +	memset(&NAME, 0, sizeof(struct TYPE));				    \
> +	NAME = (struct TYPE) {						    \
> +		.sz = sizeof(struct TYPE),				    \
> +		__VA_ARGS__						    \
> +	}
> +
> +struct bpf_object_open_opts {
> +	/* size of this struct, for forward/backward compatiblity */
> +	size_t sz;
> +	/* object name override, if provided:
> +	 * - for object open from file, this will override setting object
> +	 *   name from file path's base name;
> +	 * - for object open from memory buffer, this will specify an object
> +	 *   name and will override default "<addr>-<buf-size>" name;
> +	 */
> +	const char *object_name;
> +	/* parse map definitions non-strictly, allowing extra attributes/data */
> +	bool relaxed_maps;
> +};
> +#define bpf_object_open_opts__last_field relaxed_maps

LIBBPF_OPTS macro doesn't inspire confidence, but despite the ugliness
it is strictly better than what libbpf is using internally to interface
into kernel via similar bpf_attr api.
So I think it's an improvement.
Should this macro be used inside libbpf as well?
May be rename it too to show that it's not libbpf specific?

Anyhow all that can be done in follow up.

Applied. Thanks