On Wed, Jul 20, 2022 at 4:17 PM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > Sent: Thursday, July 21, 2022 1:15 AM > > On Wed, Jul 20, 2022 at 4:12 PM Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > wrote: > > > > > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > > > Sent: Thursday, July 21, 2022 1:09 AM > > > > On Wed, Jul 20, 2022 at 4:02 PM Roberto Sassu > > <roberto.sassu@xxxxxxxxxx> > > > > wrote: > > > > > > > > > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > > > > > Sent: Thursday, July 21, 2022 12:48 AM > > > > > > On Wed, Jul 20, 2022 at 3:44 PM Roberto Sassu > > > > <roberto.sassu@xxxxxxxxxx> > > > > > > wrote: > > > > > > > > > > > > > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > > > > > > > Sent: Thursday, July 21, 2022 12:38 AM > > > > > > > > On Wed, Jul 20, 2022 at 3:30 PM Roberto Sassu > > > > > > <roberto.sassu@xxxxxxxxxx> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > > > > > > > > > Sent: Wednesday, July 20, 2022 5:57 PM > > > > > > > > > > On Wed, Jul 20, 2022 at 1:02 AM Roberto Sassu > > > > > > > > <roberto.sassu@xxxxxxxxxx> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > > > > > > > > > > > > Sent: Tuesday, July 19, 2022 10:40 PM > > > > > > > > > > > > On Tue, Jul 19, 2022 at 12:40 PM Joe Burton > > > > > > > > <jevburton.kernel@xxxxxxxxx> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > From: Joe Burton <jevburton@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > > > > > > > Add an extensible variant of bpf_obj_get() capable of setting > > the > > > > > > > > > > > > > `file_flags` parameter. > > > > > > > > > > > > > > > > > > > > > > > > > > This parameter is needed to enable unprivileged access to > > BPF > > > > > > maps. > > > > > > > > > > > > > Without a method like this, users must manually make the > > > > syscall. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Joe Burton <jevburton@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > > > > > Reviewed-by: Stanislav Fomichev <sdf@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > > > > > For context: > > > > > > > > > > > > We've found this out while we were trying to add support for > > > > unpriv > > > > > > > > > > > > processes to open pinned r-x maps. > > > > > > > > > > > > Maybe this deserves a test as well? Not sure. > > > > > > > > > > > > > > > > > > > > > > Hi Stanislav, Joe > > > > > > > > > > > > > > > > > > > > > > I noticed now this patch. I'm doing a broader work to add opts > > > > > > > > > > > to bpf_*_get_fd_by_id(). I also adjusted permissions of bpftool > > > > > > > > > > > depending on the operation type (e.g. show, dump: > > > > BPF_F_RDONLY). > > > > > > > > > > > > > > > > > > > > > > Will send it soon (I'm trying to solve an issue with the CI, where > > > > > > > > > > > libbfd is not available in the VM doing actual tests). > > > > > > > > > > > > > > > > > > > > Is something like this patch included in your series as well? Can > > you > > > > > > > > > > use this new interface or do you need something different? > > > > > > > > > > > > > > > > > > It is very similar. Except that I called it bpf_get_fd_opts, as it > > > > > > > > > is shared with the bpf_*_get_fd_by_id() functions. The member > > > > > > > > > name is just flags, plus an extra u32 for alignment. > > > > > > > > > > > > > > > > We can bikeshed the naming, but we've been using existing > > conventions > > > > > > > > where opts fields match syscall fields, that seems like a sensible > > > > > > > > thing to do? > > > > > > > > > > > > > > The only problem is that bpf_*_get_fd_by_id() functions would > > > > > > > set the open_flags member of bpf_attr. > > > > > > > > > > > > > > Flags would be good for both, even if not exact. Believe me, > > > > > > > duplicating the opts would just create more confusion. > > > > > > > > > > > > Wait, that's completely different, right? We are talking here about > > > > > > BPF_OBJ_GET (which has related BPF_OBJ_PIN). > > > > > > Your GET_XXX_BY_ID are different so you'll still have to have another > > > > > > wrapper with opts? > > > > > > > > > > Yes, they have different wrappers, just accept the same opts as > > > > > obj_get(). From bpftool subcommands you want to set the correct > > > > > permission, and propagate it uniformly to bpf_*_get_fd_by_id() > > > > > or obj_get(). See map_parse_fds(). > > > > > > > > I don't think they are accepting the same opts. > > > > > > > > For our case, we care about: > > > > > > > > struct { /* anonymous struct used by BPF_OBJ_* commands */ > > > > __aligned_u64 pathname; > > > > __u32 bpf_fd; > > > > __u32 file_flags; > > > > }; > > > > > > > > For your case, you care about: > > > > > > > > struct { /* anonymous struct used by BPF_*_GET_*_ID */ > > > > union { > > > > __u32 start_id; > > > > __u32 prog_id; > > > > __u32 map_id; > > > > __u32 btf_id; > > > > __u32 link_id; > > > > }; > > > > __u32 next_id; > > > > __u32 open_flags; > > > > }; > > > > > > > > So your new _opts libbpf routine should be independent of what Joe is > > > > doing here. > > > > > > It is. Just I use the same opts to set file_flags or open_flags. > > > > That seems confusing. Let's have separate calls for separate syscall > > commands as we do already? > > Can you wait one day, I send what I have, so that we see > everything together? Sure, CC us both on the patches.
