Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: helpers functions to convert values host to network byte order #64

Closed
wants to merge 1 commit into from

Conversation

DaFray31
Copy link
Contributor

@DaFray31 DaFray31 commented Oct 12, 2024

  • bpf_htons
  • bpf_htonl
  • bpf_ntohl
  • bpf_ntohs

linked to #7

@dylandreimerink
Copy link
Collaborator

I think this is in the right spirit but I think we need to change its execution a bit.

I do not think we should start adding these to helper functions, since they are not the same thing (even though it appears similar to new users). I think these should go under ebpf-library/libbpf (would like to discuss the best layout of pages). The reason being that the actual bpf_htons, bpf_htonl, ect. are defined in bpf_endian.h not in any kernel headers.

Because these are not helpers, we can also adopt a new page format. For these functions the program types are not important, but we might want to look into libbpf library versions and clang/gcc support for the builtins they use instead. Talk about when the old instructions are used and when ISAv4 instructions are emitted (in this endian conversion case).

With regard to the pages layout / doc structure I was thinking the following:

  • Separate the userspace and kernel logic/functions/macros/API (since libbpf has both)
    • Have a section/dir for the userspace API
    • Have a section/dir for the kernel API, all of the header files you would include from BPF code
    • Have a section/dir for concepts / mini-tutorials for more complex stuff (Skeletons, Structops, Global data, ELF sections and meaning)

Not sure yet on how to do the details. For the kernel API side for example:

  • Do we separate on functionality (functions+function-like macros, program-macros, map-macros)?
  • Do we separate on file (bpf_endian.h, bpf_core_read.h, bpf_tracing.h, ...)?
  • Both?
  • How do we arrange the index

Not to put all of this on you, just thinking aloud. For the purposes of this PR its mostly about the kernel API side of this.

@DaFray31
Copy link
Contributor Author

I fully agree.

I miss that they are linked to libbpf, due to the name and the path in the kernel tools / lib / bpf / bpf_endian.h.

I think we should separate on file. When we search a function we mostly go on the file where the function is defined. What do you mean by index ? (Alphabetical order ?)

I will try to do something in next coming days, probably I will try to push all the libbpf doc.

@dylandreimerink
Copy link
Collaborator

dylandreimerink commented Oct 13, 2024

What do you mean by index ? (Alphabetical order ?)

I mean how to structure the index.md files.

I will try to do something in next coming days, probably I will try to push all the libbpf doc.

That would be amazing, though it seems like quite a lot of work. We don't have to do everything at once.

I couldn't help myself, so I threw together a rough idea for the layout. WDYT?
#68

While looking over the libbpf headers I noticed a lot of the public functions have doc comments, so we might be able to make a tool to parse these similar to what I did for the helper functions. That might speed things up.

Also, the project seems to track which functions were added in which libbpf version in a parse able format, so that also seems like a good opportunity to automate the libbpf version number info.

@DaFray31
Copy link
Contributor Author

DaFray31 commented Oct 14, 2024

That would be amazing, though it seems like quite a lot of work. We don't have to do everything at once.

Yeah, I will do part by part.

For the automation part, should be great. I don't have a great knowledge about Go, but I can try (or maybe you will, faster than me). So for functions, macros etc, definition I should update directly on the libbpf git doc ?
The libbpf documentation should merge with the eBPF doc ? (Because it will be a duplicate no ?)

I think we should close this PR and continue this discussion on the other PR.

@dylandreimerink
Copy link
Collaborator

I can do the tooling bit, no problem. Tough I also don't want to rob you of the experience if you want to give it a Go 😜.

 So for functions, macros etc, definition I should update directly on the libbpf git doc ?
The libbpf documentation should merge with the eBPF doc ? (Because it will be a duplicate no ?)

Adding comments/docs to libbpf has to go via the kernel mailing list. Feel free to do so, but that is a very slow process. Its more practical to make any changes on our docs, then upstream later.

@DaFray31
Copy link
Contributor Author

Tough I also don't want to rob you of the experience if you want to give it a Go

Thanks for the offer, so yes, I want to try :)

@DaFray31 DaFray31 closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants