1 Attachment(s)
New Feature Ready for Review: VLAN Support
Hello,
This is my first contribution and I haven't worked in c/c++ for quite a while, so it took me a while to figure out the exact place and method for this feature enhancement.
I have a working proof-of-concept (patch attached), but I'm hoping some existing contributor(s) can help me get it across the line. I'm not sure what your style and implementation preferences are.
Problem Statement:
Packets with VLAN tagging (802.11q) are ignored by showeq
Solution:
This feature allows showeq to decode packets that include VLAN tagging.
Motivation:
I have a network setup with a managed switch and a SPAN port that is mirroring traffic to a dedicated secondary NIC in a showeq VM. Because of my specific switch design (TP-LINK) and usage of VLANs, the mirrored traffic ends up including traffic both with and without VLAN tagging (incoming traffic is untagged, outgoing traffic is tagged).
Implementation Details:
I discovered that while the packets were being seen by libpcap's packet filter, they were ultimately ending up rejected by showeq. The additional 4 bytes in the ethernet header meant that VLAN-tagged packets ended up failing to decode, as the code would attempt to index into the memory buffer to access the packet payload, but would end up misaligned with the IP header, causing it to believe it was a non-IP packet.
To solve this, I modified packetcapture.cpp PacketCaptureThread :: packetCallback
I added a new define check, for PCAP_VLAN, which when built into the project (./configure CXXFLAGS='-DPCAP_VLAN'), examines the packet header for ETHERTYPE_VLAN, and if found, skips the 4-byte VLAN field when copying the data into the packetCache. This allows showeq to process it just like any other normal IP packet.
I used the inspiration from the PCAP_DEBUG section (which I also modified slightly), as it already showed how to examine the packet for ETHERTYPE_IP, so I just had to add an additional check there for ETHERTYPE_VLAN so I could confirm that the packets were making it past libpcap.
However, I don't know the full performance implications of the additional if checks at runtime, which is why I included this change as a compiler-time flag. If maintainers believe it would make sense at runtime, I'm open to changes!
original code
Code:
struct packetCache *pc;
PacketCaptureThread* myThis = (PacketCaptureThread*)param;
pc = (struct packetCache *) malloc (sizeof (struct packetCache) + ph->len);
pc->len = ph->len;
memcpy (pc->data, data, ph->len);
pc->next = NULL;
modified code
Code:
#ifdef PCAP_VLAN
// check the type from header
struct ether_header* packetHeader = (struct ether_header*) data;
uint16_t packetType = ntohs(packetHeader->ether_type);
uint32_t packetLength = ph->len;
if (packetType == ETHERTYPE_VLAN)
{
// will use 4 less bytes without VLAN
packetLength -= 4;
}
struct packetCache *pc;
PacketCaptureThread* myThis = (PacketCaptureThread*)param;
pc = (struct packetCache *) malloc (sizeof (struct packetCache) + packetLength);
pc->len = packetLength;
if (packetType == ETHERTYPE_VLAN)
{
// copy first 12 bytes (src/dst)
memcpy (pc->data, data, 12);
// copy remaining bytes, skipping the 4 byte VLAN header (12 + 4 = 16)
memcpy (pc->data + 12, data + 16, packetLength - 12);
}
else
{
// copy the entire packet
memcpy (pc->data, data, packetLength);
}
pc->next = NULL;
#else
struct packetCache *pc;
PacketCaptureThread* myThis = (PacketCaptureThread*)param;
pc = (struct packetCache *) malloc (sizeof (struct packetCache) + ph->len);
pc->len = ph->len;
memcpy (pc->data, data, ph->len);
pc->next = NULL;
#endif
please see patch file and advise. TIA
Re: New Feature Ready for Review: VLAN Support
Hi! Thanks for your submission.
Honestly, I'm a bit surprised by this issue. In my experience, drivers typically process and strip the dot1q tags, so you never even see them. Anecdotally, none of the systems on my network show the vlan tag in packet captures, but I know the tags are there because I can see them if I do a capture on the switch. Out of curiosity, what nic/driver is your SEQ box using?
That said, SEQ isn't exactly a typical application, so there are bound to be edge-cases, especially related to networking.
I don't think I'm going to be able to test this myself, but if it works for you, and we keep it as a compile-time option, I don't have an issue merging it. At a cursory glance, it looks fine, but I'll do a proper review as soon as I get a chance. (probably this weekend).
Re: New Feature Ready for Review: VLAN Support
Hi cn187, thank you for your response.
You're (obviously) correct that typically one might have the drivers strip VLAN information assuming the NIC is configured to do so.
That's actually one of the first things I tried while investigating why I couldn't get showeq to work.
That's when I discovered that some of the traffic hitting my interface was untagged, so even if I created a VLAN interface, which would strip the tags from that traffic, I only had some of the traffic on a single interface.
The problem comes down to my network switch, made by TP-LINK, which when doing port mirroring, doesn't care what the settings are on the destination port. Other switches, such as more advanced Cisco switches, allow you to configure the behavior on a mirror port.
So for example, ideally assuming I had "VLAN 5" traffic getting mirrored, I could set the destination port to VLAN 5 and then it would strip the tags for me automatically at the switch. Unfortunately, this TP-LINK switch doesn't do that. It's mirroring traffic from an "access" port, so all the incoming traffic it mirrors is untagged, but the outgoing traffic (destined for the router) ends up tagged.
In addition, because this is mirror traffic, I don't have the dedicated NIC that is receiving the traffic connected to an actual network. It's got a link-local address since showeq requires an IP to bind to it, but the interface is getting sent all the packets it needs without requiring any IP binding at all (confirmed with wireshark).
One of the advantages of the implementation I proposed is that in theory I can combine multiple port mirrors into my single destination port, so I could have multiple different VLANs of mirrored traffic hitting my showeq's dedicated mirror NIC, and I could sniff eq sessions on any of them.
As far as testing, I created an offline packet capture and then replayed it while I was doing my testing. I would share it but I'm not sure of the privacy of the eq packets in my capture.
Please let me know if you have any other questions! So far my solution has been working great for me, and it's awesome being able to run it in a VM in a complex network.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
mystery
One of the advantages of the implementation I proposed is that in theory I can combine multiple port mirrors into my single destination port, so I could have multiple different VLANs of mirrored traffic hitting my showeq's dedicated mirror NIC, and I could sniff eq sessions on any of them.
That's a really nice capability for multi-boxers on TrueBox servers, or households with multiple players and a dedicated showeq box.
Quote:
Originally Posted by
mystery
As far as testing, I created an offline packet capture and then replayed it while I was doing my testing. I would share it but I'm not sure of the privacy of the eq packets in my capture.
The capture will have all kinds of identifying info in it, so you definitely don't want to post it publicly. Thinking about it a little more, I probably can't reproduce your exact scenario, but I should be able to do a capture directly on my switch (since the I can see the tags there) and then use that pcap for playback, which should be good enough.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
cn187
That's a really nice capability for multi-boxers on TrueBox servers, or households with multiple players and a dedicated showeq box.
Yeah that's exactly what I was thinking. There's one edge case that I guess would still exist, where if you somehow used multiple different VLANs and then allowed the same IP addressing on each (like two diff vlans but they both had same IP). At that point you would need to instead preserve the VLAN information, or pass it further into the stack so that they could be categorized separately in the UI, but I don't forsee that situation being likely even with my weird edge case (all my boxes are on the same VLAN already).
Quote:
Originally Posted by
cn187
The capture will have all kinds of identifying info in it, so you definitely don't want to post it publicly. Thinking about it a little more, I probably can't reproduce your exact scenario, but I should be able to do a capture directly on my switch (since the I can see the tags there) and then use that pcap for playback, which should be good enough.
If you're just looking to get some traffic with 802.1q tags, and your switch supports TRUNK or GENERAL mode, you might be able to add an additional VLAN to the switch port and it will keep that traffic tagged (since trunked VLAN traffic stays tagged, or any VLAN beyond the first one on a GENERAL port, which seems to combine TRUNK and ACCESS). You probably already know all this, but just in case it helps!
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
mystery
One of the advantages of the implementation I proposed is that in theory I can combine multiple port mirrors into my single destination port, so I could have multiple different VLANs of mirrored traffic hitting my showeq's dedicated mirror NIC, and I could sniff eq sessions on any of them.
Quote:
Originally Posted by
cn187
That's a really nice capability for multi-boxers on TrueBox servers, or households with multiple players and a dedicated showeq box.
I've been wondering about how much extra load the port mirroring puts on the network. I multibox on multiple machines and the way I ended up giving the same functionality is that the secondary switch that's connected to the machines running EQ are mirroring their traffic to the switch port that is connected to another switch with the showEQ box on it. Since the port being mirrored contains all of the traffic from the other switch, I am able to easily swap between which machine it is monitoring. But I think by doing this, it's also creating a ton of additional, unnecessary traffic on the network, both when EQ is running and when it is not.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
xerxes
I've been wondering about how much extra load the port mirroring puts on the network. I multibox on multiple machines and the way I ended up giving the same functionality is that the secondary switch that's connected to the machines running EQ are mirroring their traffic to the switch port that is connected to another switch with the showEQ box on it. Since the port being mirrored contains all of the traffic from the other switch, I am able to easily swap between which machine it is monitoring. But I think by doing this, it's also creating a ton of additional, unnecessary traffic on the network, both when EQ is running and when it is not.
To clarify - You have the EQ client switch mirroring multiple ports to a singe port, then you're connecting that mirrored port to the SEQ switch. But then on the SEQ switch, you have that port mirrored again to the SEQ box's port?
1 Attachment(s)
Re: New Feature Ready for Review: VLAN Support
Code:
< Main Router > <-- Port 16 on Secondary Switch --> <-- Secondary Switch -->
< Main Router > <-- Port 4 on Main Switch --> < SEQ box >
< Secondary Switch > <------> < EQ1 >
< Secondary Switch > <------> < EQ2 >
< Secondary Switch > <------> < EQ3 >
On the second switch, port mirroring is enabled to mirror to port 16. Source is port 15, which doesn't have anything plugged into it. This has the effect of having all of the switch ports on the secondary switch be mirrored onto the link to the main router, which then gives the SEQ box on the main router access to any EQ session.
Attachment 859
https://www.showeq.net/forums/image/...AASUVORK5CYII=
Re: New Feature Ready for Review: VLAN Support
But you have 16 mirrored to 4 on main, right? Otherwise, how is SEQ on 4 seeing the traffic on 16? At any rate - it's working for you, so maybe the details don't matter.
Though, I'm curious why you say it's generating a bunch of unnecessary network traffic. It seems to me that the only system that would see more traffic than usual is the SEQ box itself. Sure, there will be higher load on the switch, but assuming the link to the secondary isn't saturated, I don't really see a problem with your setup.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
cn187
But you have 16 mirrored to 4 on main, right? Otherwise, how is SEQ on 4 seeing the traffic on 16? At any rate - it's working for you, so maybe the details don't matter.
Though, I'm curious why you say it's generating a bunch of unnecessary network traffic. It seems to me that the only system that would see more traffic than usual is the SEQ box itself. Sure, there will be higher load on the switch, but assuming the link to the secondary isn't saturated, I don't really see a problem with your setup.
Yep, that's correct. I left off that there's another switch that is mirroring the traffic to the SEQ box.
It's dumb mirroring a 16 port switch to another switch port on another switch. Not that there's a lot of traffic for EQ, but it could be slinging a lot of unnecessary packets unrelated to EQ at times.
Sorry to derail this thread. It seems somewhat related at first glance.
Re: New Feature Ready for Review: VLAN Support
Mystery - I haven't forgotten about this. I did a capture from my switch (in order to get the traffic with vlan tags) for testing, but it doesn't seem to be working. I assume it's an issue with my capture rather than with your patch, but things have been busy so I haven't had time to debug it. I'm hoping to get it wrapped up this weekend.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
cn187
Mystery - I haven't forgotten about this. I did a capture from my switch (in order to get the traffic with vlan tags) for testing, but it doesn't seem to be working. I assume it's an issue with my capture rather than with your patch, but things have been busy so I haven't had time to debug it. I'm hoping to get it wrapped up this weekend.
Thanks, no rush! I'm still happily running off the code changes on my local with no issues.
Re: New Feature Ready for Review: VLAN Support
Did you have to change the pcap filter strings? Asking because several of them include "and ether proto 0x0800", which should exclude packets with vlan tags (which are proto 0x8100) which means your modified packetCallBack would never see the packet.
Re: New Feature Ready for Review: VLAN Support
I don't think I had to change pcap filter strings, but unfortunately it's been a couple months since I messed with this. I had some major life changes and stepped away from EQ/SEQ for now.
Hopefully this patch can at least remain in this thread for anyone else who might need it in the future. Apologies that I can't finish bringing it over the line in the near term.
Re: New Feature Ready for Review: VLAN Support
No worries. I'm still planning on merging it at some point, and do appreciate you submitting what you have.
Re: New Feature Ready for Review: VLAN Support
This is generally better handled by letting the os network stack do its job, and at most run namespacing for isolation for the clients. Its total overkill to have the application understand stuff below layer 3 when the stack itself is designed to handle all of this trivially already.
Like you could add support for vxlan, qinq etc, or just configure the virtual interfaces on the box properly in about 4 lines of netconf and just tap the virtual interface and avoid all this extra work duplicating the effort.
Re: New Feature Ready for Review: VLAN Support
Quote:
Originally Posted by
santa47
This is generally better handled by letting the os network stack do its job, and at most run namespacing for isolation for the clients. Its total overkill to have the application understand stuff below layer 3 when the stack itself is designed to handle all of this trivially already.
Like you could add support for vxlan, qinq etc, or just configure the virtual interfaces on the box properly in about 4 lines of netconf and just tap the virtual interface and avoid all this extra work duplicating the effort.
If that would have worked, I would have done that. I spent extensive time trying to figure out a solution without modifying the application code.
The issue is related to the TP-LINK switch I have, and how it does 802.1q tagging. Because some of the packets end up VLAN tagged and some do not (due to the mechanics of the mirror switch port), simply assigning VLANs at the network stack level does not solve it.
Thanks for your suggestion, though.
Edit: I would also argue that doing things by "letting the os network stack do its job" seems to be ignorant of the fact that seq itself is doing packet analysis already. It's a moot point.
Re: New Feature Ready for Review: VLAN Support
You can easily bridge the tagged and untagged interface in 2 lines of netconfig....