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

x/net/bpf: consider adding bpftest package #16055

Closed
mdlayher opened this issue Jun 13, 2016 · 7 comments
Closed

x/net/bpf: consider adding bpftest package #16055

mdlayher opened this issue Jun 13, 2016 · 7 comments

Comments

@mdlayher
Copy link
Member

@mdlayher mdlayher commented Jun 13, 2016

When working with x/net/bpf recently, I realized it would be quite useful to be able to load my BPF programs into an emulated virtual machine, and test them against a variety of byte slice inputs.

I created this package to fulfill that need: https://github.com/mdlayher/bpftest.

While it is useful on its own for testing, I have two questions:

  1. Does it make sense to include this directly in x/net/bpf? Perhaps even in a package like x/net/bpf/bpftest, akin to net/http/httptest.

This package is useful for testing the logic of complicated BPF programs, without needing to actually create a socket and load the BPF program directly. This also allows testing of BPF programs on platforms like Windows, which do not have BPF.

  1. Could this package be useful for more than just testing?

For example, on Windows, there is no BPF. But, perhaps this package could be used as a pseudo-BPF guarded by build tags, so that additional filtering logic would not need to be written when running a program that makes use of a BPF filter on Windows. Of course, this approach would not be nearly as fast as using the in-kernel VM on Linux or BSD, but it would help reduce code duplication.

I'd love to hear your thoughts on these ideas, @danderson and @mikioh . Ultimately, it may just be better for the package to continue existing on my personal GitHub, but I figured I would at least get a discussion around it started. Thanks for your time!

@danderson
Copy link

@danderson danderson commented Jun 13, 2016

Having a Go implementation of the BPF VM sounds great to have in x/net/bpf directly, IMO. It only grows the package API by one function (or is there more?), and it's a natural fit. The package docs will need to be tweaked to point out that this is not how you access the in-kernel filtering functionality, but that's simple enough.

My standard concern when implementing something like that is having a good interoperability test, to make sure the VM matches the reference implementation's behavior. Do you have such a test already?

I'm -0.5 on having automagic use of the VM on platforms that don't support BPF. To me, the packet filtering performance is part of the BPF contract when you attach a program to a socket, so having it sometimes be fast and sometimes be much slower is an undesirably surprising behavior. If a caller wants that behavior, I'd rather have it be explicit in their code, where they do a RecvFrom followed by running the Go VM themselves.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jun 13, 2016

  1. The API is only two functions: https://godoc.org/github.com/mdlayher/bpftest
  • New: creates a VirtualMachine using the input filter
  • VirtualMachine.Run: processes an input byte slice and renders a result using the program.
  1. I totally agree, I did not intend to implement "auto-magical" BPF functionality on Windows and other platforms.

If a caller wants that behavior, I'd rather have it be explicit in their code, where they do a RecvFrom followed by running the Go VM themselves.

This is exactly my intent as well. Provide the capability to do so, but only if configured manually.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jun 13, 2016

As far as interoperability tests, I do not have any yet. I agree that it is a good idea though and will look into doing so with Linux in the near future.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jun 15, 2016

As of today, my bpftest package is fully tested against the Linux BPF VM. You can see the test results here: https://travis-ci.org/mdlayher/bpftest/jobs/137887349. Running tests with -tags=bpf on Linux enables integration testing using a UDP socket.

You can see the code at: https://github.com/mdlayher/bpftest.

Should I submit a CL for review, or are there any further concerns?

@danderson
Copy link

@danderson danderson commented Jun 15, 2016

Very cool, thanks so much for doing that work! I'd suggest making the interop tests Skip() themselves when not run on linux, that'll make them always run with Go's own test infrastructure, without having to do manual tweaking of tags.

Sending a CL sounds great to me. I don't have commit rights to x/net, but I'll gladly throw in a review/+1.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jun 15, 2016

@danderson CL submitted: https://go-review.googlesource.com/#/c/24136/.

I tried to rearrange the files a bit to make it more concise, but left the tests broken up because the files are fairly lengthy. Let me know if I'm on the right track.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 15, 2016

CL https://golang.org/cl/24136 mentions this issue.

@quentinmit quentinmit added this to the Unreleased milestone Jun 17, 2016
@golang golang locked and limited conversation to collaborators Jun 18, 2017
NET12115 added a commit to NET12115/Golang-C-NET that referenced this issue Feb 28, 2022
Fixes golang/go#16055.

Change-Id: I80437e2895b0f2bf23e090dec29bd20c2900db9a
Reviewed-on: https://go-review.googlesource.com/24136
Reviewed-by: David Anderson <danderson@google.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants