conn,device: support conn.ReceiveFunc nonzero packet offset #36
Conversation
sfllaw
left a comment
sfllaw
left a comment
There was a problem hiding this comment.
Just some notes about sources of confusion from someone unfamiliar with this codebase. Feel free to ignore.
| // sizes, packets, and endpoints that should be evaluated. A sizes element | ||
| // includes both the starting and ending offset for an element of packets. Some |
There was a problem hiding this comment.
❓ Confused: Is the sizes element defined as [start, end)? It would be nice if the documentation were more explicit about ranges.
❓ Confused: Is this the name sizes still the right name for this, now that it is some kind of range or interval? Granted, I cannot come up with a better name.
danderson
left a comment
danderson
left a comment
There was a problem hiding this comment.
LGTM. Reflecting some discussion off-github: we're balancing extending the functionality of this API for a pressing need, vs. doing a larger scale refactor to improve the packet wrangling API as a whole.
The latter is tempting, but is likely to spiral into a large change as it bubbles through more of the API. We'd rather tackle a broader redesign separately (and we're gearing up to, available bandwidth notwithstanding), rather than end up pulling refactors into this targeted feature change.
| msg := &(*msgs)[i] | ||
| sizes[i] = msg.N | ||
| if sizes[i] == 0 { | ||
| sizes[i][0], sizes[i][1] = 0, msg.N |
There was a problem hiding this comment.
minor suggestion, optional: move the continue branch below further up, and make the condition look at msg.N instead of the sizes array? As written here it took me a beat to translate the branch condition to "oh, skip empty messages".
There was a problem hiding this comment.
Suggestion SGTM. I'm leaving it as-is to reduce drift/differing LoC from upstream for this conn.Bind implementation that we don't use.
jwhited
force-pushed
the
jwhited/read-start-offset
branch
from
8c59ede to
0924732
Compare
This enables a conn.Bind to leverage wireguard-go packet memory for reading packets containing layers that wireguard-go should ignore, e.g. a VXLAN or Geneve header preceeding WireGuard. Signed-off-by: Jordan Whited <jordan@tailscale.com>
jwhited
force-pushed
the
jwhited/read-start-offset
branch
from
0924732 to
fd2c830
Compare
|
As-is this commit doesn't account for the extra 8 bytes on the TUN write side of the packet pipeline, which is a bug. Tests pass because there is no test exercising a nonzero receive offset. Addressing this likely requires adding a field to Closing this for now until there's time to revisit. We can have the |
3 participants
This enables a conn.Bind to leverage wireguard-go packet memory for reading packets containing layers that wireguard-go should ignore, e.g. a VXLAN or Geneve header preceeding WireGuard.