On 18/10/18 22:19, Martin Lau wrote: > As I have mentioned earlier, it is also special to > the kernel because the BTF verifier and bpf_prog_load() > need to do different checks for FUNC and FUNC_PROTO to > ensure they are sane. > > First, we need to agree that the kernel needs to verify > them differently. > > and then we can proceed to discuss how to distinguish them. > We picked the current way to avoid adding a > new BTF function section and keep it > strict forward to distinguish them w/o relying > on other hints from 'struct btf_type'. > > Are you suggesting another way of doing it? But you *do* have such a new section. The patch comment talks about a 'FuncInfo Table' which appears to map (section, insn_idx) to type_id. (I think this gets added in .BTF.ext per patch 9?) So when you're looking at a FUNC type because you looked up a type_id from that table, you know it's the signature of a subprogram, and you're checking it as such. Whereas, if you were doing something with some other type and it referenced a FUNC type (e.g., in the patch comment's example, you're checking foo's first argument against the type bar) in its type_id, you know you're using it as a formal type (a FUNC_ PROTO in your parlance) and not as a subprogram. The context in which you are using a type entry tells you which kind it is. And the verifier can and should be smart enough to know what it's doing in this way.
And it's entirely reasonable for the same type entry to get used for both those cases; in my example, you'd have a FUNC type for int foo(int), referenced both by the func_info entry for foo and by the PTR type for bar. And if you had another subprogram int baz(int), its func_info entry could reference the same type_id, because the (reference to the) name of the function should live in the func_info, not in the type. What you are proposing seems to be saying "if we have this particular special btf_kind, then this BTF entry doesn't just define a type, it declares a subprogram of that type". Oh, and with the name of the type as the subprogram name. Which just creates blurry confusion as to whether BTF entries define types or declare objects; IMNSHO the correct approach is for objects to be declared elsewhere and to reference BTF types by their type_id. Which is what the func_info table in patch 9 appears to do. (It also rather bothers me the way we are using special type names to associate maps with their k/v types, rather than extending the records in the maps section to include type_ids referencing them. It's the same kind of weird implicitness, and if I'd spotted it when it was going in I'd've nacked it, but I suppose it's ABI now and too late to change.) -Ed
