On Tue, 12 Jul 2022, Matthieu Baerts wrote:
Hi Jiri,
On 12/07/2022 14:12, Jiri Olsa wrote:
On Tue, Jul 12, 2022 at 11:06:38AM +0200, Matthieu Baerts wrote:
Hi Jiri, Mat,
On 11/07/2022 23:21, Mat Martineau wrote:
On Mon, 11 Jul 2022, Jiri Olsa wrote:
The btf_sock_ids array needs struct mptcp_sock BTF ID for
the bpf_skc_to_mptcp_sock helper.
When CONFIG_MPTCP is disabled, the 'struct mptcp_sock' is not
defined and resolve_btfids will complain with:
BTFIDS vmlinux
WARN: resolve_btfids: unresolved symbol mptcp_sock
Adding empty difinition for struct mptcp_sock when CONFIG_MPTCP
is disabled.
Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
---
include/net/mptcp.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index ac9cf7271d46..25741a52c666 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -59,6 +59,10 @@ struct mptcp_addr_info {
};
};
+#if !IS_ENABLED(CONFIG_MPTCP)
+struct mptcp_sock { };
+#endif
The only use of struct mptcp_sock I see with !CONFIG_MPTCP is from this
stub at the end of mptcp.h:
static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock
*sk) { return NULL; }
It's normally defined in net/mptcp/protocol.h for the MPTCP subsystem code.
The conditional could be added on the line before the stub to make it
clear that the empty struct is associated with that inline stub.
If this is required only for this specific BPF function, why not
modifying this stub (or add a define) to return "void *" instead of
"struct mptcp_sock *"?
so btf_sock_ids array needs BTF ID for 'struct mptcp_sock' and if CONFIG_MPTCP
is not enabled, then resolve_btfids (which resolves and populate all BTF IDs)
won't find it and will complain
btf_sock_ids keeps all socket IDs regardles the state of their CONFIG options,
and relies that sock structs are defined even if related CONFIG option is disabled
Thank you for the explanation. I didn't know about that.
Then it is fine for me to leave it in mptcp.h. If it is not directly
linked to bpf_mptcp_sock_from_subflow(), I guess it can stay there but
maybe better to wait for Mat's answer about that.
if that is false assumption then maybe we need to make btf_sock_ids values optional
somehow
I'd rather keep the full mptcp_sock definition in net/mptcp/protocol.h
since moving it would require also moving a few other structs it depends
on.
Defining the empty struct in mptcp.h is fine with me and it sounds like
that meets the needs of btf_sock_ids - but I'd like a v2 of this patch
that moves the new empty struct declaration next to the inline
btf_mptcp_sock_from_subflow function in mptcp.h
--
Mat Martineau
Intel
