On Mon, Jul 25, 2022 at 12:10 AM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > From: Alexei Starovoitov [mailto:alexei.starovoitov@xxxxxxxxx] > > Sent: Friday, July 22, 2022 7:55 PM > > On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote: > > > The bpf() system call validates the bpf_attr structure received as > > > argument, and considers data until the last field, defined for each > > > operation. The remaing space must be filled with zeros. > > > > > > Currently, for bpf_*_get_fd_by_id() functions except bpf_map_get_fd_by_id() > > > the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space > > > will result in bpf() rejecting the argument. > > > > The kernel is doing the right thing. It should not ignore fields. > > Exactly. As Andrii requested to add opts to all bpf_*_get_fd_by_id() > functions, the last field in the kernel needs to be updated accordingly. > It's been a while ago so details are hazy. But the idea was that if we add _opts variant for bpf_map_get_fd_by_id() for interface consistency all the other bpf_*_get_fd_by_id() probably should get _opts variant and use the same opts struct. Right now kernel doesn't support specifying flags for non-maps and that's fine. I agree with Alexei that kernel shouldn't just ignore unrecognized field silently. I think we still can add _opts() for all APIs, but user will need to know that non-map variants expect 0 as flags. For now. If we eventually add ability to specify flags for, say, links, then existing API will just work. One can see how this get_fd_by_id() can use read-only flags to return FDs that only support read-only operations on objects (e.g., fetching link info for links, dumping prog instructions for programs), but not modification operations (e.g., updating prog for links, or whatever write operation could be for programs). So I don't think there is contradiction here. We might choose to add bpf_map_get_fd_by_id_opts() only, but we probably still should use common struct name as if all bpf_*_get_fd_by_id_opts() exist. > Roberto
