Re: [PATCH v6 bpf-next 3/9] libbpf: Add BTF_KIND_FLOAT support

[Yonghong Song <yhs@fb.com>]

On 2/25/21 5:30 PM, John Fastabend wrote:
> Ilya Leoshkevich wrote:
>> The logic follows that of BTF_KIND_INT most of the time. Sanitization
>> replaces BTF_KIND_FLOATs with equally-sized empty BTF_KIND_STRUCTs on
>> older kernels, for example, the following:
>>
>>      [4] FLOAT 'float' size=4
>>
>> becomes the following:
>>
>>      [4] STRUCT '(anon)' size=4 vlen=0
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>> ---
> 
> [...]
> 
>> @@ -2445,6 +2450,10 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
>>   		} else if (!has_func_global && btf_is_func(t)) {
>>   			/* replace BTF_FUNC_GLOBAL with BTF_FUNC_STATIC */
>>   			t->info = BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0);
>> +		} else if (!has_float && btf_is_float(t)) {
>> +			/* replace FLOAT with an equally-sized empty STRUCT */
>> +			t->name_off = 0;
> 
> Can we keep the name_off from btf__add_float()? Or just explain why
> we zero it here, its not obvious to me at least.

This is mostly to avoid type name collision, e.g., after sanitation, we 
may end up with "struct float {}" or "typedef char float[8]". All these 
will be rejected with strict enforcement although we didn't do it today.
Consider a rare case, users get btf from kernel, dump it into a C file
and then it may not compile...
In my opinion, if user needs any information, libbpf should emit them 
during the time of sanitation. Maybe some comments will be good here.

> 
>> +			t->info = BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 0);
>>   		}
>>   	}
>>   }
>> @@ -3882,6 +3891,18 @@ static int probe_kern_btf_datasec(void)