retis-org · atenart · Feb 6, 2025 · Jan 27, 2025 · Jan 27, 2025 · Feb 6, 2025
diff --git a/Vagrantfile b/Vagrantfile
@@ -19,11 +19,20 @@ dnf install -y \
     socat \
     nftables \
     make \
-    jq
+    jq \
+    ethtool
 
     python3 -m pip install pytest pyroute2
 SCRIPT
 
+# CentOS mirror URL changed but the c8s image is no longer being built. We
+# have to fix them manually in order to install packages later.
+$fix_centos_repositories = <<SCRIPT
+sed -i s/mirror.centos.org/vault.centos.org/g /etc/yum.repos.d/*.repo
+sed -i s/^#.*baseurl=http/baseurl=http/g /etc/yum.repos.d/*.repo
+sed -i s/^mirrorlist=http/#mirrorlist=http/g /etc/yum.repos.d/*.repo
+SCRIPT
+
 def get_box(url, pattern)
   require 'open-uri'
   require 'nokogiri'
@@ -64,20 +73,14 @@ Vagrant.configure("2") do |config|
     centos.vm.box = "centos-8-stream"
     centos.vm.box_url = get_box("https://cloud.centos.org/centos/8-stream/x86_64/images/", /.*latest\.x86_64\.vagrant-libvirt\.box$/)
 
-    # CentOS mirror URL changed but the c8s image is no longer being built. We
-    # have to fix them manually in order to install packages later.
-    centos.vm.provision "repos-fixup", type: "shell", inline: <<-SHELL
-       sed -i s/mirror.centos.org/vault.centos.org/g /etc/yum.repos.d/*.repo
-       sed -i s/^#.*baseurl=http/baseurl=http/g /etc/yum.repos.d/*.repo
-       sed -i s/^mirrorlist=http/#mirrorlist=http/g /etc/yum.repos.d/*.repo
-    SHELL
-
     centos.vm.provision "shell", inline: <<-SHELL
+       #{$fix_centos_repositories}!
        dnf config-manager --set-enabled powertools
     SHELL
     centos.vm.provision "common", type: "shell", inline: $bootstrap_rhel_common
     centos.vm.provision "shell", inline: <<-SHELL
        dnf install -y centos-release-nfv-openvswitch
+       #{$fix_centos_repositories}!
        dnf install -y openvswitch3.1
     SHELL
 

diff --git a/retis/src/module/skb/bpf/include/if_vlan.h b/retis/src/module/skb/bpf/include/if_vlan.h
@@ -1,7 +1,8 @@
 #ifndef __MODULE_SKB_BPF_IF_VLAN__
 #define __MODULE_SKB_BPF_IF_VLAN__
 
-/* Code to handle VLAN tags ported from source/include/linux/if_vlan.h and modified to handle BPF */
+/* Code to handle VLAN tags ported from source/include/linux/if_vlan.h and
+   modified to handle BPF */
 
 #include <vmlinux.h>
 #include <bpf/bpf_core_read.h>
@@ -14,6 +15,10 @@
 #define ETH_P_8021Q	0x8100
 #define ETH_P_8021AD	0x88A8
 
+// Used for kernels prior to 0c4b2d370514cb4f3454dd3b18f031d2651fab73.
+#define VLAN_CFI_MASK		0x1000 /* Canonical Format Indicator */
+#define VLAN_TAG_PRESENT	VLAN_CFI_MASK
+
 #define set_skb_vlan_event(e, vlan_tci, vlan_accel)	{ \
 	e->pcp = (vlan_tci & 0xe000) >> 13; \
 	e->dei = (vlan_tci & 0x1000) >> 12; \
@@ -28,21 +33,56 @@ struct skb_vlan_event {
 	u8 acceleration;
 } __binding;
 
+/**
+ * vlan_tag_present tries to determine if a VLAN tag is present. There have been
+ * various changes over the years in the kernel: The kernel uses
+ * skb_vlan_tag_present which in more recent versions either relies on
+ * vlan_present or on vlan_all.
+ * We use CO-RE functionality to probe if either field is present and return
+ * false otherwise.
+ * See kernel commit 354259fa73e2
+ *
+ * @param skb
+ *
+ * Returns true if a VLAN tag is present in vlan_present or vlan_all
+ */
 static __always_inline bool vlan_tag_present(const struct sk_buff *skb)
 {
-       struct sk_buff___6_1_0 *skb_61 = (struct sk_buff___6_1_0 *)skb;
+	struct sk_buff___6_1_0 *skb_61 = (struct sk_buff___6_1_0 *)skb;
+
+	// Older kernerls, e.g. RHEL 9.
+	if (bpf_core_field_exists(skb_61->vlan_present))
+		return BPF_CORE_READ_BITFIELD_PROBED(skb_61, vlan_present);
+
+	// New kernels, e.g. Fedora 40 and later.
+	if (bpf_core_field_exists(skb->vlan_all))
+		return BPF_CORE_READ(skb, vlan_all);
 
-       if (bpf_core_field_exists(skb_61->vlan_present))
-               return BPF_CORE_READ_BITFIELD_PROBED(skb_61, vlan_present);
+	return false;
+}
 
-       return BPF_CORE_READ(skb, vlan_all);
+/**
+ * vlan_tag_present_old implements logic from before kernel commit 0c4b2d370514
+ * to determine if a VLAN tag is present. In old kernel versions,
+ * __vlan_hwaccel_put_tag set skb->vlan_tci = VLAN_TAG_PRESENT | vlan_tci.
+ * Therefore, old versions of the kernel use the DEI / CFI bit indicate / detect
+ * presence of the VLAN tag.
+ *
+ * @param skb
+ *
+ * Returns true if a VLAN tag is present
+ */
+static __always_inline bool vlan_tag_present_old(const struct sk_buff *skb) {
+	return (BPF_CORE_READ(skb, vlan_tci) & VLAN_TAG_PRESENT);
 }
 
 /**
- * __vlan_hwaccel_get_tag - get the VLAN ID that is in @skb->cb[]
- * The kernel uses skb_vlan_tag_present which either relies on vlan_present or
- * on vlan_all depending on the kernel version (see commit 354259fa73e2aac92ae5e19522adb69a92c15b49).
- * We use CO-RE functionality to probe either field in vlan_tag_present.
+ * __vlan_hwaccel_get_tag - get the VLAN TCI that is in @skb->cb[]. In newer
+ * versions of the kernel, report the vlan_tci as is. Versions of the kernel
+ * before commit 0c4b2d370514 always set the DEI / CFI bit to on. As it is more
+ * common to have this bit off, and in order to not report a false positive, set
+ * the bit to 0 for these old kernels.
+ *
  * @skb: skbuff to query
  * @vlan_tci: buffer to store value
  *
@@ -54,6 +94,9 @@ static inline int __vlan_hwaccel_get_tag(const struct sk_buff *skb,
 	if (vlan_tag_present(skb)) {
 		*vlan_tci = BPF_CORE_READ(skb, vlan_tci);
 		return 0;
+	} else if (vlan_tag_present_old(skb)) {
+		*vlan_tci = BPF_CORE_READ(skb, vlan_tci) & 0xefff;
+		return 0;
 	} else {
 		*vlan_tci = 0;
 		return -ENODATA;
@@ -110,4 +153,4 @@ static inline int __vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
 	return 0;
 }
 
-#endif /* __MODULE_SKB_BPF_IF_VLAN__ */
+#endif /* __MODULE_SKB_BPF_IF_VLAN__ */
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -83,6 +83,94 @@ def two_ns_simple(netns):
     yield netns
 
 
+@pytest.fixture
+def two_ns_vlan(two_ns_simple):
+    """Fixture that creates two netns connected through VLAN interfaces on top of a
+    veth pair."""
+    netns = two_ns_simple
+
+    # Get netns which was previously created
+    ns0 = netns.get("ns0")
+    ns1 = netns.get("ns1")
+
+    # Create VLANs
+    ns0.link(
+        "add",
+        ifname="veth01.123",
+        link=ns0.link_lookup(ifname="veth01")[0],
+        kind="vlan",
+        vlan_id=123,
+    )
+    ns1.link(
+        "add",
+        ifname="veth10.123",
+        link=ns1.link_lookup(ifname="veth10")[0],
+        kind="vlan",
+        vlan_id=123,
+    )
+
+    # Set VLANs up
+    ns0.link("set", ifname="veth01.123", state="up")
+    ns1.link("set", ifname="veth10.123", state="up")
+
+    # Assign IP addresses to VLANs
+    ns0.addr(
+        "add",
+        index=ns0.link_lookup(ifname="veth01.123")[0],
+        address="10.0.43.1",
+        prefixlen=24,
+    )
+    ns1.addr(
+        "add",
+        index=ns1.link_lookup(ifname="veth10.123")[0],
+        address="10.0.43.2",
+        prefixlen=24,
+    )
+
+    netns.run(
+        "ns0",
+        "ip",
+        "link",
+        "set",
+        "veth01.123",
+        "type",
+        "vlan",
+        "egress",
+        "0:0",
+        "1:1",
+        "2:2",
+        "3:3",
+        "4:4",
+        "5:5",
+        "6:6",
+        "7:7",
+    )
+    netns.run(
+        "ns0",
+        "iptables",
+        "-t",
+        "mangle",
+        "-I",
+        "OUTPUT",
+        "-o",
+        "veth01.123",
+        "-j",
+        "CLASSIFY",
+        "--set-class",
+        "0:6",
+    )
+    netns.run(
+        "ns1",
+        "ethtool",
+        "-K",
+        "veth10",
+        "tx-vlan-offload",
+        "off",
+    )
+
+    yield netns
+
+
 @pytest.fixture
 def three_ns_nat(netns):
     """Fixture that creates three netns connected through veth pairs, a VIP and DNATing

diff --git a/tests/test_skb.py b/tests/test_skb.py
@@ -304,3 +304,150 @@ def test_skb_tcp_cc(two_ns_simple):
     ]
     events = retis.events()
     assert_events_present(events, expected_events)
+
+
+def test_skb_vlan(two_ns_vlan):
+    ns = two_ns_vlan
+    retis = Retis()
+
+    retis.collect(
+        "-c",
+        "skb",
+        "--skb-sections",
+        "arp,dev,eth,vlan",
+        "-f",
+        "tcp port 80 or arp",
+        "-p",
+        "net:net_dev_start_xmit",
+    )
+    print(ns.run_bg("ns1", "socat", "TCP-LISTEN:80", "STDOUT"))
+    print(ns.run("ns0", "socat", "-T", "1", "-", "TCP:10.0.43.2:80"))
+    retis.stop()
+
+    # A known limitation for now in Retis: it does not support VLAN nor tunnels
+    # for parsing the inner payload right at the moment. Therefore, for the
+    # return packets which are not offloaded, do not verify ARP or TCP.
+    expected_events = [
+        # ARP req
+        {
+            "common": {
+                "task": {
+                    "comm": "socat",
+                },
+            },
+            "kernel": {
+                "probe_type": "raw_tracepoint",
+                "symbol": "net:net_dev_start_xmit",
+            },
+            "skb": {
+                "arp": {
+                    "operation": "Request",
+                    "spa": "10.0.43.1",
+                    "tpa": "10.0.43.2",
+                },
+                "dev": {
+                    "name": "veth01",
+                },
+                "eth": {
+                    "etype": 0x806,  # ARP
+                },
+                "vlan": {
+                    "acceleration": True,
+                    "dei": False,
+                    "pcp": 0,
+                    "vid": 123,
+                },
+            },
+        },
+        # ARP rep
+        {
+            "common": {
+                "task": {
+                    "comm": "socat",
+                },
+            },
+            "kernel": {
+                "probe_type": "raw_tracepoint",
+                "symbol": "net:net_dev_start_xmit",
+            },
+            "skb": {
+                "dev": {
+                    "name": "veth10",
+                },
+                "eth": {
+                    "etype": 0x8100,  # 802.1Q
+                },
+                "vlan": {
+                    "acceleration": False,
+                    "dei": False,
+                    "pcp": 0,
+                    "vid": 123,
+                },
+            },
+        },
+        # SYN
+        {
+            "common": {
+                "task": {
+                    "comm": "socat",
+                },
+            },
+            "kernel": {
+                "probe_type": "raw_tracepoint",
+                "symbol": "net:net_dev_start_xmit",
+            },
+            "skb": {
+                "dev": {
+                    "name": "veth01",
+                },
+                "eth": {
+                    "etype": 0x800,  # IP
+                },
+                "vlan": {
+                    "acceleration": True,
+                    "dei": False,
+                    "pcp": 6,
+                    "vid": 123,
+                },
+                "ip": {
+                    "daddr": "10.0.43.2",
+                    "ecn": 0,
+                    "protocol": 6,
+                    "saddr": "10.0.43.1",
+                    "ttl": 64,
+                },
+                "tcp": {
+                    "dport": 80,
+                    "flags": 2,
+                },
+            },
+        },
+        # SYN,ACK
+        {
+            "common": {
+                "task": {
+                    "comm": "socat",
+                },
+            },
+            "kernel": {
+                "probe_type": "raw_tracepoint",
+                "symbol": "net:net_dev_start_xmit",
+            },
+            "skb": {
+                "dev": {
+                    "name": "veth10",
+                },
+                "eth": {
+                    "etype": 0x8100,  # 802.1Q
+                },
+                "vlan": {
+                    "acceleration": False,
+                    "dei": False,
+                    "pcp": 0,
+                    "vid": 123,
+                },
+            },
+        },
+    ]
+    events = retis.events()
+    assert_events_present(events, expected_events)