Re: [PATCH bpf-next v1 7/8] libbpf: sync progs autoload with maps autocreate for struct_ops maps

[Kui-Feng Lee <sinquersw@gmail.com>]

On 2/27/24 15:09, Eduard Zingerman wrote:
> On Tue, 2024-02-27 at 14:55 -0800, Kui-Feng Lee wrote:
> [...]
> 
>>> @@ -4509,6 +4514,28 @@ static int bpf_get_map_info_from_fdinfo(int fd, struct bpf_map_info *info)
>>>    	return 0;
>>>    }
>>>    
>>> +/* Sync autoload and autocreate state between struct_ops map and
>>> + * referenced programs.
>>> + */
>>> +static void bpf_map__struct_ops_toggle_progs_autoload(struct bpf_map *map, bool autocreate)
>>> +{
>>> +	struct bpf_program *prog;
>>> +	int i;
>>> +
>>> +	for (i = 0; i < btf_vlen(map->st_ops->type); ++i) {
>>> +		prog = map->st_ops->progs[i];
>>> +
>>> +		if (!prog || prog->autoload_user_set)
>>> +			continue;
>>> +
>>> +		if (autocreate)
>>> +			prog->struct_ops_refs++;
>>> +		else
>>> +			prog->struct_ops_refs--;
>>> +		prog->autoload = prog->struct_ops_refs != 0;
>>> +	}
>>> +}
>>> +
>>
>> This part is related to the other patch [1], which allows
>> a user to change the value of a function pointer field. The behavior of
>> autocreate and autoload may suprise a user if the user call
>> bpf_map__set_autocreate() after changing the value of a function pointer
>> field.
>>
>> [1]
>> https://lore.kernel.org/all/20240227010432.714127-1-thinker.li@gmail.com/
> 
> So, it appears that with shadow types users would have more or less
> convenient way to disable / enable related BPF programs
> (the references to programs are available, but reference counting
>   would have to be implemented by user using some additional data
>   structure, if needed).
> 
> I don't see a way to reconcile shadow types with this autoload/autocreate toggling
> => my last two patches would have to be dropped.

How about to update autoload according to the value of autocreate of
maps before loading the programs? For example, update autoload in
bpf_map__init_kern_struct_ops()?

> 
> Wdyt?