From patchwork Wed Dec 20 13:34:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduard Zingerman X-Patchwork-Id: 13500080 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0915636AED for ; Wed, 20 Dec 2023 13:34:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="emQfkZEA" Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-a2331e7058aso525275066b.2 for ; Wed, 20 Dec 2023 05:34:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1703079264; x=1703684064; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=eekmmOvsRvN29JgWFiEZxo+yj4o/fAvOvFvAoDaBH7Y=; b=emQfkZEAiUZzB4k0YRLiE9aeNYw9Jvg/XUMc3YIUWOtPFarO63z5s3paBf0ODKMj4C F5wwc/jP9E79JeZ5cQx54QA0Qqr/J+ssyz2eKS43aSY4lOnOQJ/oMlU8szKGeh8UWTw8 JhT2IHZx79M71BCqDgBamzK5jlAVYXAGJaHlKv0EQz4PKRBLcpqHWD/Lu9oWnStb6Jpm +7uwrnFs6hDUebjqKcJINDGQSq5nJetqLO0eJT5BcX9OAWp4QCuHeNufgtAVwwok5Ugh 9RHAFP53BYXPdWQfeW+oLhK6iNs5uwr29R+srO492mogh+6tJCIxaQfbvCi9x7wTAQyU 3szQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703079264; x=1703684064; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eekmmOvsRvN29JgWFiEZxo+yj4o/fAvOvFvAoDaBH7Y=; b=tWo0ggclty/hpEoTtvivR71wr+LrSDK4xWWkh+Nvwj0dDC5g/Qb7/BUQR05Yn3rfHi OHaW+yMqBKZT69AHKGBnu1KFTaLwqLI26akqqHAN6M8MnrSmtTNITB9Knen/d2+NAB3g 7pSy5BojgO6QHXbrd2IW7hubvghzUPIvGZcmjBS7B+SdbtR0EOw8WXvVW3fqxUbLxIp1 OCUxxBFdDxQZXQzGmmsSodTAJSrHbI7mg3x/+hV9I2o7ufMUy+5WtMkzZwZzwRmtc+KU M2Vjklp1m2vTcbbS3ZpX9ye2GX6PlFddKvUe1ZY2IuyevnyXOP8nGbJJa0h5qXlCtOv7 kv8Q== X-Gm-Message-State: AOJu0Yw/4612scYzboNLyxMY1eVok76ju+kP278TBv5kmFZWzD+63U7a 803aRsGWr5h2gg9Zq3RACvhlv9MKf+g= X-Google-Smtp-Source: AGHT+IGvlObYDaBFjkY6Vq+KXBRk3k1RkK772QkN+0MKlNEtlYY41AXDQhoEEeePRpFi+vEqPw8itw== X-Received: by 2002:a17:907:9702:b0:a26:85ec:f185 with SMTP id jg2-20020a170907970200b00a2685ecf185mr925306ejc.117.1703079263608; Wed, 20 Dec 2023 05:34:23 -0800 (PST) Received: from localhost.localdomain (host-176-36-0-241.b024.la.net.ua. [176.36.0.241]) by smtp.gmail.com with ESMTPSA id vs4-20020a170907a58400b00a22fb8901c4sm9951032ejc.12.2023.12.20.05.34.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 05:34:23 -0800 (PST) From: Eduard Zingerman To: bpf@vger.kernel.org, ast@kernel.org Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev, kernel-team@fb.com, yonghong.song@linux.dev, quentin@isovalent.com, alan.maguire@oracle.com, Eduard Zingerman Subject: [RFC v3 1/3] bpf: Mark virtual BPF context structures as preserve_static_offset Date: Wed, 20 Dec 2023 15:34:09 +0200 Message-ID: <20231220133411.22978-2-eddyz87@gmail.com> X-Mailer: git-send-email 2.42.1 In-Reply-To: <20231220133411.22978-1-eddyz87@gmail.com> References: <20231220133411.22978-1-eddyz87@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net X-Patchwork-State: RFC Add __attribute__((preserve_static_offset)) for the following BPF related structures: - __sk_buff (*) - bpf_cgroup_dev_ctx (*) - bpf_nf_ctx - bpf_perf_event_data (*) - bpf_raw_tracepoint_args - bpf_sk_lookup (*) - bpf_sock (*) - bpf_sock_addr (*) - bpf_sock_ops (*) - bpf_sockopt (*) - bpf_sysctl (*) - sk_msg_md (*) - sk_reuseport_md (*) - xdp_md (*) Access to structures marked with (*) is rewritten by BPF verifier. (See verifier.c:convert_ctx_access). The rewrite requires that offsets used in access to fields of these structures are constant values. For the rest of the structures verifier just disallows access via modified context pointer in the following code path: check_mem_access check_ptr_off_reg __check_ptr_off_reg if (!fixed_off_ok && reg->off) "dereference of modified %s ptr R%d off=%d disallowed\n" Attribute preserve_static_offset [0] is a hint to clang that ensures that constant offsets are used. Attribute __attribute__((btf_decl_tag("preserve_static_offset"))) would be used as a marker for bpftool to emit preserve_static_offset attribute when vmlinux.h is generated from BTF. Type 'pt_regs' is not handled yet. Note: !defined(__cplusplus) is necessary only to make tools/testing/selftests/bpf/test_cpp.cpp selftest compile: this test includes bpf.h and is compiled as C++. Without !defined(__cplusplus) the following error message is reported: "error: 'btf_decl_tag' attribute ignored". Apparently, when compiling C++, clang's implementation of __has_attribute returns true for attributes declared as COnly. This is possibly incorrect [1], but even if fixed in newer clang versions would be necessary for backwards compatibility. [0] https://clang.llvm.org/docs/AttributeReference.html#preserve-static-offset [1] https://discourse.llvm.org/t/when-compiling-c-has-attribute-returns-1-for-attributes-declared-as-conly-in-attr-td/ Signed-off-by: Eduard Zingerman --- include/net/netfilter/nf_bpf_link.h | 12 +++++++- include/uapi/linux/bpf.h | 34 +++++++++++++++-------- include/uapi/linux/bpf_perf_event.h | 12 +++++++- tools/include/uapi/linux/bpf.h | 34 +++++++++++++++-------- tools/include/uapi/linux/bpf_perf_event.h | 12 +++++++- 5 files changed, 77 insertions(+), 27 deletions(-) diff --git a/include/net/netfilter/nf_bpf_link.h b/include/net/netfilter/nf_bpf_link.h index 6c984b0ea838..7308efd4454f 100644 --- a/include/net/netfilter/nf_bpf_link.h +++ b/include/net/netfilter/nf_bpf_link.h @@ -1,9 +1,17 @@ /* SPDX-License-Identifier: GPL-2.0 */ +#if __has_attribute(preserve_static_offset) && defined(__bpf__) +#define __bpf_ctx __attribute__((preserve_static_offset)) +#elif __has_attribute(btf_decl_tag) && !defined(__cplusplus) +#define __bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#else +#define __bpf_ctx +#endif + struct bpf_nf_ctx { const struct nf_hook_state *state; struct sk_buff *skb; -}; +} __bpf_ctx; #if IS_ENABLED(CONFIG_NETFILTER_BPF_LINK) int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); @@ -13,3 +21,5 @@ static inline int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog return -EOPNOTSUPP; } #endif + +#undef __bpf_ctx diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e0545201b55f..351c39842b7b 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -69,6 +69,14 @@ enum { /* BPF has 10 general purpose 64-bit registers and stack frame. */ #define MAX_BPF_REG __MAX_BPF_REG +#if __has_attribute(preserve_static_offset) && defined(__bpf__) +#define __bpf_ctx __attribute__((preserve_static_offset)) +#elif __has_attribute(btf_decl_tag) && !defined(__cplusplus) +#define __bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#else +#define __bpf_ctx +#endif + struct bpf_insn { __u8 code; /* opcode */ __u8 dst_reg:4; /* dest register */ @@ -6190,7 +6198,7 @@ struct __sk_buff { __u8 tstamp_type; __u32 :24; /* Padding, future use. */ __u64 hwtstamp; -}; +} __bpf_ctx; struct bpf_tunnel_key { __u32 tunnel_id; @@ -6271,7 +6279,7 @@ struct bpf_sock { __u32 dst_ip6[4]; __u32 state; __s32 rx_queue_mapping; -}; +} __bpf_ctx; struct bpf_tcp_sock { __u32 snd_cwnd; /* Sending congestion window */ @@ -6379,7 +6387,7 @@ struct xdp_md { __u32 rx_queue_index; /* rxq->queue_index */ __u32 egress_ifindex; /* txq->dev->ifindex */ -}; +} __bpf_ctx; /* DEVMAP map-value layout * @@ -6429,7 +6437,7 @@ struct sk_msg_md { __u32 size; /* Total size of sk_msg */ __bpf_md_ptr(struct bpf_sock *, sk); /* current socket */ -}; +} __bpf_ctx; struct sk_reuseport_md { /* @@ -6468,7 +6476,7 @@ struct sk_reuseport_md { */ __bpf_md_ptr(struct bpf_sock *, sk); __bpf_md_ptr(struct bpf_sock *, migrating_sk); -}; +} __bpf_ctx; #define BPF_TAG_SIZE 8 @@ -6678,7 +6686,7 @@ struct bpf_sock_addr { * Stored in network byte order. */ __bpf_md_ptr(struct bpf_sock *, sk); -}; +} __bpf_ctx; /* User bpf_sock_ops struct to access socket values and specify request ops * and their replies. @@ -6761,7 +6769,7 @@ struct bpf_sock_ops { * been written yet. */ __u64 skb_hwtstamp; -}; +} __bpf_ctx; /* Definitions for bpf_sock_ops_cb_flags */ enum { @@ -7034,11 +7042,11 @@ struct bpf_cgroup_dev_ctx { __u32 access_type; __u32 major; __u32 minor; -}; +} __bpf_ctx; struct bpf_raw_tracepoint_args { __u64 args[0]; -}; +} __bpf_ctx; /* DIRECT: Skip the FIB rules and go to FIB table associated with device * OUTPUT: Do lookup from egress perspective; default is ingress @@ -7245,7 +7253,7 @@ struct bpf_sysctl { __u32 file_pos; /* Sysctl file position to read from, write to. * Allows 1,2,4-byte read an 4-byte write. */ -}; +} __bpf_ctx; struct bpf_sockopt { __bpf_md_ptr(struct bpf_sock *, sk); @@ -7256,7 +7264,7 @@ struct bpf_sockopt { __s32 optname; __s32 optlen; __s32 retval; -}; +} __bpf_ctx; struct bpf_pidns_info { __u32 pid; @@ -7280,7 +7288,7 @@ struct bpf_sk_lookup { __u32 local_ip6[4]; /* Network byte order */ __u32 local_port; /* Host byte order */ __u32 ingress_ifindex; /* The arriving interface. Determined by inet_iif. */ -}; +} __bpf_ctx; /* * struct btf_ptr is used for typed pointer representation; the @@ -7406,4 +7414,6 @@ struct bpf_iter_num { __u64 __opaque[1]; } __attribute__((aligned(8))); +#undef __bpf_ctx + #endif /* _UAPI__LINUX_BPF_H__ */ diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h index eb1b9d21250c..4f3f1e906f96 100644 --- a/include/uapi/linux/bpf_perf_event.h +++ b/include/uapi/linux/bpf_perf_event.h @@ -10,10 +10,20 @@ #include +#if __has_attribute(preserve_static_offset) && defined(__bpf__) +#define __bpf_ctx __attribute__((preserve_static_offset)) +#elif __has_attribute(btf_decl_tag) && !defined(__cplusplus) +#define __bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#else +#define __bpf_ctx +#endif + struct bpf_perf_event_data { bpf_user_pt_regs_t regs; __u64 sample_period; __u64 addr; -}; +} __bpf_ctx; + +#undef __bpf_ctx #endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */ diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index e0545201b55f..351c39842b7b 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -69,6 +69,14 @@ enum { /* BPF has 10 general purpose 64-bit registers and stack frame. */ #define MAX_BPF_REG __MAX_BPF_REG +#if __has_attribute(preserve_static_offset) && defined(__bpf__) +#define __bpf_ctx __attribute__((preserve_static_offset)) +#elif __has_attribute(btf_decl_tag) && !defined(__cplusplus) +#define __bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#else +#define __bpf_ctx +#endif + struct bpf_insn { __u8 code; /* opcode */ __u8 dst_reg:4; /* dest register */ @@ -6190,7 +6198,7 @@ struct __sk_buff { __u8 tstamp_type; __u32 :24; /* Padding, future use. */ __u64 hwtstamp; -}; +} __bpf_ctx; struct bpf_tunnel_key { __u32 tunnel_id; @@ -6271,7 +6279,7 @@ struct bpf_sock { __u32 dst_ip6[4]; __u32 state; __s32 rx_queue_mapping; -}; +} __bpf_ctx; struct bpf_tcp_sock { __u32 snd_cwnd; /* Sending congestion window */ @@ -6379,7 +6387,7 @@ struct xdp_md { __u32 rx_queue_index; /* rxq->queue_index */ __u32 egress_ifindex; /* txq->dev->ifindex */ -}; +} __bpf_ctx; /* DEVMAP map-value layout * @@ -6429,7 +6437,7 @@ struct sk_msg_md { __u32 size; /* Total size of sk_msg */ __bpf_md_ptr(struct bpf_sock *, sk); /* current socket */ -}; +} __bpf_ctx; struct sk_reuseport_md { /* @@ -6468,7 +6476,7 @@ struct sk_reuseport_md { */ __bpf_md_ptr(struct bpf_sock *, sk); __bpf_md_ptr(struct bpf_sock *, migrating_sk); -}; +} __bpf_ctx; #define BPF_TAG_SIZE 8 @@ -6678,7 +6686,7 @@ struct bpf_sock_addr { * Stored in network byte order. */ __bpf_md_ptr(struct bpf_sock *, sk); -}; +} __bpf_ctx; /* User bpf_sock_ops struct to access socket values and specify request ops * and their replies. @@ -6761,7 +6769,7 @@ struct bpf_sock_ops { * been written yet. */ __u64 skb_hwtstamp; -}; +} __bpf_ctx; /* Definitions for bpf_sock_ops_cb_flags */ enum { @@ -7034,11 +7042,11 @@ struct bpf_cgroup_dev_ctx { __u32 access_type; __u32 major; __u32 minor; -}; +} __bpf_ctx; struct bpf_raw_tracepoint_args { __u64 args[0]; -}; +} __bpf_ctx; /* DIRECT: Skip the FIB rules and go to FIB table associated with device * OUTPUT: Do lookup from egress perspective; default is ingress @@ -7245,7 +7253,7 @@ struct bpf_sysctl { __u32 file_pos; /* Sysctl file position to read from, write to. * Allows 1,2,4-byte read an 4-byte write. */ -}; +} __bpf_ctx; struct bpf_sockopt { __bpf_md_ptr(struct bpf_sock *, sk); @@ -7256,7 +7264,7 @@ struct bpf_sockopt { __s32 optname; __s32 optlen; __s32 retval; -}; +} __bpf_ctx; struct bpf_pidns_info { __u32 pid; @@ -7280,7 +7288,7 @@ struct bpf_sk_lookup { __u32 local_ip6[4]; /* Network byte order */ __u32 local_port; /* Host byte order */ __u32 ingress_ifindex; /* The arriving interface. Determined by inet_iif. */ -}; +} __bpf_ctx; /* * struct btf_ptr is used for typed pointer representation; the @@ -7406,4 +7414,6 @@ struct bpf_iter_num { __u64 __opaque[1]; } __attribute__((aligned(8))); +#undef __bpf_ctx + #endif /* _UAPI__LINUX_BPF_H__ */ diff --git a/tools/include/uapi/linux/bpf_perf_event.h b/tools/include/uapi/linux/bpf_perf_event.h index eb1b9d21250c..4f3f1e906f96 100644 --- a/tools/include/uapi/linux/bpf_perf_event.h +++ b/tools/include/uapi/linux/bpf_perf_event.h @@ -10,10 +10,20 @@ #include +#if __has_attribute(preserve_static_offset) && defined(__bpf__) +#define __bpf_ctx __attribute__((preserve_static_offset)) +#elif __has_attribute(btf_decl_tag) && !defined(__cplusplus) +#define __bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#else +#define __bpf_ctx +#endif + struct bpf_perf_event_data { bpf_user_pt_regs_t regs; __u64 sample_period; __u64 addr; -}; +} __bpf_ctx; + +#undef __bpf_ctx #endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */ From patchwork Wed Dec 20 13:34:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduard Zingerman X-Patchwork-Id: 13500081 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 594752D63F for ; Wed, 20 Dec 2023 13:34:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dOaMs9h2" Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-553338313a0so4588041a12.2 for ; Wed, 20 Dec 2023 05:34:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1703079265; x=1703684065; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=YQixnI1ROZf+QNKlUZTJhjGervT3Yg/doBsBIU+T8oc=; b=dOaMs9h2vf0hprQ9W0q8cUdW6/bptdWPj7cz+MNOKJnKTBbfIPwHIju9lrQXkg2Vnq zhAqGcKtq2RB3ReRCNAalAK9c3SkzavVDKaCKUQIciaW36SuvagnZeujcPlfeeled4LU EYHBhTEHAgjMTRs1DSgq7LimDu6kOE+c+umq2/z+adWuBcK6x47nGnIRDhk23xuU5Iao QSPu7boPafOqOy4qcIymioD68myPURq+VC7/QOUfrQVkc8+OSO3qTP71Fe5513QY/Z+0 K8QwxubtwhmxVycGl6WBYR3EfEyhgg+ogXIhvPoaQUP3O0TjYxdbv5psZ3qx1YZqRX1a zYnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703079265; x=1703684065; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YQixnI1ROZf+QNKlUZTJhjGervT3Yg/doBsBIU+T8oc=; b=tkfdnYUdEfGGxbEzU+xjkstJ/ASL3WnqgV/TK8US/TJVtOODz667a/JcGZvzilonS7 PUt7sJwyK4DLTNOcaqxEHHD1be7j6WmIek4sIRDjLVnstsvEEFzuPZuJ2tJXX1nx0s4+ llkK9Dmf68kXgOfzWMWqSd0Z5Af4vytjnPQxxMM3efMt+RxDVHMB4h4HukeltnhIZ3Wm C61KvD/fuaUKO5OFWPvhKX9Jf6I8M1zVuRLJS0HdxqbBhQP2M9L2scwwkPuHy1Re63ei C5v+Ie+NwRYV2wIpWtaiwGn35lx04wV5dUnyesaVf+g9pdsEMnzD/4iLHlMeeNJMGazO fXqQ== X-Gm-Message-State: AOJu0YyN0HMRECrFAfDQmaCq4J5FZ/MrzFqgfzgf7V3a7GKIj4F6QDg7 hcOqxEdeiKcM2ver014zIQYD38w/2uA= X-Google-Smtp-Source: AGHT+IF66e75sFeTtVjiwBwyscr8V1OCIuSOenAP3qZoqzl+sMQ3kXH4ti6+ynBCTqCOAKyHTpw1Rw== X-Received: by 2002:a17:907:86a2:b0:a23:5530:f33b with SMTP id qa34-20020a17090786a200b00a235530f33bmr3550975ejc.108.1703079265001; Wed, 20 Dec 2023 05:34:25 -0800 (PST) Received: from localhost.localdomain (host-176-36-0-241.b024.la.net.ua. [176.36.0.241]) by smtp.gmail.com with ESMTPSA id vs4-20020a170907a58400b00a22fb8901c4sm9951032ejc.12.2023.12.20.05.34.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 05:34:24 -0800 (PST) From: Eduard Zingerman To: bpf@vger.kernel.org, ast@kernel.org Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev, kernel-team@fb.com, yonghong.song@linux.dev, quentin@isovalent.com, alan.maguire@oracle.com, Eduard Zingerman Subject: [RFC v3 2/3] bpftool: add attribute preserve_static_offset for context types Date: Wed, 20 Dec 2023 15:34:10 +0200 Message-ID: <20231220133411.22978-3-eddyz87@gmail.com> X-Mailer: git-send-email 2.42.1 In-Reply-To: <20231220133411.22978-1-eddyz87@gmail.com> References: <20231220133411.22978-1-eddyz87@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net X-Patchwork-State: RFC When printing vmlinux.h, emit attribute preserve_static_offset [0] for types that have associated BTF decl tag T with value "preserve_static_offset". To avoid hacking libbpf dump logic emit forward declarations annotated with attribute. Such forward declarations have to come before structure definitions. Only emit such forward declarations when context types are present in target BTF (identified by name). C language standard wording in section "6.7.2.1 Structure and union specifiers" [1] is vague, but example in 6.7.2.1.21 explicitly allows such notation, and it matches clang behavior. Here is how 'bpftool btf gen ... format c' looks after this change: #ifndef __VMLINUX_H__ #define __VMLINUX_H__ #if !defined(BPF_NO_PRESERVE_STATIC_OFFSET) && \ __has_attribute(preserve_static_offset) #pragma clang attribute push \ (__attribute__((preserve_static_offset)), apply_to = record) struct bpf_cgroup_dev_ctx; ... #pragma clang attribute pop #endif ... rest of the output unchanged ... This is a follow up for discussion in thread [2]. [0] https://clang.llvm.org/docs/AttributeReference.html#preserve-static-offset [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3088.pdf [2] https://lore.kernel.org/bpf/fce6188a-6ccc-4b92-9aa7-9ee18b2f2fa1@isovalent.com/ Signed-off-by: Eduard Zingerman --- tools/bpf/bpftool/btf.c | 135 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 91fcb75babe3..feded48ddd76 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -460,6 +460,136 @@ static void __printf(2, 0) btf_dump_printf(void *ctx, vfprintf(stdout, fmt, args); } +/* Recursively walk all dependencies of 'id' and mark those as true in + * array 'deps'. The goal is to find all types that would be printed by + * btf_dump if 'id' is dumped. + */ +static void mark_dependencies(const struct btf *btf, __u32 id, bool *deps) +{ + const struct btf_param *params; + const struct btf_array *arr; + const struct btf_type *t; + struct btf_member *m; + __u16 vlen, i; + + if (id == 0 || deps[id]) + return; + + deps[id] = true; + t = btf__type_by_id(btf, id); + if (!t) + return; + + switch (btf_kind(t)) { + case BTF_KIND_PTR: + case BTF_KIND_TYPEDEF: + case BTF_KIND_VOLATILE: + case BTF_KIND_CONST: + case BTF_KIND_RESTRICT: + case BTF_KIND_TYPE_TAG: + case BTF_KIND_DECL_TAG: + case BTF_KIND_FUNC: + mark_dependencies(btf, t->type, deps); + break; + case BTF_KIND_STRUCT: + case BTF_KIND_UNION: + vlen = btf_vlen(t); + m = btf_members(t); + for (i = 0; i < vlen; ++i) + mark_dependencies(btf, m[i].type, deps); + break; + case BTF_KIND_ARRAY: + arr = btf_array(t); + mark_dependencies(btf, arr->type, deps); + break; + case BTF_KIND_FUNC_PROTO: + vlen = btf_vlen(t); + params = btf_params(t); + mark_dependencies(btf, t->type, deps); + for (i = 0; i < vlen; ++i) + mark_dependencies(btf, params[i].type, deps); + break; + default: + /* ignore */ + break; + } +} + +/* Iterate all types in 'btf', if there are BTF_DECL_TAG records R + * with "preserve_static_offset" tag - emit a forward declaration + * for R->type annotated with preserve_static_offset attribute [0]. + * + * If root_type_ids/root_type_cnt is specified, filter generated declarations + * to only include root_type_ids and corresponding dependencies. + * + * [0] https://clang.llvm.org/docs/AttributeReference.html#preserve-static-offset + */ +static int emit_static_offset_protos(const struct btf *btf, + __u32 *root_type_ids, int root_type_cnt) +{ + bool *root_type_deps = NULL; + bool first = true; + __u32 i, id, cnt; + + cnt = btf__type_cnt(btf); + if (root_type_cnt) { + root_type_deps = calloc(cnt, sizeof(*root_type_deps)); + if (!root_type_deps) + return -ENOMEM; + + for (i = 0; i < (__u32)root_type_cnt; ++i) + mark_dependencies(btf, root_type_ids[i], root_type_deps); + } + + for (id = 1; id < cnt; ++id) { + const struct btf_type *t, *s; + const char *name, *tag; + + t = btf__type_by_id(btf, id); + if (!t) + continue; + + if (!btf_is_decl_tag(t)) + continue; + + tag = btf__name_by_offset(btf, t->name_off); + if (strcmp(tag, "preserve_static_offset") != 0) + continue; + + if (root_type_deps && !root_type_deps[t->type]) + continue; + + s = btf__type_by_id(btf, t->type); + if (!s) + continue; + + if (!btf_is_struct(s) && !btf_is_union(s)) + continue; + + name = btf__name_by_offset(btf, s->name_off); + if (!name) + continue; + + if (first) { + first = false; + printf("#if !defined(BPF_NO_PRESERVE_STATIC_OFFSET) && __has_attribute(preserve_static_offset)\n"); + printf("#pragma clang attribute push (__attribute__((preserve_static_offset)), apply_to = record)\n"); + printf("\n"); + } + + printf("struct %s;\n", name); + } + + if (!first) { + printf("\n"); + printf("#pragma clang attribute pop\n"); + printf("#endif /* BPF_NO_PRESERVE_STATIC_OFFSET */\n\n"); + } + + free(root_type_deps); + return 0; +} + static int dump_btf_c(const struct btf *btf, __u32 *root_type_ids, int root_type_cnt) { @@ -473,6 +603,11 @@ static int dump_btf_c(const struct btf *btf, printf("#ifndef __VMLINUX_H__\n"); printf("#define __VMLINUX_H__\n"); printf("\n"); + + err = emit_static_offset_protos(btf, root_type_ids, root_type_cnt); + if (err) + goto done; + printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n"); printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n"); printf("#endif\n\n"); From patchwork Wed Dec 20 13:34:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduard Zingerman X-Patchwork-Id: 13500082 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5ECDC34571 for ; Wed, 20 Dec 2023 13:34:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="duvH+fN6" Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-55333eb0312so3560727a12.1 for ; Wed, 20 Dec 2023 05:34:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1703079266; x=1703684066; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=w3A/URYk/DG7Nbo2s4FISJ26H5RRuamlRT5uGABUpxw=; b=duvH+fN6/JpqfzK+rrN7ehQ8SkyZbb7NIbJqyEf9aXG9i3BrCZUFkBMxQ0wXE3K2XY vkZ3CT6SHcbKE7kHsY+mDxDmRL3AKB0FyIMySYUgB6d2vgjo5NZYZoEkoEzWuhe/V7KE JKrE5xAl//rhetgnoTf/trbSi7owwFo1vW757Lim8tSfQwWbRW+CJr5RrJn5Df24R9UV NEZXz74513IL9eWXDT90l2uYmLyM5hg4vRq3pWkJtqx020MZt7iwNbPQWCw1iovwuBGV 4wi44ipdWC3QaO7glokLwn3U5f11fTjGHqc38q8CYjc85A8H3JJEsitnxjmLq6Uf+U2K w5+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703079266; x=1703684066; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=w3A/URYk/DG7Nbo2s4FISJ26H5RRuamlRT5uGABUpxw=; b=WcT6rnOvCEqXPA3fzESqWh/QummsGnz1Df+CycgLAOJ8lmxoq/X5mexIVVzUvaGCH0 mZYq8gtjWfElVV3TlqhpIsZxhhbzFDTv5NJXvY/wbRWZk1lg+rjKnDMNR5Xck9cxWkQS PruAweKzBENWaTn0SVV1DzGGNR2bOOHxXE5XHtOiCOZz4J8del1H7oTW/MDiCiUtWSb3 89QwdqAPCQhdxSZ3zgQ7EYOPVgKn05+yecHePq2wQ1SrRJlFMeLbczbGfka9Vj7qsbBk fND91eNeAWtOSyPkypYXgo7iZ6edUKNw1ejc9C0sodzMeSyy6Ov7xVjNT+2Zf/IL+JYb 7x1Q== X-Gm-Message-State: AOJu0YzioX3reH16mpdBkPSxyVKrNZ2tHnJ9gTHdRJI4jva9WIjvN7Hu cBMF8QKQ1De/VA5z5MQd7sfoxHrPtEY= X-Google-Smtp-Source: AGHT+IHlsOOIMQ6illWNLSeMhgElrmpwGRDjOW2Fk6H1fH90XRmaik9e0+vylhGEtL8+h05d4Os06A== X-Received: by 2002:a17:906:270a:b0:a23:54a3:696e with SMTP id z10-20020a170906270a00b00a2354a3696emr3110136ejc.13.1703079266198; Wed, 20 Dec 2023 05:34:26 -0800 (PST) Received: from localhost.localdomain (host-176-36-0-241.b024.la.net.ua. [176.36.0.241]) by smtp.gmail.com with ESMTPSA id vs4-20020a170907a58400b00a22fb8901c4sm9951032ejc.12.2023.12.20.05.34.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 05:34:25 -0800 (PST) From: Eduard Zingerman To: bpf@vger.kernel.org, ast@kernel.org Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev, kernel-team@fb.com, yonghong.song@linux.dev, quentin@isovalent.com, alan.maguire@oracle.com, Eduard Zingerman Subject: [RFC v3 3/3] selftests/bpf: verify bpftool emits preserve_static_offset Date: Wed, 20 Dec 2023 15:34:11 +0200 Message-ID: <20231220133411.22978-4-eddyz87@gmail.com> X-Mailer: git-send-email 2.42.1 In-Reply-To: <20231220133411.22978-1-eddyz87@gmail.com> References: <20231220133411.22978-1-eddyz87@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net X-Patchwork-State: RFC Extend test_bpftool.py with following test cases: - Load a small program that has some context types in it's BTF, verify that "bpftool btf dump file ... format c" emits preserve_static_offset attribute. - Load a small program that has no context types in it's BTF, verify that "bpftool btf dump file ... format c" does not emit preserve_static_offset attribute. - Load a small program that uses a map, verify that "bpftool btf dump map pinned ... value format c" emit preserve_static_offset for expected types. Signed-off-by: Eduard Zingerman --- .../bpf/progs/dummy_no_context_btf.c | 12 +++ .../selftests/bpf/progs/dummy_prog_with_map.c | 65 ++++++++++++ .../selftests/bpf/progs/dummy_sk_buff_user.c | 29 +++++ tools/testing/selftests/bpf/test_bpftool.py | 100 ++++++++++++++++++ 4 files changed, 206 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/dummy_no_context_btf.c create mode 100644 tools/testing/selftests/bpf/progs/dummy_prog_with_map.c create mode 100644 tools/testing/selftests/bpf/progs/dummy_sk_buff_user.c diff --git a/tools/testing/selftests/bpf/progs/dummy_no_context_btf.c b/tools/testing/selftests/bpf/progs/dummy_no_context_btf.c new file mode 100644 index 000000000000..5a1df4984dce --- /dev/null +++ b/tools/testing/selftests/bpf/progs/dummy_no_context_btf.c @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include + +/* A dummy program that does not reference context types in it's BTF */ +SEC("tc") +__u32 dummy_prog(void *ctx) +{ + return 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/dummy_prog_with_map.c b/tools/testing/selftests/bpf/progs/dummy_prog_with_map.c new file mode 100644 index 000000000000..0268317494ef --- /dev/null +++ b/tools/testing/selftests/bpf/progs/dummy_prog_with_map.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include "bpf_misc.h" + +#if __has_attribute(btf_decl_tag) +#define __decl_tag_bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#endif + +struct test_struct_a { + int v; +} __decl_tag_bpf_ctx; + +struct test_struct_b { + int v; +} __decl_tag_bpf_ctx; + +struct test_struct_c { + int v; +} __decl_tag_bpf_ctx; + +struct test_struct_d { + int v; +} __decl_tag_bpf_ctx; + +struct test_struct_e { + int v; +} __decl_tag_bpf_ctx; + +struct test_struct_f { + int v; +} __decl_tag_bpf_ctx; + +typedef struct test_struct_c test_struct_c_td; + +struct map_value { + struct test_struct_a a; + struct test_struct_b b[2]; + test_struct_c_td c; + const struct test_struct_d *(*d)(volatile struct test_struct_e *); +}; + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 4); + __type(key, int); + __type(value, struct map_value); +} test_map1 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 4); + __type(key, int); + __type(value, struct test_struct_f); +} test_map2 SEC(".maps"); + +/* A dummy program that references map 'test_map', used by test_bpftool.py */ +SEC("tc") +int dummy_prog_with_map(void *ctx) +{ + return 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/dummy_sk_buff_user.c b/tools/testing/selftests/bpf/progs/dummy_sk_buff_user.c new file mode 100644 index 000000000000..8c10aebe8689 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/dummy_sk_buff_user.c @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* In linux/bpf.h __bpf_ctx macro is defined differently for BPF and + * non-BPF targets: + * - for BPF it is __attribute__((preserve_static_offset)) + * - for non-BPF it is __attribute__((btf_decl_tag("preserve_static_offset"))) + * + * bpftool uses decl tag as a signal to emit preserve_static_offset, + * thus additional declaration is needed in this test. + */ +#if __has_attribute(btf_decl_tag) +#define __decl_tag_bpf_ctx __attribute__((btf_decl_tag(("preserve_static_offset")))) +#endif + +struct __decl_tag_bpf_ctx __sk_buff; + +#include +#include + +/* A dummy program that references __sk_buff type in it's BTF, + * used by test_bpftool.py. + */ +SEC("tc") +int sk_buff_user(struct __sk_buff *skb) +{ + return 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/test_bpftool.py b/tools/testing/selftests/bpf/test_bpftool.py index 1c2408ee1f5d..d3a8b71afcc1 100644 --- a/tools/testing/selftests/bpf/test_bpftool.py +++ b/tools/testing/selftests/bpf/test_bpftool.py @@ -3,10 +3,12 @@ import collections import functools +import io import json import os import socket import subprocess +import tempfile import unittest @@ -25,6 +27,10 @@ class UnprivilegedUserError(Exception): pass +class MissingDependencyError(Exception): + pass + + def _bpftool(args, json=True): _args = ["bpftool"] if json: @@ -63,12 +69,26 @@ DMESG_EMITTING_HELPERS = [ "bpf_trace_vprintk", ] +BPFFS_MOUNT = "/sys/fs/bpf/" + +DUMMY_SK_BUFF_USER_OBJ = cur_dir + "/dummy_sk_buff_user.bpf.o" +DUMMY_NO_CONTEXT_BTF_OBJ = cur_dir + "/dummy_no_context_btf.bpf.o" +DUMMY_PROG_WITH_MAP_OBJ = cur_dir + "/dummy_prog_with_map.bpf.o" + class TestBpftool(unittest.TestCase): @classmethod def setUpClass(cls): if os.getuid() != 0: raise UnprivilegedUserError( "This test suite needs root privileges") + objs = [DUMMY_SK_BUFF_USER_OBJ, + DUMMY_NO_CONTEXT_BTF_OBJ, + DUMMY_PROG_WITH_MAP_OBJ] + for obj in objs: + if os.path.exists(obj): + continue + raise MissingDependencyError( + "File " + obj + " does not exist, make sure progs/*.c are compiled") @default_iface def test_feature_dev_json(self, iface): @@ -172,3 +192,83 @@ class TestBpftool(unittest.TestCase): res = bpftool(["feature", "probe", "macros"]) for pattern in expected_patterns: self.assertRegex(res, pattern) + + def assertStringsPresent(self, text, patterns): + pos = 0 + for i, pat in enumerate(patterns): + m = text.find(pat, pos) + if m == -1: + with io.StringIO() as msg: + print("Can't find expected string:", file=msg) + for s in patterns[0:i]: + print(" MATCHED: " + s, file=msg) + print("NOT MATCHED: " + pat, file=msg) + print("", file=msg) + print("Searching in:", file=msg) + print(text, file=msg) + self.fail(msg.getvalue()) + pos += len(pat) + + def assertPreserveStaticOffset(self, btf_dump, types): + self.assertStringsPresent(btf_dump, [ + "#if !defined(BPF_NO_PRESERVE_STATIC_OFFSET) && " + + "__has_attribute(preserve_static_offset)", + "#pragma clang attribute push " + + "(__attribute__((preserve_static_offset)), apply_to = record)" + ] + ["struct " + t + ";" for t in types] + [ + "#endif /* BPF_NO_PRESERVE_STATIC_OFFSET */" + ]) + + # Load a small program that has some context types in it's BTF, + # verify that "bpftool btf dump file ... format c" emits + # preserve_static_offset attribute. + def test_c_dump_preserve_static_offset_present(self): + res = bpftool(["btf", "dump", "file", DUMMY_SK_BUFF_USER_OBJ, "format", "c"]) + self.assertPreserveStaticOffset(res, ["__sk_buff"]) + + # Load a small program that has no context types in it's BTF, + # verify that "bpftool btf dump file ... format c" does not emit + # preserve_static_offset attribute. + def test_c_dump_no_preserve_static_offset(self): + res = bpftool(["btf", "dump", "file", DUMMY_NO_CONTEXT_BTF_OBJ, "format", "c"]) + self.assertNotRegex(res, "preserve_static_offset") + self.assertStringsPresent(res, [ + "preserve_access_index", + "typedef unsigned int __u32;" + ]) + + # When BTF is dumped for maps bpftool follows a slightly different + # code path, that filters which BTF types would be printed. + # Test this code path here: + # - load a program that uses a map, value type of which references + # a number of structs annotated with preserve_static_offset; + # - dump BTF for that map and check preserve_static_offset annotation + # for expected structs. + def test_c_dump_preserve_static_offset_map(self): + prog_pin = tempfile.mktemp(prefix="dummy_prog_with_map", dir=BPFFS_MOUNT) + maps_dir = tempfile.mktemp(prefix="dummy_prog_with_map_maps", dir=BPFFS_MOUNT) + map_pin1 = maps_dir + "/test_map1" + map_pin2 = maps_dir + "/test_map2" + + bpftool(["prog", "load", DUMMY_PROG_WITH_MAP_OBJ, prog_pin, "pinmaps", maps_dir]) + try: + map1 = bpftool(["btf", "dump", "map", "pinned", map_pin1, "value", "format", "c"]) + map2 = bpftool(["btf", "dump", "map", "pinned", map_pin2, "value", "format", "c"]) + finally: + os.remove(prog_pin) + os.remove(map_pin1) + os.remove(map_pin2) + os.rmdir(maps_dir) + + # test_map1 should have all types except struct test_struct_f + self.assertPreserveStaticOffset(map1, [ + "test_struct_a", "test_struct_b", "test_struct_c", + "test_struct_d", "test_struct_e", + ]) + self.assertNotRegex(map1, "test_struct_f") + + # test_map2 should have only struct test_struct_f + self.assertPreserveStaticOffset(map2, [ + "test_struct_f" + ]) + self.assertNotRegex(map2, "struct test_struct_a")