Re: [PATCH bpf-next 4/5] bpf: Limit up to 512 bytes for bpf_global_percpu_ma allocation

[Yonghong Song <yonghong.song@linux.dev>]

On 12/13/23 3:09 AM, Hou Tao wrote:
> Hi,
>
> On 12/13/2023 6:31 AM, Yonghong Song wrote:
>> For percpu data structure allocation with bpf_global_percpu_ma,
>> the maximum data size is 4K. But for a system with large
>> number of cpus, bigger data size (e.g., 2K, 4K) might consume
>> a lot of memory. For example, the percpu memory consumption
>> with unit size 2K and 1024 cpus will be 2K * 1K * 1k = 2GB
>> memory.
>>
>> We should discourage such usage. Let us limit the maximum data
>> size to be 512 for bpf_global_percpu_ma allocation.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/verifier.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 0c55fe4451e1..e5cb6b7526b6 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -43,6 +43,8 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
>>   };
>>   
>>   struct bpf_mem_alloc bpf_global_percpu_ma;
>> +#define LLIST_NODE_SZ sizeof(struct llist_node)
>> +#define BPF_GLOBAL_PERCPU_MA_MAX_SIZE  (512 - LLIST_NODE_SZ)
> It seems for per-cpu allocation the extra subtraction is not needed, we
> could use all allocated space in per-cpu pointer. Maybe we could update
> bpf_mem_alloc() firstly to use size instead of size + sizeof(void *) to
> select cache.

Good point. If this works, it can also ensure if users try to allocate
512 bytes. It will go to 512-byte bucket instead of 1024-byte buck.
Will investigate.

>>   
>>   /* bpf_check() is a static code analyzer that walks eBPF program
>>    * instruction by instruction and updates register/stack state.
>> @@ -12091,6 +12093,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   				}
>>   
>>   				if (meta.func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
>> +					if (ret_t->size > BPF_GLOBAL_PERCPU_MA_MAX_SIZE) {
>> +						verbose(env, "bpf_percpu_obj_new type size (%d) is greater than %lu\n",
>> +							ret_t->size, BPF_GLOBAL_PERCPU_MA_MAX_SIZE);
>> +						return -EINVAL;
>> +					}
>>   					mutex_lock(&bpf_percpu_ma_lock);
>>   					err = bpf_mem_alloc_percpu_unit_init(&bpf_global_percpu_ma, ret_t->size);
>>   					mutex_unlock(&bpf_percpu_ma_lock);