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

Support attaching SocketFilter programs to raw sockets #540

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

folbricht
Copy link
Contributor

@folbricht folbricht commented Dec 29, 2021

Adds support for attaching Socket Filter programs to raw sockets. It would look like this:

link.SocketFilterAttachProgram(conn, objs.Filter)

link.SocketFilterDetachProgram(conn)

Any feedback is welcome. I'll also provide an example.

@folbricht folbricht marked this pull request as ready for review January 9, 2022 03:19
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Frank,

How are you opening the raw sockets currently? Go stdlib or via some third part package (e.g. mdlayher/raw)?

  • Taking a plain SockFD int is problematic, since it doesn't work well with the stdlib. It invites calling os.File.Fd() to get the file, but that puts the socket into blocking mode. It's also problematic because it may lead to closed fd reuse. We should take https://golang.google.cn/pkg/syscall/#Conn
  • We don't have to shoehorn filters into the Link interface, it's not possible to call most of the methods. Two free functions to attach and detach would be better i think, similar to RawAttachProgram.

@folbricht
Copy link
Contributor Author

I've been using the stdlib to open the raw socket (similar to the example above). If I understand your suggestion correctly, I should:

  • Wrap the FD into a syscall.Conn which would prevent direct access to it
  • Add a couple of functions to attach/detach. As for naming them, how does RawAttachSocketFilter(syscall.Conn, *ebpf.Program) sound?

@lmb
Copy link
Collaborator

lmb commented Jan 17, 2022

Add a couple of functions to attach/detach.

Yes, without introducing a new link type though. So "replacing linkSocketFilter with two free function" is what I'd like to do.

Can you add a test as well please?

@folbricht
Copy link
Contributor Author

Replaced the Link interface implementation with two functions, and added a simple test (which just checks that attaching/detaching) works. Using a full raw socket involves a bit more code to implement syscall.Conn but I could do that in an example.

@lmb
Copy link
Collaborator

lmb commented Jan 20, 2022

Renamed the free functions to be closer to other functions in the package.

@lmb lmb merged commit 1946eef into cilium:master Jan 20, 2022
@lmb
Copy link
Collaborator

lmb commented Jan 20, 2022

Thanks!

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.

None yet

2 participants