New Feature Ready for Review: VLAN Support

[mystery]

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

    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

#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

[cn187]

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).

[mystery]

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.

[cn187]

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.

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.

[mystery]

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).

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!

[xerxes]

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.

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.

[cn187]

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?

[xerxes]

< 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.

[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.

[xerxes]

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.

[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.

[mystery]

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.

[cn187]

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.

[mystery]

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.

[cn187]

No worries. I'm still planning on merging it at some point, and do appreciate you submitting what you have.