Comments (22)
100% correct :) There's no logical reason why we should mount the bpffs on .../test
rather than simply .../test/pins
in that case, and mounting it this way could even hide other files that are present under .../test/
, so we would like to fix that. Thanks!
from bpftool.
Hi. I have got a patch ready and have tested it. I have never sent a patch to a linux subsystem's mailing list before. I would like to clarify one thing before sending this patch.
Apart from sending the patch to [email protected]
, I will also need to cc the mail ids suggested by the get_maintainers.pl
script in the linux repo, right?
from bpftool.
Apart from sending the patch to
[email protected]
, I will also need to cc the mail ids suggested by theget_maintainers.pl
script in the linux repo, right?
Correct - You need at least myself (for bpftool), the BPF maintainers (Alexei, Daniel, Andrii), ideally the BPF reviewers; and just add the other people as well if the script mentions them. You don't need linux-kernel@
in copy, though.
For your information, there's some doc on the submitting process:
- Here for the submissions related to BPF
- Here for the generic documentation on submitting patches (it's dense).
Let me know if there's anything else I can help with. If you feel more comfortable sending me a preliminary version to test your setup, that's entirely doable, too.
from bpftool.
Please target
bpf-next
. This is not a kernel bug, simply a bug in bpftool, and we're usually routing them throughbpf-next
- they land in the GitHub repo just as fast.
Got it.
Thanks, I will try to take a look tomorrow.
Sure thing! Thank you.
Relevant, yes, required, no. We have a few tests for bpftool but overall we're behind in terms of CI, so there's no requirement on that side currently. See #64
In that case, I think I'll let this be until later, unless the changes in this patch necessitates a test as soon as possible.
from bpftool.
Hi. I have gone through this page on submitting bpf patches. I also opened a PR here to run the CI against my changes.
I decided to send the patch to the mailing list since there were no problems with the CI run. Please let me know if I have missed something.
Patch thread here.
from bpftool.
The submission itself is all good, thanks! I commented on the code directly on the mailing list.
from bpftool.
Thank you for the review. I'll make the changes and will send a new patch.
from bpftool.
This is the way :)
from bpftool.
Note: Looking at the code around mounting the bpffs in bpftool's prog.c
, I see that we don't call mount_bpffs_for_pin()
for the directory where the user wants to pin maps, if pinmaps
is passed:
err = mount_bpffs_for_pin(pinfile, !first_prog_only);
if (err)
goto err_close_obj;
if (first_prog_only) {
prog = bpf_object__next_program(obj, NULL);
if (!prog) {
p_err("object file doesn't contain any bpf program");
goto err_close_obj;
}
if (auto_attach)
err = auto_attach_program(prog, pinfile);
else
err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
if (err) {
p_err("failed to pin program %s",
bpf_program__section_name(prog));
goto err_close_obj;
}
} else {
if (auto_attach)
err = auto_attach_programs(obj, pinfile);
else
err = bpf_object__pin_programs(obj, pinfile);
if (err) {
p_err("failed to pin all programs");
goto err_close_obj;
}
}
if (pinmaps) {
// XXX No guarantee that the bpffs is mounted for directory pinmaps, here
err = bpf_object__pin_maps(obj, pinmaps);
if (err) {
p_err("failed to pin all maps");
goto err_unpin;
}
}
I haven't tested it though, but I expect the map pinning to fail if we pass a pinmaps
directory that is not under the bpffs.
This is something you could address by adding a second patch to your series, if you wanted - obviously you don't have to. I'll open a dedicated issue to track this otherwise.
from bpftool.
I haven't tested it though, but I expect the map pinning to fail if we pass a
pinmaps
directory that is not under the bpffs.This is something you could address by adding a second patch to your series, if you wanted - obviously you don't have to. I'll open a dedicated issue to track this otherwise.
Hi. I would definitely like to take this up too. I am almost ready with v2 of the first patch. I'll test this out as well. If I can test this out and get a fix ready for this, then I'll send this patch as well as part of v2. Otherwise - if it's all right - I'll work on this while concurrently sending v2 of the first patch for review.
from bpftool.
v5 now merged to bpf-next, thank you!
from bpftool.
Thank you for the help and reviews. I enjoyed working on this issue.
This is great to hear, thanks for your work! And please don't worry about the delays between revisions, it's expected that you're busy (everyone is!) and may need several days to find the time to address feedback.
You've been very responsive to feedback and it was a pleasure to do the reviews on my side, too.
I'll submit another patch for this.
Perfect, thank you!
from bpftool.
Hi. I am quite new to bpf/ebpf. I only recently started studying about it. Having spent some time tinkering with ebpf and bpftool, I have got some familiarity with it.
I am interested in the bpf subsystem of the linux kernel as well, and I think this would be a good first issue to get some exposure to bpftool's internals and linux bpf in general.
I haven't verified firsthand if this issue still exists. I plan on doing that and if it exists, I would like to work on resolving it as well.
from bpftool.
I've not seen any fix being posted or merged, so I'm pretty sure the issue is still present. Thanks for picking this up! Let me know if you need help.
from bpftool.
I'm pretty sure the issue is still present
And this should be easy to check by running an action that may try to load the bpffs on a given directory. If I remember correctly, bpftool prog loadall <elf_file> <some_dir>
should do that and end up mounting the bpffs on the parent dir of <some_dir>
.
from bpftool.
I've not seen any fix being posted or merged, so I'm pretty sure the issue is still present. Thanks for picking this up! Let me know if you need help.
Thank you for the reply. Sure, I'll let you know if I get stuck and need help.
And this should be easy to check by running an action that may try to load the bpffs on a given directory. If I remember correctly,
bpftool prog loadall <elf_file> <some_dir>
should do that and end up mounting the bpffs on the parent dir of<some_dir>
.
Got it. I'll start with this.
from bpftool.
Hi. I have gone through the current implementation and tinkered with bpftool prog loadall
. I think I have understood what the current behavior is and what its expected behavior should be. Correct me if I am wrong.
Reproducing the issue
-
Create new bpffs mountpoint
mount bpffs /var/run/example/map -t bpf
-
Create a normal directory
mkdir -p /var/run/example/test/pins
-
Load and pin bpf program
bpftool prog loadall ELF_OBJ /var/run/example/map/bpf Running `bpftool --bpffs prog show name BPF_OBJ_NAME`
This simply pins the bpf program as
/var/run/example/map/bpf/BPF_OBJ_NAME
. Here,/var/run/example/map
is the bpffs.But when pinning to
/var/run/example/test/pins
,bpftool prog loadall ELF_OBJ /var/run/example/test/pins
the bpf program is pinned as
/var/run/example/test/pins/BPF_OBJ_NAME
. However,cat /proc/mounts
shows that a new bpffs is mounted at/var/run/example/test
.
Resolving this issue would mount the bpffs at /var/run/example/test/pins
, right?
from bpftool.
Thank you for the reply.
mounting it this way could even hide other files that are present under
.../test/
Right, I noticed this and for a moment I thought I had corrupted my system. I'll start working on a fix.
from bpftool.
Thank you for the reply.
Correct - You need at least myself (for bpftool), the BPF maintainers (Alexei, Daniel, Andrii), ideally the BPF reviewers; and just add the other people as well if the script mentions them. You don't need
linux-kernel@
in copy, though.
Understood, I'll do that.
For your information, there's some doc on the submitting process:
* [Here for the submissions related to BPF](https://docs.kernel.org/bpf/bpf_devel_QA.html#submitting-patches) * [Here for the generic documentation on submitting patches](https://docs.kernel.org/process/submitting-patches.html) (it's dense).
I had read through the second resource. I almost missed the first one. I'll go through that too.
Let me know if there's anything else I can help with.
I am confused about one thing. I prepared my patch with the bpf-next
prefix since that's the authoritative source code for development and this repo is synced with that tree. After reading the answer to this question and netdev's doc, I am slightly confused about which tree it should target. Since this issue has been tagged as a bug, should I rebase my patch against the bpf
tree?
If you feel more comfortable sending me a preliminary version to test your setup, that's entirely doable, too.
Thank you, that will be very helpful. I'll attach the file here.
0001-bpftool-Mount-bpffs-on-provided-dir-instead-of-paren.zip
Also, I haven't written any test yet. That will be a learning curve as well for me. I'll work on writing a test if that's required and relevant for this patch.
from bpftool.
I am confused about one thing. I prepared my patch with the
bpf-next
prefix since that's the authoritative source code for development and this repo is synced with that tree. After reading the answer to this question and netdev's doc, I am slightly confused about which tree it should target. Since this issue has been tagged as a bug, should I rebase my patch against thebpf
tree?
Please target bpf-next
. This is not a kernel bug, simply a bug in bpftool, and we're usually routing them through bpf-next
- they land in the GitHub repo just as fast.
If you feel more comfortable sending me a preliminary version to test your setup, that's entirely doable, too.
Thank you, that will be very helpful. I'll attach the file here.
0001-bpftool-Mount-bpffs-on-provided-dir-instead-of-paren.zip
Thanks, I will try to take a look tomorrow.
Also, I haven't written any test yet. That will be a learning curve as well for me. I'll work on writing a test if that's required and relevant for this patch.
Relevant, yes, required, no. We have a few tests for bpftool but overall we're behind in terms of CI, so there's no requirement on that side currently. See #64
from bpftool.
v5 now merged to bpf-next, thank you!
Thank you for the help and reviews. I enjoyed working on this issue.
This is something you could address by adding a second patch to your series, if you wanted - obviously you don't have to. I'll open a dedicated issue to track this otherwise.
I'll submit another patch for this.
from bpftool.
This is great to hear, thanks for your work! And please don't worry about the delays between revisions, it's expected that you're busy (everyone is!) and may need several days to find the time to address feedback.
You've been very responsive to feedback and it was a pleasure to do the reviews on my side, too.
Thank you, I appreciate it.
from bpftool.
Related Issues (20)
- Print error and exit instead of diplaying an empty map for unsupported map types
- Fix weird indent in documentation HOT 1
- streamline bpftool net dump HOT 1
- "make install" for man pages stops. HOT 1
- Wrong callq address displayed HOT 5
- Error: No JIT disassembly support HOT 2
- "libbpf: map 'xxx': unsupported map linkage static" in Android HOT 7
- Have bpftool return ID of created objects
- Attach programs to tcx HOT 6
- The proper usage of `bpftool prog attach` HOT 2
- BPFTool Prog Loadall command Deletes /sys/fs/cgroup virtual file system directory HOT 3
- bpftool: error while loading shared libraries: libLLVM-17.so: cannot open shared object file: No such file or directory HOT 14
- use bpftool dump this bpf_prog_982904fb4a4dfbdb_tracepoint_sche at 0x8a0/0x1000 how to do it? HOT 1
- Typo: `cgroup/sendmsg°unix` HOT 2
- Check that the list of supported programs, map types, attach types, ... are up-to-date HOT 1
- Dump libbpf's output for `build_obj_refs_table()` when user asks for debug info HOT 2
- When loading eBPF binary with bpftool, the 'bpf_trace_printk' seems no output to '/sys/kernel/debug/tracing/trace_pipe' HOT 8
- bpftool 7.4.0 prog load Segmentation fault (core dumped) HOT 2
- Missing bpffs mount when pinning maps for prog load (`pinmaps`) HOT 6
- Cannot list programs attached to cgroup HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from bpftool.