From 8e69ec111fb8fd1e7fd5153cfaee3ffbef7abba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sun, 7 Mar 2021 07:06:13 -0800 Subject: [PATCH] bpf offload - make sure bpf code can access packet headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a tc ebpf program writes into a packet using direct packet access then the packet will automatically be uncloned and pulled by additional prologue inserted by the kernel itself. See tc_cls_act_prologue() & bpf_unclone_prologue() in kernel sources (this is how the clat ebpf program works, which does DPA writes). However in the forwarding programs we only *read* from the packets using direct packet access, but never write. All writes happen via kernel bpf helpers (this is mostly an implementation detail: since we need to use helpers for checksum updates, I decided to also use checksums for the writes themselves). As such the insert 'automatic unclone/pull' logic doesn't trigger. It is thus possible (it depends on the skb layout delivered by the nic driver) for 0 bytes of the packet to be accessible for read using direct packet access. We thus need to explicitly try to pull in the header of the packet so that we can inspect it. In most cases (on most drivers for most packet types) this will end up being a no-op (because the headers will already be in the linear portion of the skb). But on some drivers for some packet types it ends up mattering. Test: TreeHugger, makes icmpv6 tether forwarding work on bramble Signed-off-by: Maciej Żenczykowski Change-Id: I4b07e57728ce544ffb908527ea11ecc315e5acec --- Tethering/bpf_progs/offload.c | 40 +++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/Tethering/bpf_progs/offload.c b/Tethering/bpf_progs/offload.c index 5f29d4f60a..4254ab720c 100644 --- a/Tethering/bpf_progs/offload.c +++ b/Tethering/bpf_progs/offload.c @@ -107,17 +107,24 @@ DEFINE_BPF_MAP_GRW(tether_upstream6_map, HASH, TetherUpstream6Key, Tether6Value, static inline __always_inline int do_forward6(struct __sk_buff* skb, const bool is_ethernet, const bool downstream) { - const int l2_header_size = is_ethernet ? sizeof(struct ethhdr) : 0; - void* data = (void*)(long)skb->data; - const void* data_end = (void*)(long)skb->data_end; - struct ethhdr* eth = is_ethernet ? data : NULL; // used iff is_ethernet - struct ipv6hdr* ip6 = is_ethernet ? (void*)(eth + 1) : data; + // Must be meta-ethernet IPv6 frame + if (skb->protocol != htons(ETH_P_IPV6)) return TC_ACT_OK; // Require ethernet dst mac address to be our unicast address. if (is_ethernet && (skb->pkt_type != PACKET_HOST)) return TC_ACT_OK; - // Must be meta-ethernet IPv6 frame - if (skb->protocol != htons(ETH_P_IPV6)) return TC_ACT_OK; + const int l2_header_size = is_ethernet ? sizeof(struct ethhdr) : 0; + + // Since the program never writes via DPA (direct packet access) auto-pull/unclone logic does + // not trigger and thus we need to manually make sure we can read packet headers via DPA. + // Note: this is a blind best effort pull, which may fail or pull less - this doesn't matter. + // It has to be done early cause it will invalidate any skb->data/data_end derived pointers. + try_make_readable(skb, l2_header_size + IP6_HLEN + TCP_HLEN); + + void* data = (void*)(long)skb->data; + const void* data_end = (void*)(long)skb->data_end; + struct ethhdr* eth = is_ethernet ? data : NULL; // used iff is_ethernet + struct ipv6hdr* ip6 = is_ethernet ? (void*)(eth + 1) : data; // Must have (ethernet and) ipv6 header if (data + l2_header_size + sizeof(*ip6) > data_end) return TC_ACT_OK; @@ -346,18 +353,25 @@ DEFINE_BPF_MAP_GRW(tether_upstream4_map, HASH, Tether4Key, Tether4Value, 1024, A static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool is_ethernet, const bool downstream, const bool updatetime) { - const int l2_header_size = is_ethernet ? sizeof(struct ethhdr) : 0; - void* data = (void*)(long)skb->data; - const void* data_end = (void*)(long)skb->data_end; - struct ethhdr* eth = is_ethernet ? data : NULL; // used iff is_ethernet - struct iphdr* ip = is_ethernet ? (void*)(eth + 1) : data; - // Require ethernet dst mac address to be our unicast address. if (is_ethernet && (skb->pkt_type != PACKET_HOST)) return TC_ACT_OK; // Must be meta-ethernet IPv4 frame if (skb->protocol != htons(ETH_P_IP)) return TC_ACT_OK; + const int l2_header_size = is_ethernet ? sizeof(struct ethhdr) : 0; + + // Since the program never writes via DPA (direct packet access) auto-pull/unclone logic does + // not trigger and thus we need to manually make sure we can read packet headers via DPA. + // Note: this is a blind best effort pull, which may fail or pull less - this doesn't matter. + // It has to be done early cause it will invalidate any skb->data/data_end derived pointers. + try_make_readable(skb, l2_header_size + IP4_HLEN + TCP_HLEN); + + void* data = (void*)(long)skb->data; + const void* data_end = (void*)(long)skb->data_end; + struct ethhdr* eth = is_ethernet ? data : NULL; // used iff is_ethernet + struct iphdr* ip = is_ethernet ? (void*)(eth + 1) : data; + // Must have (ethernet and) ipv4 header if (data + l2_header_size + sizeof(*ip) > data_end) return TC_ACT_OK;