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