Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

block/aoe: use BPF to filter requests for other servers #191

Merged
merged 1 commit into from
Jun 2, 2016
Merged

block/aoe: use BPF to filter requests for other servers #191

merged 1 commit into from
Jun 2, 2016

Conversation

mdlayher
Copy link
Contributor

@mdlayher mdlayher commented Jun 2, 2016

I added functionality tonight to my raw sockets package to enable attaching a BPF program to the raw socket. glide gave me some difficulties so that package bump is not included here. I'd be happy to look into getting that done tomorrow.

This is the improvement I mentioned in #184 , and should stop the AoE server from getting bogged down handling requests that it doesn't care about.

@mischief
Copy link
Contributor

mischief commented Jun 2, 2016

what happens if the mtu changes at runtime?

@mdlayher
Copy link
Contributor Author

mdlayher commented Jun 2, 2016

The interface's MTU is already relied on for:

  • allocating a byte slice to read in ethernet frames
  • determining sector size to report to AoE clients

So I imagine this would not be the only problem area if the interface MTU were to change underneath the service. In this case, we could also just hardcode it to 9000 since the return value just asks BPF to return "up to X" bytes.

I'd be happy to hear if you have a more intelligent solution in mind though.

prog, err := bpf.Assemble([]bpf.Instruction{
// Load major address value from AoE header
bpf.LoadAbsolute{
Off: 16,
Copy link
Contributor

Choose a reason for hiding this comment

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

i hope there is a const for location/size of the major field in header. but seems like the aoe pkg does not expose it. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I tried to keep the API surface small for that package if at all possible. Perhaps this BPF program should reside in the AoE package instead. I do want to create a proper "server" type in that package which would definitely overlap with some of the code in this package.

For the time being, I could add comments explaining the meaning of this magic number, if that works.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. I think a few comments should be good enough for now.

@mdlayher
Copy link
Contributor Author

mdlayher commented Jun 2, 2016

Updated, @xiang90 ! @mischief , any more thoughts on the MTU issue?

@barakmich
Copy link
Contributor

All looks good to me operationally. I'd say, yeah, you probably want a server type, of which this is roughly a good first implementation. Someday, when you want to bake it into your aoe package, we'll just use it :)

👍

@barakmich barakmich merged commit 9761692 into coreos:master Jun 2, 2016
@mdlayher mdlayher deleted the aoe-bpf branch June 2, 2016 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants