collector/event: skb: only report VLAN accel data

The outer VLAN header can be "accelerated" in Linux, aka. part of the skb metadata and not the payload. Only report those as part of the skb event as for all other cases the VLAN information will be part of the payload and displayed correctly. Signed-off-by: Antoine Tenart <[email protected]>
retis-org · Jan 29, 2025 · be9cf3f · be9cf3f
1 parent 6942d1b
commit be9cf3f
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 84 deletions.
diff --git a/docs/collectors/skb.md b/docs/collectors/skb.md
@@ -45,10 +45,10 @@ if {interface index} ({interface name}) rxif {rx interface index}
 {src mac} > {dst mac} ethertype {etype name} ({etype hex})
 ```
 
-### VLAN section
+### VLAN acceleration section
 
 ```none
-vlan (id {id} prio {prio} [drop] [accel])
+vlan_accel (id {id} prio {prio} [drop])
 ```
 
 ### ARP section

diff --git a/retis-events/src/skb.rs b/retis-events/src/skb.rs
@@ -116,8 +116,8 @@ impl Tcpdump {
 #[event_section(SectionId::Skb)]
 #[derive(Default)]
 pub struct SkbEvent {
-    /// VLAN tag fields, if any.
-    pub vlan: Option<SkbVlanEvent>,
+    /// VLAN acceleration tag fields, if any.
+    pub vlan_accel: Option<SkbVlanAccelEvent>,
     /// Net device data, if any.
     pub dev: Option<SkbDevEvent>,
     /// Net namespace data, if any.
@@ -169,15 +169,15 @@ impl EventFmt for SkbEvent {
         // otherwise we would print this one but not the VLAN data in the
         // payload. This would be quite confusing.
         if format.print_ll {
-            if let Some(vlan) = &self.vlan {
+            if let Some(vlan) = &self.vlan_accel {
                 space.write(f)?;
 
-                let drop = if vlan.dei { " drop" } else { "" };
-                let accel = if vlan.acceleration { " accel" } else { "" };
                 write!(
                     f,
-                    "vlan (id {} prio {}{}{})",
-                    vlan.vid, vlan.pcp, drop, accel
+                    "vlan_accel (id {} prio {}{})",
+                    vlan.vid,
+                    vlan.pcp,
+                    if vlan.dei { " drop" } else { "" }
                 )?;
             }
         }
@@ -290,17 +290,15 @@ pub struct SkbEthEvent {
     pub dst: String,
 }
 
-/// VLAN fields.
+/// VLAN acceleration fields.
 #[event_type]
-pub struct SkbVlanEvent {
+pub struct SkbVlanAccelEvent {
     /// Priority Code Point, also called CoS.
     pub pcp: u8,
     /// Drop eligible indicator.
     pub dei: bool,
     /// VLAN ID.
     pub vid: u16,
-    /// VLAN acceleration field.
-    pub acceleration: bool,
 }
 
 /// ARP fields.

diff --git a/retis/src/bindings/if_vlan_uapi.rs b/retis/src/bindings/if_vlan_uapi.rs
@@ -10,5 +10,4 @@ pub struct skb_vlan_event {
     pub pcp: u8_,
     pub dei: u8_,
     pub vid: u16_,
-    pub acceleration: u8_,
 }
diff --git a/retis/src/module/skb/bpf.rs b/retis/src/module/skb/bpf.rs
@@ -66,14 +66,13 @@ pub(super) fn unmarshal_meta(raw_section: &BpfRawSection) -> Result<SkbMetaEvent
     })
 }
 
-pub(super) fn unmarshal_vlan(raw_section: &BpfRawSection) -> Result<SkbVlanEvent> {
+pub(super) fn unmarshal_vlan(raw_section: &BpfRawSection) -> Result<SkbVlanAccelEvent> {
     let raw = parse_raw_section::<skb_vlan_event>(raw_section)?;
 
-    Ok(SkbVlanEvent {
+    Ok(SkbVlanAccelEvent {
         pcp: raw.pcp,
         dei: raw.dei == 1,
         vid: raw.vid,
-        acceleration: raw.acceleration == 1,
     })
 }
 
@@ -121,7 +120,7 @@ impl RawEventSectionFactory for SkbEventFactory {
 
         for section in raw_sections.iter() {
             match section.header.data_type as u32 {
-                SECTION_VLAN => event.vlan = Some(unmarshal_vlan(section)?),
+                SECTION_VLAN => event.vlan_accel = Some(unmarshal_vlan(section)?),
                 SECTION_DEV => event.dev = unmarshal_dev(section)?,
                 SECTION_NS => event.ns = Some(unmarshal_ns(section)?),
                 SECTION_META => event.meta = Some(unmarshal_meta(section)?),

diff --git a/retis/src/module/skb/bpf/include/if_vlan.h b/retis/src/module/skb/bpf/include/if_vlan.h
@@ -11,21 +11,17 @@
 #include <compat.h>
 
 #define	ENODATA		61
-#define ETH_P_8021Q	0x8100
-#define ETH_P_8021AD	0x88A8
 
-#define set_skb_vlan_event(e, vlan_tci, vlan_accel)	{ \
+#define set_skb_vlan_event(e, vlan_tci)	{ \
 	e->pcp = (vlan_tci & 0xe000) >> 13; \
 	e->dei = (vlan_tci & 0x1000) >> 12; \
 	e->vid = vlan_tci & 0x0fff; \
-	e->acceleration = vlan_accel; \
 }
 
 struct skb_vlan_event {
 	u8 pcp;
 	u8 dei;
 	u16 vid;
-	u8 acceleration;
 } __binding;
 
 static __always_inline bool vlan_tag_present(const struct sk_buff *skb)
@@ -60,54 +56,4 @@ static inline int __vlan_hwaccel_get_tag(const struct sk_buff *skb,
 	}
 }
 
-/**
- * eth_type_vlan - check for valid vlan ether type.
- * @ethertype: ether type to check
- *
- * Returns true if the ether type is a vlan ether type.
- */
-static inline bool eth_type_vlan(__be16 ethertype)
-{
-	switch (ethertype) {
-	case bpf_htons(ETH_P_8021Q):
-	case bpf_htons(ETH_P_8021AD):
-		return true;
-	default:
-		return false;
-	}
-}
-
-static inline struct vlan_ethhdr *skb_vlan_eth_hdr(const struct sk_buff *skb)
-{
-	if (is_mac_data_valid(skb)) {
-		unsigned char *head = BPF_CORE_READ(skb, head);
-		int mac = BPF_CORE_READ(skb, mac_header);
-		return (struct vlan_ethhdr *)(head + mac);
-	}
-
-	return (struct vlan_ethhdr *) BPF_CORE_READ(skb, data);
-}
-
-/**
- * __vlan_get_tag - get the VLAN ID that is part of the payload
- * @skb: skbuff to query
- * @vlan_tci: buffer to store value
- *
- * Returns error if the skb is not of VLAN type
- */
-static inline int __vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
-{
-	struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb);
-	u16 h_vlan_proto;
-	u16 h_vlan_TCI;
-
-	bpf_probe_read_kernel(&h_vlan_proto, sizeof(__be16), &veth->h_vlan_proto);
-	if (!eth_type_vlan(h_vlan_proto))
-		return -ENODATA;
-
-	bpf_probe_read_kernel(&h_vlan_TCI, sizeof(__be16), &veth->h_vlan_TCI);
-	*vlan_tci = bpf_ntohs(h_vlan_TCI);
-	return 0;
-}
-
-#endif /* __MODULE_SKB_BPF_IF_VLAN__ */
+#endif /* __MODULE_SKB_BPF_IF_VLAN__ */
diff --git a/retis/src/module/skb/bpf/skb_hook.bpf.c b/retis/src/module/skb/bpf/skb_hook.bpf.c
@@ -304,25 +304,16 @@ static __always_inline int process_skb(struct retis_raw_event *event,
 	}
 
 	if (cfg->sections & BIT(SECTION_VLAN)) {
-		bool is_vlan = false;
-		bool is_accel = false;
 		u16 vlan_tci;
 
 		if (!__vlan_hwaccel_get_tag(skb, &vlan_tci)) {
-			is_vlan = true;
-			is_accel = true;
-		} else if (!__vlan_get_tag(skb, &vlan_tci)) {
-			is_vlan = true;
-		}
-
-		if(is_vlan) {
 			struct skb_vlan_event *e =
 				get_event_section(event, COLLECTOR_SKB,
 						  SECTION_VLAN, sizeof(*e));
 			if (!e)
 				return 0;
 
-			set_skb_vlan_event(e, vlan_tci, is_accel);
+			set_skb_vlan_event(e, vlan_tci);
 		}
 	}