sk_lookup loading support #216
Conversation
|
Hello @raftario This feature looks cool! Thanks to this PR I got to know about sk lookup feature. IMO it's okay to include new kernel features in RedBPF. Even though sk lookup feature If users have problems while they try calling the sk lookup functions with the And you concerned that you implemented only the userspace part of sk |
redbpf/src/lib.rs
Outdated
|
|
||
| pub fn set(&mut self, mut idx: u32, mut fd: RawFd) -> Result<()> { | ||
| pub fn set(&mut self, mut idx: u32, fd: RawFd) -> Result<()> { | ||
| let mut fd = fd as u64; |
There was a problem hiding this comment.
@raftario
I'm not sure why you changed this.
Can you elaborate?
There was a problem hiding this comment.
That was a temporary fix of mine, I tested my changes with the code from this talk which uses sizeof(__u64) as the value size for its socket map, so passing it a pointer to a 32bit value caused errors. This can probably be removed given the value size should be sizeof(int).
|
I'm not particularly familiar with the library so I was probably confused. |
|
Would you be interested in merging this without support in the other crates ? |
|
@raftario I wonder how you used this API. Can you add some example as a doc comment in the SkLookup structure? |
|
I'll add some documentation and convert to a proper PR then ! |
Signed-off-by: Raphaël Thériault <self@raftar.io>
Signed-off-by: Raphaël Thériault <self@raftar.io>
Signed-off-by: Raphaël Thériault <self@raftar.io>
Signed-off-by: Raphaël Thériault <self@raftar.io>
Signed-off-by: Raphaël Thériault <self@raftar.io>
| fn drop(&mut self) { | ||
| if let Some((nfd, lfd)) = self.link.take() { | ||
| unsafe { | ||
| libc::close(lfd); |
There was a problem hiding this comment.
@raftario
I am confused that bpf_link_detach is needed or not.
- Only
closeis needed? bpf_link_detachhas nothing to do with cleanup?bpf_link_detachis called first and thencloseshould be called?
Can you explain about that?
There was a problem hiding this comment.
If you never pin the link, it'll automatically be cleaned up when the file descriptor is closed
This pull request is pretty simple and just adds support for loading
sk_lookupprograms to theredbpfcrate. I'm opening it as a draft as2.sk_lookuprequires a relatively recent kernel version (5.9) and merging this would probably break things for many peoplePoint 1 could be addressed by simply building upon this to add support everywhere else (I'll probably do it at some point but I don't personally need it at the moment),
while for point 2 it might be interesting to consider feature-gating different program types.