Re: [PATCH bpf-next v1 2/8] libbpf: tie struct_ops programs to kernel BTF ids, not to local ids

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2024-02-28 at 11:23 -0600, David Vernet wrote:
> On Tue, Feb 27, 2024 at 10:45:50PM +0200, Eduard Zingerman wrote:
> > Enforce the following existing limitation on struct_ops programs based
> > on kernel BTF id instead of program-local BTF id:
> > 
> >     struct_ops BPF prog can be re-used between multiple .struct_ops &
> >     .struct_ops.link as long as it's the same struct_ops struct
> >     definition and the same function pointer field
> 
> Am I correct in understanding the code that the prog also has to be at the same
> offset in the new type?

Yes, but after this patch it would be offset in current kernel BTF type,
not local BTF type.

> So if we have for example:
> 
> SEC("struct_ops/test")
> int BPF_PROG(foo) { ... }
> 
> struct some_ops___v1 {
> 	int (*test)(void);
> };
> 
> struct some_ops___v2 {
> 	int (*init)(void);
> 	int (*test)(void);
> };

From pov of kernel BTF there is only one 'struct some_ops'.
 
> Then this wouldn't work? If so, would it be possible for libbpf to do something
> like invisibly duplicate the prog and create a separate one for each struct_ops
> map where it's encountered? It feels like a rather awkward restriction to
> impose given that the idea behind the feature is to enable loading one of
> multiple possible definitions of a struct_ops type. 

In combination with the next patch, the idea is to not assign offset
in struct_ops maps which have autocreate == false.

If object corresponding to program above would be opened and
autocreate would be disabled either for some_ops___v1 or some_ops___v2
before load, the program 'test' would get it's offset entry only from
one map. Thus no program duplication would be necessary.

For example, see test case in patch #6:

    struct bpf_testmod_ops___v1 {
    	int (*test_1)(void);
    };

    struct bpf_testmod_ops___v2 {
    	int (*test_1)(void);
    	int (*does_not_exist)(void);
    };

    SEC(".struct_ops.link")
    struct bpf_testmod_ops___v1 testmod_1 = {
    	.test_1 = (void *)test_1
    };

    SEC(".struct_ops.link")
    struct bpf_testmod_ops___v2 testmod_2 = {
    	.test_1 = (void *)test_1,
    	.does_not_exist = (void *)test_2
    };


static void can_load_partial_object(void)
{
	...
	skel = struct_ops_autocreate__open_opts(&opts);
	bpf_program__set_autoload(skel->progs.test_2, false);
	bpf_map__set_autocreate(skel->maps.testmod_2, false);
	struct_ops_autocreate__load(skel);
        ...
}

This should handle your example as well.
Do you find this sufficient or would you still like to have implicit
program duplication logic?