Re: [PATCH bpf-next 4/4] selftests/bpf: test subskeleton functionality

[Delyan Kratunov <delyank@xxxxxx>]

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__ */