-
Notifications
You must be signed in to change notification settings - Fork 873
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 filtering VNTag frames #1480
base: master
Are you sure you want to change the base?
Add support for filtering VNTag frames #1480
Conversation
I think maybe it's worth revisiting if VLAN should be implicit. Re-reading the PCAPs and thinking on this today, the VNTag header is like the VLAN header which gets injected between dest addr and ethertype. I always forget that VLAN (and apparently VNTag) "splits" the ethernet header. I'll revise this to remove the implicit VLAN and see how it looks. It'll probably still work and be a little simpler (and more explicit, which I think is good). |
9145cb1
to
47df308
Compare
Updated with implicit VLAN removed; this looks much cleaner! |
Thank you, please have some patience because there is a number of other work items that need to be resolved and are in progress already. |
In the next revision of these changes please also add a line to the "packet filtering" section of the |
Reminder to self: rebase and squash before merging! |
Completely understood! No pressure or rush! I'm mostly trying to figure out how to contribute meaningfully in your project's style and cadence. I've been walled up in my own team's process for so long, it's been a while since I worked on other teams. Please let me know if there's anything I can do/change to make the process easier/smoother for you. Thanks for all your hard work maintaining such a great library! |
b4a1cf4
to
18de787
Compare
Add a "vntag" keyword similar to the existing "vlan" and "mpls" keywords to filter on VNTag frames. VNTag was proposed as 802.1Qbh but has been replaced in the standards by 802.1Qbr although some vendors are still using VNTag. Skips the 6 bytes of the VNTag header similar to the 4 byte skip of VLAN. They both inject themselves between Ethernet dest addr and EtherType.
18de787
to
29a7f4e
Compare
Add a "vntag" keyword similar to the existing "vlan" and "mpls" keywords to filter on VNTag frames. VNTag was proposed as 802.1Qbh but has been replaced in the standards by 802.1Qbr although some vendors are still using VNTag.
Because VNTag is always followed by VLAN and the following VLAN has the next EtherType, both the VNTag and VLAN have to be present to match and both have to be skipped in processing. So, this commit also adds an optional
vlan [vlan_id]
after thevntag
keyword to easily filter on the VLAN ID following the VNTag.Fixes #770
First, sorry about having to close the other PR. I had two branches on my side (a misnamed
nvtag
and thisvntag
) and just missed on the PR with the wrong one! I had already started some tests in thevntag
branch. This PR replaced #1479The PCAP file is a real PCAP manually adjusted with Scapy to include a VNTag+VLAN header. It parses properly in Wireshark as VNTag, giving it some legitimacy. I do not have direct access to native VNTag traffic but am asking the team I'm helping out if they can get one. I'm not sure it's necessary, though, given Wireshark agreeing:
