virtualsquare / vde-2

GNU General Public License v2.0
217 stars 38 forks source link

VLAN packets are dropped by default #49

Open olivier-matz-6wind opened 10 months ago

olivier-matz-6wind commented 10 months ago

Hello,

In vde_switch, VLAN tagged packets are dropped by default, i.e. when the port VLAN is set to 0.

From what I see in the code, packets are only accepted if their VLAN is enabled for the port. But it seems not possible for a port to be in several VLANs at the same time (i.e. act as a trunk). See portsetvlan().

To me, the default behavior for a switch should be to ignore VLANs in packets. Am I wrong?

To reproduce the issue:

# start vde switch
vde_switch
# plug 2 tap interfaces
sudo vde_plug2tap tap0 &
sudo vde_plug2tap tap1 &
# configure tap0
sudo ip link set tap0 up
sudo ip addr add 192.168.50.1/24 dev tap0
# configure tap1 in another netns, so we can ping
sudo ip netns add temp
sudo ip link set tap1 netns temp
sudo ip netns exec temp ip link set tap1 up
sudo ip netns exec temp ip addr add 192.168.50.2/24 dev tap1
# in a dedicated terminal, dump traffic on tap1
sudo ip netns exec temp tcpdump -i tap1 -n -N -l -e
# check ping
ping -c 1 192.168.50.2

# using scapy, forge an ethernet packet, it passes the switch
p = Ether()/IP(dst='192.168.50.2')/UDP()
sendp(p, iface='tap0')
# a VLAN packet is dropped
p = Ether()/Dot1Q(vlan=1)/IP(dst='192.168.50.2')/UDP()
sendp(p, iface='tap0')

# remove temporary netns when test is done
sudo ip netns delete temp

Here is a draft patch, please let me know if I should submit a PR:

From 416538eb4fff1f7c685e8c9303abfafcdfbbeba0 Mon Sep 17 00:00:00 2001
From: Olivier Matz <olivier.matz@6wind.com>
Date: Fri, 17 Nov 2023 12:07:55 +0100
Subject: [PATCH] vde_switch/port: ignore VLAN by default

Currently, VLAN tagged packets are dropped by default, i.e. when the
port VLAN is set to 0.

Packets are only accepted if their VLAN is enabled for the port. But it
is not possible for a port to be in several VLANs at the same
time (i.e. act as a trunk). See portsetvlan() to be convinced.

When port VLAN is set to 0, do not read the VLAN header, always accept
the packet as-is (as if it is not tagged). In other words, these ports
will act as trunks that are active for all VLANs.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 src/vde_switch/port.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/vde_switch/port.c b/src/vde_switch/port.c
index 016fbf1..a5c3449 100644
--- a/src/vde_switch/port.c
+++ b/src/vde_switch/port.c
@@ -608,7 +608,10 @@ void handle_in_packet(struct endpoint *ep,  struct packet *packet, int len)
                    SEND_PACKET_PORT(portv[i],i,packet,len,&tmpbuf);
 #endif
        } else { /* This is a switch, not a HUB! */
-           if (packet->header.proto[0] == 0x81 && packet->header.proto[1] == 0x00) {
+           if (portv[port]->vlanuntag == 0) {
+               tagged = 0;
+               vlan = 0;
+           } else if (packet->header.proto[0] == 0x81 && packet->header.proto[1] == 0x00) {
                tagged=1;
                vlan=((packet->data[0] << 8) + packet->data[1]) & 0xfff;
                if (! ba_check(vlant[vlan].table,port))
-- 
2.30.2
MagicRB commented 2 months ago

Ran into this myself, attempting to simulate PPPoE exactly the way KPN in the Netherlands does it.