From bdccc50824a1e7a36cd48bd5f00ca3ca8ab776ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Tue, 18 Apr 2023 06:45:35 +0000 Subject: [PATCH] allow ingress TCP FINs in doze mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I don't know if this will truly help: We'll still drop the expected egress TCP ACK (or FIN-ACK) reply to the newly allowed ingress TCP FIN... However: I don't think this will make things worse. The presence of an ingress packet is proof the hardware already woke up to receive it. This behaviour doesn't change when allowing ingress *anything*. ie. the main reason we don't allow ingress packets is that it would be illogical to be asymmetrical. So even if we do immediately send back a reply (I think a RST is the only real possibility at the moment, since ACK would still be dropped). Worst case we're waking the hardware up from RX processing to full blown TX processing. Furthermore if an inbound FIN causes an outbound RST, then that RST will most likely prevent receiving future FIN retransmits. So we're trading an RX->TX hardware wake up now, for less RX wakeups in the (near) future. This *might* just be an overall win. I think a true solution likely needs to be smarter still and allow skb->sk state != BPF_TCP_ESTABLISHED (or something) Bug: 259199087 Bug: 264903985 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I143f12342f72d89f9450560c8d60dad4c79ffe64 --- bpf_progs/netd.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bpf_progs/netd.c b/bpf_progs/netd.c index d98fa5f987..ce3315b04d 100644 --- a/bpf_progs/netd.c +++ b/bpf_progs/netd.c @@ -300,7 +300,8 @@ static __always_inline inline void do_packet_tracing( bpf_packet_trace_ringbuf_submit(pkt); } -static __always_inline inline bool skip_owner_match(struct __sk_buff* skb, const unsigned kver) { +static __always_inline inline bool skip_owner_match(struct __sk_buff* skb, bool egress, + const unsigned kver) { uint32_t flag = 0; if (skb->protocol == htons(ETH_P_IP)) { uint8_t proto; @@ -330,7 +331,8 @@ static __always_inline inline bool skip_owner_match(struct __sk_buff* skb, const } else { return false; } - return flag & TCP_FLAG_RST; // false on read failure + // Always allow RST's, and additionally allow ingress FINs + return flag & (TCP_FLAG_RST | (egress ? 0 : TCP_FLAG_FIN)); // false on read failure } static __always_inline inline BpfConfig getConfig(uint32_t configKey) { @@ -352,7 +354,7 @@ static __always_inline inline int bpf_owner_match(struct __sk_buff* skb, uint32_ bool egress, const unsigned kver) { if (is_system_uid(uid)) return PASS; - if (skip_owner_match(skb, kver)) return PASS; + if (skip_owner_match(skb, egress, kver)) return PASS; BpfConfig enabledRules = getConfig(UID_RULES_CONFIGURATION_KEY);