On Fri, Jun 11, 2021 at 02:07:10PM -0700, Tanner Love wrote:
quoted
A nit. It is a good chance to move the new BTF_ID_LIST_SINGLE
and most of the check_flow_keys_access() to filter.c.
Take a look at check_sock_access().
It's not clear to me why it's preferable to move most of the
check_flow_keys_access() to filter.c. In particular, the part of
your comment that I don't understand is the "most of" part. Why
would we want to separate the flow-keys-access-checking logic
into two separate functions? Thanks
Right, actually, the whole function can be moved.
I found it easier to follow from flow_dissector_is_valid_access()
to flow_keys's access check without jumping around between two
different files.
Additionally, it seems that we don't actually need the
BTF_ID_LIST_SINGLE. Can we not, instead, set
regs[value_regno].btf_id to
btf_find_by_name_kind(btf_vmlinux, "virtio_net_hdr", BTF_KIND_STRUCT)
? (And we'll check that that value is not <= 0.)
BTF_ID_LIST_SINGLE is resolved during compilation.
btf_find_by_name_kind() will be repeatedly finding the btf_id during runtime.
It is not like a killer but still unnecessary.