All of lore.kernel.org
 help / color / mirror / Atom feed
From: Delyan Kratunov <delyank@fb.com>
To: "alexei.starovoitov@gmail.com" <alexei.starovoitov@gmail.com>
Cc: "daniel@iogearbox.net" <daniel@iogearbox.net>,
	"ast@kernel.org" <ast@kernel.org>,
	"andrii@kernel.org" <andrii@kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 4/4] selftests/bpf: test subskeleton functionality
Date: Thu, 3 Mar 2022 00:06:14 +0000	[thread overview]
Message-ID: <decc0e45afff1ed4bdb65e929aab24d7f66ed921.camel@fb.com> (raw)
In-Reply-To: <20220302223008.7deshk5qfw3pytxm@ast-mbp.dhcp.thefacebook.com>

On Wed, 2022-03-02 at 14:30 -0800, Alexei Starovoitov wrote:
> On Wed, Mar 02, 2022 at 02:48:48AM +0000, Delyan Kratunov wrote:
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/subskeleton.c
> > @@ -0,0 +1,38 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2019 Facebook */
> > +
> > +#include <test_progs.h>
> > +#include "test_subskeleton.skel.h"
> > +
> > +extern void subskeleton_lib_setup(struct bpf_object *obj);
> > +extern int subskeleton_lib_subresult(struct bpf_object *obj);
> > +
> > +void test_subskeleton(void)
> > +{
> > +	int duration = 0, err, result;
> > +	struct test_subskeleton *skel;
> > +
> > +	skel = test_subskeleton__open();
> > +	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> > +		return;
> > +
> > +	skel->rodata->rovar1 = 10;
> 
> The rodata vars in subskeleton will need extra '*', right?

Possibly, depending on the exact structure (see the var3 assignments in the
subskel). 
Overall, the naming here is confusing. This is the subskeleton.c test, the 
subskeleton.o final bpf object, but this is the *full* skeleton for that object.

Naming suggestions welcome!

> The above is confusing to read comparing to below:
> 
> > +void subskeleton_lib_setup(struct bpf_object *obj)
> > +{
> > +	struct test_subskeleton_lib *lib = test_subskeleton_lib__open(obj);
> > +
> > +	ASSERT_OK_PTR(lib, "open subskeleton");
> > +
> > +	*lib->data.var1 = 1;
> > +	*lib->bss.var2 = 2;
> > +	lib->bss.var3->var3_1 = 3;
> > +	lib->bss.var3->var3_2 = 4;
> > +}
> 
> Could you add rodata to subskel as well?
> Just to make it obvious that rodata is not special.

Sure, no objections from me.

> An example of generated skel in commit log would be great.

Will add in reroll. This is it in its entirety:

/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */

/* THIS FILE IS AUTOGENERATED! */
#ifndef __TEST_SUBSKELETON_LIB_SKEL_H__
#define __TEST_SUBSKELETON_LIB_SKEL_H__

#include <errno.h>
#include <stdlib.h>
#include <bpf/libbpf.h>

struct test_subskeleton_lib {
	struct bpf_object *obj;
	struct bpf_object_subskeleton *subskel;
	struct test_subskeleton_lib__data {
		int *var1;
	} data;
	struct test_subskeleton_lib__bss {
		int *var2;
		struct {
			int var3_1;
			__s64 var3_2;
		} *var3;
		int *libout1;
	} bss;

#ifdef __cplusplus
	static inline struct test_subskeleton_lib *open(const struct
bpf_object_open_opts *opts = nullptr);
	static inline void test_subskeleton_lib::destroy(struct
test_subskeleton_lib *skel);
#endif /* __cplusplus */
};

static inline void
test_subskeleton_lib__destroy(struct test_subskeleton_lib *skel)
{
	if (!skel)
		return;
	if (skel->subskel)
		bpf_object__destroy_subskeleton(skel->subskel);
	free(skel);
}

static inline struct test_subskeleton_lib *
test_subskeleton_lib__open(const struct bpf_object *src)
{
	struct test_subskeleton_lib *obj;
	struct bpf_object_subskeleton *subskel;
	struct bpf_sym_skeleton *syms;
	int err;

	obj = (struct test_subskeleton_lib *)calloc(1, sizeof(*obj));
	if (!obj) {
		errno = ENOMEM;
		return NULL;
	}
	subskel = (struct bpf_object_subskeleton *)calloc(1, sizeof(*subskel));
	if (!subskel) {
		errno = ENOMEM;
		return NULL;
	}
	subskel->sz = sizeof(*subskel);
	subskel->obj = src;
	subskel->sym_skel_sz = sizeof(struct bpf_sym_skeleton);
	subskel->sym_cnt = 4;
	obj->subskel = subskel;

	syms = (struct bpf_sym_skeleton *)calloc(4, sizeof(*syms));
	if (!syms) {
		free(subskel);
		errno = ENOMEM;
		return NULL;
	}
	subskel->syms = syms;

	syms[0].name = "var1";
	syms[0].section = ".data";
	syms[0].addr = (void**) &obj->data.var1;
	syms[1].name = "var2";
	syms[1].section = ".bss";
	syms[1].addr = (void**) &obj->bss.var2;
	syms[2].name = "var3";
	syms[2].section = ".bss";
	syms[2].addr = (void**) &obj->bss.var3;
	syms[3].name = "libout1";
	syms[3].section = ".bss";
	syms[3].addr = (void**) &obj->bss.libout1;

	err = bpf_object__open_subskeleton(subskel);
	if (err) {
		test_subskeleton_lib__destroy(obj);
		errno = err;
		return NULL;
	}

	return obj;
}

#ifdef __cplusplus
struct test_subskeleton_lib *test_subskeleton_lib::open(const struct bpf_object
*src) { return test_subskeleton_lib__open(src); }
void test_subskeleton_lib::destroy(struct test_subskeleton_lib *skel) {
test_subskeleton_lib__destroy(skel); }
#endif /* __cplusplus */

#endif /* __TEST_SUBSKELETON_LIB_SKEL_H__ */



  reply	other threads:[~2022-03-03  0:06 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-02  2:48 [PATCH bpf-next 0/4] Subskeleton support for BPF libraries Delyan Kratunov
2022-03-02  2:48 ` [PATCH bpf-next 3/4] libbpf: add subskeleton scaffolding Delyan Kratunov
2022-03-02 21:43   ` Daniel Borkmann
2022-03-03  0:20     ` Delyan Kratunov
2022-03-03  0:28       ` Andrii Nakryiko
2022-03-03  0:44         ` Delyan Kratunov
2022-03-03  4:33   ` Andrii Nakryiko
2022-03-03  4:34     ` Andrii Nakryiko
2022-03-03 19:09       ` Delyan Kratunov
2022-03-04 19:40         ` Andrii Nakryiko
2022-03-02  2:48 ` [PATCH bpf-next 4/4] selftests/bpf: test subskeleton functionality Delyan Kratunov
2022-03-02 22:30   ` Alexei Starovoitov
2022-03-03  0:06     ` Delyan Kratunov [this message]
2022-03-03  4:58   ` Andrii Nakryiko
2022-03-02  2:48 ` [PATCH bpf-next 1/4] libbpf: expose map elf section name Delyan Kratunov
2022-03-03  1:13   ` Andrii Nakryiko
2022-03-03 18:19     ` Delyan Kratunov
2022-03-02  2:48 ` [PATCH bpf-next 2/4] bpftool: add support for subskeletons Delyan Kratunov
2022-03-03  1:46   ` Andrii Nakryiko
2022-03-03 18:57     ` Delyan Kratunov
2022-03-03 20:54       ` Delyan Kratunov
2022-03-04 19:29         ` Andrii Nakryiko
2022-03-04 19:29       ` Andrii Nakryiko
2022-03-10  0:09         ` Delyan Kratunov
2022-03-10  0:38           ` 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=decc0e45afff1ed4bdb65e929aab24d7f66ed921.camel@fb.com \
    --to=delyank@fb.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    /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.