From ec2be4e7f2b865c4b24beabdf60412dfbf29753f Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 3 Sep 2019 13:54:38 +0200 Subject: [PATCH 1/5] common: parsing_headers.h reduce default VLAN depth to 2 Given XDP is very performance focused, it is unwise to default parse many VLAN layers. Those 2 as it is very uncommon to have more. Signed-off-by: Jesper Dangaard Brouer --- common/parsing_helpers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/parsing_helpers.h b/common/parsing_helpers.h index c129ea5c..85734355 100644 --- a/common/parsing_helpers.h +++ b/common/parsing_helpers.h @@ -56,7 +56,7 @@ struct icmphdr_common { /* Allow users of header file to redefine VLAN max depth */ #ifndef VLAN_MAX_DEPTH -#define VLAN_MAX_DEPTH 4 +#define VLAN_MAX_DEPTH 2 #endif static __always_inline int proto_is_vlan(__u16 h_proto) From ffba6492e0a9c3721c3a2a0aee448ab2a7171cd0 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 3 Sep 2019 14:01:19 +0200 Subject: [PATCH 2/5] packet-solutions: xdp_vlan02_kern VLAN extraction missed bpf_ntohs() Signed-off-by: Jesper Dangaard Brouer --- packet-solutions/xdp_vlan02_kern.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packet-solutions/xdp_vlan02_kern.c b/packet-solutions/xdp_vlan02_kern.c index b354e50c..9ce09542 100644 --- a/packet-solutions/xdp_vlan02_kern.c +++ b/packet-solutions/xdp_vlan02_kern.c @@ -51,7 +51,8 @@ static __always_inline int __parse_ethhdr_vlan(struct hdr_cursor *nh, h_proto = vlh->h_vlan_encapsulated_proto; if (vlans) { - vlans->id[i] = vlh->h_vlan_TCI & VLAN_VID_MASK; + vlans->id[i] = + bpf_ntohs(vlh->h_vlan_TCI) & VLAN_VID_MASK; } vlh++; } From fb079b1ca71e9ad674491ef23cd4c75b025a4e3c Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 3 Sep 2019 16:47:21 +0200 Subject: [PATCH 3/5] Create parse_ethhdr_vlan() in parsing_helpers.h This is based on same function from packet-solutions/xdp_vlan02_kern.c that also extracts VLANs. It needs to be removed from packet-solutions/xdp_vlan02_kern.c, but for now its just ifdef defined out-of-code. Need to run some tests and look at BPF byte-code to make sure it does the right thing. The original parse_ethhdr() is implemented by calling parse_ethhdr_vlan() with NULL argument for stucture to collect IDs into. The compiler should remove the parts that are not needed. Signed-off-by: Jesper Dangaard Brouer --- common/parsing_helpers.h | 23 +++++++++++++++++++++-- packet-solutions/xdp_vlan02_kern.c | 6 +++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/common/parsing_helpers.h b/common/parsing_helpers.h index 85734355..6647b552 100644 --- a/common/parsing_helpers.h +++ b/common/parsing_helpers.h @@ -59,6 +59,11 @@ struct icmphdr_common { #define VLAN_MAX_DEPTH 2 #endif +#define VLAN_VID_MASK 0x0fff /* VLAN Identifier */ +struct vlans { + __u16 id[VLAN_MAX_DEPTH]; +}; + static __always_inline int proto_is_vlan(__u16 h_proto) { return !!(h_proto == bpf_htons(ETH_P_8021Q) || @@ -70,8 +75,10 @@ static __always_inline int proto_is_vlan(__u16 h_proto) * Ethernet header. Thus, caller can look at eth->h_proto to see if this was a * VLAN tagged packet. */ -static __always_inline int parse_ethhdr(struct hdr_cursor *nh, void *data_end, - struct ethhdr **ethhdr) +static __always_inline int parse_ethhdr_vlan(struct hdr_cursor *nh, + void *data_end, + struct ethhdr **ethhdr, + struct vlans *vlans) { struct ethhdr *eth = nh->pos; int hdrsize = sizeof(*eth); @@ -102,6 +109,10 @@ static __always_inline int parse_ethhdr(struct hdr_cursor *nh, void *data_end, break; h_proto = vlh->h_vlan_encapsulated_proto; + if (vlans) /* collect VLAN ids */ + vlans->id[i] = + (bpf_ntohs(vlh->h_vlan_TCI) & VLAN_VID_MASK); + vlh++; } @@ -109,6 +120,14 @@ static __always_inline int parse_ethhdr(struct hdr_cursor *nh, void *data_end, return h_proto; /* network-byte-order */ } +static __always_inline int parse_ethhdr(struct hdr_cursor *nh, + void *data_end, + struct ethhdr **ethhdr) +{ + /* Expect compiler to remove collect VLAN ids */ + return parse_ethhdr_vlan(nh, data_end, ethhdr, NULL); +} + static __always_inline int parse_ip6hdr(struct hdr_cursor *nh, void *data_end, struct ipv6hdr **ip6hdr) diff --git a/packet-solutions/xdp_vlan02_kern.c b/packet-solutions/xdp_vlan02_kern.c index 9ce09542..78292b23 100644 --- a/packet-solutions/xdp_vlan02_kern.c +++ b/packet-solutions/xdp_vlan02_kern.c @@ -10,11 +10,14 @@ #define VLAN_MAX_DEPTH 10 #include "../common/parsing_helpers.h" +#if 0 #define VLAN_VID_MASK 0x0fff /* VLAN Identifier */ struct vlans { __u16 id[VLAN_MAX_DEPTH]; }; +#endif +#if 0 /* moved to parsing_helpers.h */ /* Based on parse_ethhdr() */ static __always_inline int __parse_ethhdr_vlan(struct hdr_cursor *nh, void *data_end, @@ -60,6 +63,7 @@ static __always_inline int __parse_ethhdr_vlan(struct hdr_cursor *nh, nh->pos = vlh; return h_proto; /* network-byte-order */ } +#endif SEC("xdp_vlan02") int xdp_vlan_02(struct xdp_md *ctx) @@ -75,7 +79,7 @@ int xdp_vlan_02(struct xdp_md *ctx) struct vlans vlans; struct ethhdr *eth; - eth_type = __parse_ethhdr_vlan(&nh, data_end, ð, &vlans); + eth_type = parse_ethhdr_vlan(&nh, data_end, ð, &vlans); if (eth_type < 0) return XDP_ABORTED; From 61702d6e32e68c87f4664c44e0605cd3f8a98c8f Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Sun, 15 Nov 2020 13:14:03 +0100 Subject: [PATCH 4/5] packet-solutions: Change struct name and improve some comments Signed-off-by: Jesper Dangaard Brouer --- common/parsing_helpers.h | 7 ++++--- packet-solutions/xdp_vlan02_kern.c | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/common/parsing_helpers.h b/common/parsing_helpers.h index 6647b552..f889fb36 100644 --- a/common/parsing_helpers.h +++ b/common/parsing_helpers.h @@ -60,7 +60,8 @@ struct icmphdr_common { #endif #define VLAN_VID_MASK 0x0fff /* VLAN Identifier */ -struct vlans { +/* Struct for collecting VLANs after parsing via parse_ethhdr_vlan */ +struct collect_vlans { __u16 id[VLAN_MAX_DEPTH]; }; @@ -78,7 +79,7 @@ static __always_inline int proto_is_vlan(__u16 h_proto) static __always_inline int parse_ethhdr_vlan(struct hdr_cursor *nh, void *data_end, struct ethhdr **ethhdr, - struct vlans *vlans) + struct collect_vlans *vlans) { struct ethhdr *eth = nh->pos; int hdrsize = sizeof(*eth); @@ -124,7 +125,7 @@ static __always_inline int parse_ethhdr(struct hdr_cursor *nh, void *data_end, struct ethhdr **ethhdr) { - /* Expect compiler to remove collect VLAN ids */ + /* Expect compiler removes the code that collects VLAN ids */ return parse_ethhdr_vlan(nh, data_end, ethhdr, NULL); } diff --git a/packet-solutions/xdp_vlan02_kern.c b/packet-solutions/xdp_vlan02_kern.c index 78292b23..f935ff85 100644 --- a/packet-solutions/xdp_vlan02_kern.c +++ b/packet-solutions/xdp_vlan02_kern.c @@ -12,7 +12,7 @@ #if 0 #define VLAN_VID_MASK 0x0fff /* VLAN Identifier */ -struct vlans { +struct collect_vlans { __u16 id[VLAN_MAX_DEPTH]; }; #endif @@ -22,7 +22,7 @@ struct vlans { static __always_inline int __parse_ethhdr_vlan(struct hdr_cursor *nh, void *data_end, struct ethhdr **ethhdr, - struct vlans *vlans) + struct collect_vlans *vlans) { struct ethhdr *eth = nh->pos; int hdrsize = sizeof(*eth); @@ -76,7 +76,7 @@ int xdp_vlan_02(struct xdp_md *ctx) int eth_type; nh.pos = data; - struct vlans vlans; + struct collect_vlans vlans; struct ethhdr *eth; eth_type = parse_ethhdr_vlan(&nh, data_end, ð, &vlans); From 2bb3dbccfed41438ba36f9038abafefb3173b3f0 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Sun, 15 Nov 2020 13:52:30 +0100 Subject: [PATCH 5/5] packet-solutions: Explain value of eth_type and where eth points Signed-off-by: Jesper Dangaard Brouer --- packet-solutions/xdp_vlan02_kern.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packet-solutions/xdp_vlan02_kern.c b/packet-solutions/xdp_vlan02_kern.c index f935ff85..ac7211c9 100644 --- a/packet-solutions/xdp_vlan02_kern.c +++ b/packet-solutions/xdp_vlan02_kern.c @@ -79,10 +79,14 @@ int xdp_vlan_02(struct xdp_md *ctx) struct collect_vlans vlans; struct ethhdr *eth; - eth_type = parse_ethhdr_vlan(&nh, data_end, ð, &vlans); + eth_type = parse_ethhdr_vlan(&nh, data_end, ð, &vlans); if (eth_type < 0) return XDP_ABORTED; + /* The eth_type have skipped VLAN-types, but collected VLAN ids. The + * eth ptr still points to Ethernet header, thus to check if this is a + * VLAN packet do proto_is_vlan(eth->h_proto). + */ /* The LLVM compiler is very clever, it sees that program only access * 2nd "inner" vlan (array index 1), and only does loop unroll of 2, and @@ -117,7 +121,7 @@ int xdp_vlan_02(struct xdp_md *ctx) } #endif /* Hint: to inspect BPF byte-code run: - * llvm-objdump -S xdp_vlan02_kern.o + * llvm-objdump --no-show-raw-insn -S xdp_vlan02_kern.o */ return XDP_PASS; }