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

Add support for out-of-band VLAN IDs on afpacket sources. #462

Merged
merged 3 commits into from
Sep 6, 2018

Conversation

deadpixi
Copy link
Contributor

@deadpixi deadpixi commented Jun 6, 2018

The Linux kernel supports VLAN offloading, which will strip off VLAN headers before passing the packet data up to the capture layer. This results in the odd situation where the data presented to the capture layer is not what was actually visible on the wire.

For those who needed visibility into the VLAN layer, the afpacket.OptAddVLANHeader option was provided. When set, a synthetic VLAN header is created and copied over. While this works and provides a more accurate view of the data on the wire, it is slow (as noted in the documentation of the option, sometimes by an order of magnitude).

This commit augments the gopacket.CaptureInfo structure by adding a VLAN field that stores the VID (VLAN ID) of the packet if it was provided out-of-band by the capture mechanism. If it was not provided, this field defaults to -1 (it should not default to zero, as it is possible to have a zero VID, which is used for priority tagging).

This provides a fast mechanism to access VLAN information for those who need it while retaining the afpacket.OptAddVLANHeader mechanism for those who need it.

The Linux kernel suports VLAN offloading, which can end up
stripping VLAN headers off of captured packets.

For those who need it, the `afpacket.OptAddVLANHeader` option
will add in a synthetic VLAN header created from the out-of-
band information reported by the kernel. This is great, but
slow.

This commit instead provides that information out-of-band,
which is faster but works out-of-band from the normal layer
decoding mechanism.
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@deadpixi
Copy link
Contributor Author

deadpixi commented Jun 6, 2018

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

Copy link
Collaborator

@gconnell gconnell left a comment

Choose a reason for hiding this comment

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

I'm a little leary of this, as it puts something (VLAN) that's only needed rarely (AF_PACKET-specific) in a very general (CaptureInfo) location.

Anyone else have any thoughts on this?

PS: I'll be on vacation through 6/18, so probably won't respond again until at least then. Apologies for the delay.

@deadpixi
Copy link
Contributor Author

deadpixi commented Jun 7, 2018

Perhaps it would be better to have a slice of interface{}s in the returned CaptureInfo that are "ancillary data" or something? It could be populated with, for example, afpacket.AuxDataVLAN or something that could be discovered at runtime.That would keep it generic but allow extra information should the capture layer be able to provide it.

This would be analogous to the ancillary data on UNIX domain sockets.

@gconnell
Copy link
Collaborator

Hey, sorry for the slow response, just coming back from a vacation.

I like the idea of ancillary data. So I'm guessing it would be something like this:

func (c *CaptureInfo) AddAncillary(a interface{}) { ... }
func (c CaptureInfo) Ancillary() []interface{} { ... }

And it would be used like this:

for _, a := range ci.Ancillary() {
  switch v := a.(type) {
    case afpacket.AncillaryVLAN:
      log.Printf("vlan ID %d", int(v))
  }
}

@deadpixi
Copy link
Contributor Author

@gconnell I've switched to a more generic "ancillary data" mechanism now in my branch. Currently it's simply a slice of interface{} that can be appended to by the packet provider; I can switch to using accessor methods if you think that's better. Please let me know.

@gconnell gconnell merged commit d552596 into google:master Sep 6, 2018
traetox pushed a commit to traetox/gopacket that referenced this pull request Jan 2, 2019
* Added support for out-of-band VLAN tagging.

The Linux kernel suports VLAN offloading, which can end up
stripping VLAN headers off of captured packets.

For those who need it, the `afpacket.OptAddVLANHeader` option
will add in a synthetic VLAN header created from the out-of-
band information reported by the kernel. This is great, but
slow.

This commit instead provides that information out-of-band,
which is faster but works out-of-band from the normal layer
decoding mechanism.

* Add AncillaryData to CaptureInfo.

* Report capture VLAN via AncillaryData.
traetox pushed a commit to traetox/gopacket that referenced this pull request Jan 2, 2019
* Added support for out-of-band VLAN tagging.

The Linux kernel suports VLAN offloading, which can end up
stripping VLAN headers off of captured packets.

For those who need it, the `afpacket.OptAddVLANHeader` option
will add in a synthetic VLAN header created from the out-of-
band information reported by the kernel. This is great, but
slow.

This commit instead provides that information out-of-band,
which is faster but works out-of-band from the normal layer
decoding mechanism.

* Add AncillaryData to CaptureInfo.

* Report capture VLAN via AncillaryData.
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.

3 participants