include/linux/bpf.h | 19 ----------------- include/linux/btf_ids.h | 3 ++- include/linux/net.h | 1 + kernel/bpf/verifier.c | 20 ------------------ net/core/filter.c | 13 ++++++++++++ net/mptcp/bpf.c | 46 ++++++----------------------------------- 6 files changed, 22 insertions(+), 80 deletions(-)
This patch addressed to the comments in [1] from BPF maintainer Alexei:
'''
On Fri, Sep 18, 2020 at 02:10:42PM +0200, Nicolas Rybowski wrote:
> +
> +BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
> +{
> + if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
> + struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
Could you add !sk check here as well?
See commit 8c33dadc3e0e ("bpf: Bpf_skc_to_* casting helpers require a NULL check on sk")
It's not strictly necessary yet, but see below.
'''
I added this !sk check as he suggested.
'''
I think we shouldn't extend the verifier with PTR_TO_MPTCP_SOCK and similar concept anymore.
This approach doesn't scale and we have better way to handle such field access with BTF.
> + }
> + return (unsigned long)NULL;
> +}
> +
> +const struct bpf_func_proto bpf_mptcp_sock_proto = {
> + .func = bpf_mptcp_sock,
> + .gpl_only = false,
> + .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
In this particular case you can do:
+ .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
Then bpf_mptcp_sock_convert_ctx_access() will no longer be necessary
and bpf prog will be able to access all mptcp_sock fields right away.
Will that work for your use case?
'''
I dropped bpf_mptcp_sock_convert_ctx_access() and use RET_PTR_TO_BTF_ID_OR_NULL
instead of RET_PTR_TO_MPTCP_SOCK_OR_NULL as he suggested..
Both 'bpf selftests' and 'bpf exampe' work with this patch.
[1]
https://lore.kernel.org/netdev/20200922040830.3iis6xiavhvpfq3v@ast-mbp.dhcp.thefacebook.com/
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
include/linux/bpf.h | 19 -----------------
include/linux/btf_ids.h | 3 ++-
include/linux/net.h | 1 +
kernel/bpf/verifier.c | 20 ------------------
net/core/filter.c | 13 ++++++++++++
net/mptcp/bpf.c | 46 ++++++-----------------------------------
6 files changed, 22 insertions(+), 80 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5a17c1e3a6bc..6f840ece0e96 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -407,7 +407,6 @@ enum bpf_return_type {
RET_PTR_TO_MAP_VALUE, /* returns a pointer to map elem value */
RET_PTR_TO_SOCKET, /* returns a pointer to a socket */
RET_PTR_TO_TCP_SOCK, /* returns a pointer to a tcp_sock */
- RET_PTR_TO_MPTCP_SOCK, /* returns a pointer to mptcp_sock */
RET_PTR_TO_SOCK_COMMON, /* returns a pointer to a sock_common */
RET_PTR_TO_ALLOC_MEM, /* returns a pointer to dynamically allocated memory */
RET_PTR_TO_MEM_OR_BTF_ID, /* returns a pointer to a valid memory or a btf_id */
@@ -418,7 +417,6 @@ enum bpf_return_type {
RET_PTR_TO_MAP_VALUE_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MAP_VALUE,
RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
- RET_PTR_TO_MPTCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MPTCP_SOCK,
RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_ALLOC_MEM,
RET_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID,
@@ -499,7 +497,6 @@ enum bpf_reg_type {
PTR_TO_SOCKET, /* reg points to struct bpf_sock */
PTR_TO_SOCK_COMMON, /* reg points to sock_common */
PTR_TO_TCP_SOCK, /* reg points to struct tcp_sock */
- PTR_TO_MPTCP_SOCK, /* reg points to struct mptcp_sock */
PTR_TO_TP_BUFFER, /* reg points to a writable raw tp's buffer */
PTR_TO_XDP_SOCK, /* reg points to struct xdp_sock */
/* PTR_TO_BTF_ID points to a kernel struct that does not need
@@ -528,7 +525,6 @@ enum bpf_reg_type {
PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | PTR_TO_SOCKET,
PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | PTR_TO_SOCK_COMMON,
PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | PTR_TO_TCP_SOCK,
- PTR_TO_MPTCP_SOCK_OR_NULL = PTR_MAYBE_NULL | PTR_TO_MPTCP_SOCK,
PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | PTR_TO_BTF_ID,
/* This must be the last entry. Its purpose is to ensure the enum is
@@ -2364,12 +2360,6 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
bool bpf_mptcp_sock_is_valid_access(int off, int size,
enum bpf_access_type type,
struct bpf_insn_access_aux *info);
-
-u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog,
- u32 *target_size);
#else /* CONFIG_MPTCP */
static inline bool bpf_mptcp_sock_is_valid_access(int off, int size,
enum bpf_access_type type,
@@ -2377,15 +2367,6 @@ static inline bool bpf_mptcp_sock_is_valid_access(int off, int size,
{
return false;
}
-
-static inline u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog,
- u32 *target_size)
-{
- return 0;
-}
#endif /* CONFIG_MPTCP */
enum bpf_text_poke_type {
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index bc5d9cc34e4c..335a19092368 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -178,7 +178,8 @@ extern struct btf_id_set name;
BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP6, tcp6_sock) \
BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP, udp_sock) \
BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP6, udp6_sock) \
- BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock)
+ BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock) \
+ BTF_SOCK_TYPE(BTF_SOCK_TYPE_MPTCP, mptcp_sock)
enum {
#define BTF_SOCK_TYPE(name, str) name,
diff --git a/include/linux/net.h b/include/linux/net.h
index ba736b457a06..9c1a634e5ac5 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -238,6 +238,7 @@ int sock_recvmsg(struct socket *sock, struct msghdr *msg, int flags);
struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname);
struct socket *sockfd_lookup(int fd, int *err);
struct socket *sock_from_file(struct file *file);
+struct sock *msk_from_subflow(struct sock *sk);
#define sockfd_put(sock) fput(sock->file)
int net_ratelimit(void);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1bb1b5d0419d..d7473fee247c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -430,7 +430,6 @@ static bool type_is_sk_pointer(enum bpf_reg_type type)
return type == PTR_TO_SOCKET ||
type == PTR_TO_SOCK_COMMON ||
type == PTR_TO_TCP_SOCK ||
- type == PTR_TO_MPTCP_SOCK ||
type == PTR_TO_XDP_SOCK;
}
@@ -438,7 +437,6 @@ static bool reg_type_not_null(enum bpf_reg_type type)
{
return type == PTR_TO_SOCKET ||
type == PTR_TO_TCP_SOCK ||
- type == PTR_TO_MPTCP_SOCK ||
type == PTR_TO_MAP_VALUE ||
type == PTR_TO_MAP_KEY ||
type == PTR_TO_SOCK_COMMON;
@@ -454,7 +452,6 @@ static bool reg_type_may_be_refcounted_or_null(enum bpf_reg_type type)
{
return base_type(type) == PTR_TO_SOCKET ||
base_type(type) == PTR_TO_TCP_SOCK ||
- base_type(type) == PTR_TO_MPTCP_SOCK ||
base_type(type) == PTR_TO_MEM ||
base_type(type) == PTR_TO_BTF_ID;
}
@@ -554,7 +551,6 @@ static const char *reg_type_str(struct bpf_verifier_env *env,
[PTR_TO_SOCKET] = "sock",
[PTR_TO_SOCK_COMMON] = "sock_common",
[PTR_TO_TCP_SOCK] = "tcp_sock",
- [PTR_TO_MPTCP_SOCK] = "mptcp_sock",
[PTR_TO_TP_BUFFER] = "tp_buffer",
[PTR_TO_XDP_SOCK] = "xdp_sock",
[PTR_TO_BTF_ID] = "ptr_",
@@ -2778,7 +2774,6 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
case PTR_TO_BTF_ID:
case PTR_TO_BUF:
@@ -3671,9 +3666,6 @@ static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
case PTR_TO_TCP_SOCK:
valid = bpf_tcp_sock_is_valid_access(off, size, t, &info);
break;
- case PTR_TO_MPTCP_SOCK:
- valid = bpf_mptcp_sock_is_valid_access(off, size, t, &info);
- break;
case PTR_TO_XDP_SOCK:
valid = bpf_xdp_sock_is_valid_access(off, size, t, &info);
break;
@@ -3830,9 +3822,6 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
case PTR_TO_TCP_SOCK:
pointer_desc = "tcp_sock ";
break;
- case PTR_TO_MPTCP_SOCK:
- pointer_desc = "mptcp_sock ";
- break;
case PTR_TO_XDP_SOCK:
pointer_desc = "xdp_sock ";
break;
@@ -6762,9 +6751,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
} else if (base_type(ret_type) == RET_PTR_TO_TCP_SOCK) {
mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].type = PTR_TO_TCP_SOCK | ret_flag;
- } else if (base_type(ret_type) == RET_PTR_TO_MPTCP_SOCK) {
- mark_reg_known_zero(env, regs, BPF_REG_0);
- regs[BPF_REG_0].type = PTR_TO_MPTCP_SOCK | ret_flag;
} else if (base_type(ret_type) == RET_PTR_TO_ALLOC_MEM) {
mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag;
@@ -7479,7 +7465,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
verbose(env, "R%d pointer arithmetic on %s prohibited\n",
dst, reg_type_str(env, ptr_reg->type));
@@ -10854,7 +10839,6 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
/* Only valid matches are exact, which memcmp() above
* would have accepted
@@ -11385,7 +11369,6 @@ static bool reg_type_mismatch_ok(enum bpf_reg_type type)
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
case PTR_TO_BTF_ID:
return false;
@@ -12810,9 +12793,6 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
case PTR_TO_TCP_SOCK:
convert_ctx_access = bpf_tcp_sock_convert_ctx_access;
break;
- case PTR_TO_MPTCP_SOCK:
- convert_ctx_access = bpf_mptcp_sock_convert_ctx_access;
- break;
case PTR_TO_XDP_SOCK:
convert_ctx_access = bpf_xdp_sock_convert_ctx_access;
break;
diff --git a/net/core/filter.c b/net/core/filter.c
index a07b28997ad3..a143f7631bba 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11143,6 +11143,19 @@ const struct bpf_func_proto bpf_sock_from_file_proto = {
.arg1_btf_id = &bpf_sock_from_file_btf_ids[1],
};
+BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
+{
+ return (unsigned long)msk_from_subflow(sk);
+}
+
+const struct bpf_func_proto bpf_mptcp_sock_proto = {
+ .func = bpf_mptcp_sock,
+ .gpl_only = false,
+ .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
+ .arg1_type = ARG_PTR_TO_SOCK_COMMON,
+ .ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_MPTCP],
+};
+
static const struct bpf_func_proto *
bpf_sk_base_func_proto(enum bpf_func_id func_id)
{
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 5332469fbb28..6877fa5b5c66 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -26,47 +26,13 @@ bool bpf_mptcp_sock_is_valid_access(int off, int size, enum bpf_access_type type
}
}
-u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog, u32 *target_size)
+struct sock *msk_from_subflow(struct sock *sk)
{
- struct bpf_insn *insn = insn_buf;
+ if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
+ struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
-#define BPF_MPTCP_SOCK_GET_COMMON(FIELD) \
- do { \
- BUILD_BUG_ON(sizeof_field(struct mptcp_sock, FIELD) > \
- sizeof_field(struct bpf_mptcp_sock, FIELD)); \
- *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct mptcp_sock, FIELD), \
- si->dst_reg, si->src_reg, \
- offsetof(struct mptcp_sock, FIELD)); \
- } while (0)
-
- if (insn > insn_buf)
- return insn - insn_buf;
-
- switch (si->off) {
- case offsetof(struct bpf_mptcp_sock, token):
- BPF_MPTCP_SOCK_GET_COMMON(token);
- break;
+ return subflow->conn;
}
-
- return insn - insn_buf;
+ return NULL;
}
-
-BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
-{
- if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
- struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
-
- return (unsigned long)mptcp_sfc->conn;
- }
- return (unsigned long)NULL;
-}
-
-const struct bpf_func_proto bpf_mptcp_sock_proto = {
- .func = bpf_mptcp_sock,
- .gpl_only = false,
- .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
- .arg1_type = ARG_PTR_TO_SOCK_COMMON,
-};
+EXPORT_SYMBOL(msk_from_subflow);
--
2.34.1
© 2016 - 2024 Red Hat, Inc.