From a9167579d2f134fec9a20d3b772b67e039f0fadd Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Wed, 18 Dec 2024 21:14:13 +0000 Subject: [PATCH 1/8] Add the skeleton to attach kfunc probes Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/bpf_tools/bcc_wrapper.cc | 35 +++++++++++++++++++ src/stirling/bpf_tools/bcc_wrapper.h | 18 ++++++++++ .../bpf_tools/probe_specs/probe_specs.h | 11 ++++++ .../tcp_stats/bcc_bpf/tcp_stats.c | 15 ++++++++ .../tcp_stats/tcp_stats_connector.cc | 14 ++++++-- 5 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/stirling/bpf_tools/bcc_wrapper.cc b/src/stirling/bpf_tools/bcc_wrapper.cc index 370f421836d..9bb3c9ec964 100644 --- a/src/stirling/bpf_tools/bcc_wrapper.cc +++ b/src/stirling/bpf_tools/bcc_wrapper.cc @@ -243,6 +243,25 @@ Status BCCWrapperImpl::AttachSamplingProbe(const SamplingProbeSpec& probe) { return AttachPerfEvent(perf_event_spec); } +Status BCCWrapperImpl::AttachKFunc(const KFuncSpec& probe) { + LOG(INFO) << "Deploying kfunc: " << probe.ToString(); + + int prog_fd; + auto res = bpf_.load_func(probe.kfunc, BPF_PROG_TYPE_TRACING, prog_fd); + LOG_IF(ERROR, !res.ok()) << res.msg(); + + int attach_fd = bpf_attach_kfunc(prog_fd); + if (attach_fd < 0) { + bpf_.unload_func(probe.kfunc); + return error::Internal("Unable to attach kfunc"); + } + + kfuncs_.push_back(probe); + ++num_attached_kfuncs_; + + return Status::OK(); +} + Status BCCWrapperImpl::AttachKProbes(const ArrayView<KProbeSpec>& probes) { for (const KProbeSpec& p : probes) { PX_RETURN_IF_ERROR(AttachKProbe(p)); @@ -271,6 +290,13 @@ Status BCCWrapperImpl::AttachSamplingProbes(const ArrayView<SamplingProbeSpec>& return Status::OK(); } +Status BCCWrapperImpl::AttachKFuncs(const ArrayView<KFuncSpec>& probes) { + for (const KFuncSpec& p : probes) { + PX_RETURN_IF_ERROR(AttachKFunc(p)); + } + return Status::OK(); +} + // This will replace the XDP program previously-attached on the the same device. // Newer kernel allows attaching multiple XDP programs on the same device: // https://lwn.net/Articles/801478/ @@ -323,6 +349,15 @@ Status BCCWrapperImpl::DetachTracepoint(const TracepointSpec& probe) { return Status::OK(); } +// Status BCCWrapperImpl::DetachKFunc(const KFuncSpec& probe) { +// LOG(INFO) << "Detaching kfunc " << probe.ToString(); + +// PX_RETURN_IF_ERROR(bpf_.detach_kfunc(probe.kfunc)); + +// --num_attached_kfuncs_; +// return Status::OK(); +// } + void BCCWrapperImpl::DetachKProbes() { for (const auto& p : kprobes_) { auto res = DetachKProbe(p); diff --git a/src/stirling/bpf_tools/bcc_wrapper.h b/src/stirling/bpf_tools/bcc_wrapper.h index c33ff6188df..53909258c8f 100644 --- a/src/stirling/bpf_tools/bcc_wrapper.h +++ b/src/stirling/bpf_tools/bcc_wrapper.h @@ -139,6 +139,13 @@ class BCCWrapper { */ virtual Status AttachSamplingProbe(const SamplingProbeSpec& probe) = 0; + /** + * Attach a single kfunc. + * @param probe Specifications of the kfunc. + * @return Error if probe fails to attach. + */ + virtual Status AttachKFunc(const KFuncSpec& probe) = 0; + /** * Open a perf buffer for reading events. * @param perf_buff Specifications of the perf buffer (name, callback function, etc.). @@ -182,6 +189,13 @@ class BCCWrapper { */ virtual Status AttachSamplingProbes(const ArrayView<SamplingProbeSpec>& probes) = 0; + /** + * Convenience function that attaches multiple kfuncs. + * @param probes Vector of probes. + * @return Error of first probe to fail to attach (remaining probe attachments are not attempted). + */ + virtual Status AttachKFuncs(const ArrayView<KFuncSpec>& probes) = 0; + /** * Convenience function that attaches a XDP program. */ @@ -260,6 +274,7 @@ class BCCWrapper { inline static size_t num_attached_tracepoints_; inline static size_t num_open_perf_buffers_; inline static size_t num_attached_perf_events_; + inline static size_t num_attached_kfuncs_; private: // This is shared by all source connectors that uses BCCWrapper. @@ -289,12 +304,14 @@ class BCCWrapperImpl : public BCCWrapper { Status AttachUProbe(const UProbeSpec& probe) override; Status AttachTracepoint(const TracepointSpec& probe) override; Status AttachSamplingProbe(const SamplingProbeSpec& probe) override; + Status AttachKFunc(const KFuncSpec& probe) override; Status OpenPerfBuffer(const PerfBufferSpec& perf_buffer) override; Status AttachPerfEvent(const PerfEventSpec& perf_event) override; Status AttachKProbes(const ArrayView<KProbeSpec>& probes) override; Status AttachTracepoints(const ArrayView<TracepointSpec>& probes) override; Status AttachUProbes(const ArrayView<UProbeSpec>& uprobes) override; Status AttachSamplingProbes(const ArrayView<SamplingProbeSpec>& probes) override; + Status AttachKFuncs(const ArrayView<KFuncSpec>& probes) override; Status AttachXDP(const std::string& dev_name, const std::string& fn_name) override; Status OpenPerfBuffers(const ArrayView<PerfBufferSpec>& perf_buffers) override; Status AttachPerfEvents(const ArrayView<PerfEventSpec>& perf_events) override; @@ -332,6 +349,7 @@ class BCCWrapperImpl : public BCCWrapper { std::vector<UProbeSpec> uprobes_; std::vector<TracepointSpec> tracepoints_; std::vector<PerfEventSpec> perf_events_; + std::vector<KFuncSpec> kfuncs_; protected: std::vector<PerfBufferSpec> perf_buffer_specs_; diff --git a/src/stirling/bpf_tools/probe_specs/probe_specs.h b/src/stirling/bpf_tools/probe_specs/probe_specs.h index 4eb53071204..b14b649f65f 100644 --- a/src/stirling/bpf_tools/probe_specs/probe_specs.h +++ b/src/stirling/bpf_tools/probe_specs/probe_specs.h @@ -138,6 +138,17 @@ struct TracepointSpec { } }; +/** + * Describes a probe on a kernel function. + */ +struct KFuncSpec { + std::string kfunc; + + std::string ToString() const { + return absl::Substitute("[kfunc=$0]", kfunc); + } +}; + /** * Describes a sampling probe that triggers according to a time period. * This is in contrast to KProbes and UProbes, which trigger based on diff --git a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c index 8d78e4579c7..72e9f251e93 100644 --- a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c +++ b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c @@ -226,3 +226,18 @@ int probe_entry_tcp_retransmit_skb(struct pt_regs* ctx, struct sock* skp, struct tcp_events.perf_submit(ctx, &event, sizeof(event)); return 0; } + +KFUNC_PROBE(kfunc__tcp_sendmsg, struct sock* sk) { + return tcp_send_entry(sk); +} + +KRETFUNC_PROBE(kretfunc__tcp_sendmsg, struct pt_regs* regs) { + int size = PT_REGS_RC(regs); + if (size > 0) { + uint64_t id = bpf_get_current_pid_tgid(); + uint32_t tgid = id >> 32; + return tcp_sendstat(regs, tgid, id, size); + } else { + return 0; + } +} diff --git a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc index 76a4af9075a..9ffd53881f3 100644 --- a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc +++ b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc @@ -40,9 +40,7 @@ constexpr uint32_t kPerfBufferPerCPUSizeBytes = 50 * 1024 * 1024; using ProbeType = bpf_tools::BPFProbeAttachType; const auto kProbeSpecs = MakeArray<bpf_tools::KProbeSpec>( - {{"tcp_sendmsg", ProbeType::kEntry, "probe_entry_tcp_sendmsg", /*is_syscall*/ false}, - {"tcp_sendmsg", ProbeType::kReturn, "probe_ret_tcp_sendmsg", /*is_syscall*/ false}, - {"tcp_cleanup_rbuf", ProbeType::kEntry, "probe_entry_tcp_cleanup_rbuf", /*is_syscall*/ false}, + {{"tcp_cleanup_rbuf", ProbeType::kEntry, "probe_entry_tcp_cleanup_rbuf", /*is_syscall*/ false}, {"tcp_retransmit_skb", ProbeType::kEntry, "probe_entry_tcp_retransmit_skb", /*is_syscall*/ false}}); @@ -50,6 +48,10 @@ const auto kSendPageProbeSpecs = MakeArray<bpf_tools::KProbeSpec>( {{"tcp_sendpage", ProbeType::kEntry, "probe_entry_tcp_sendpage", /*is_syscall*/ false}, {"tcp_sendpage", ProbeType::kReturn, "probe_ret_tcp_sendpage", /*is_syscall*/ false}}); +// FIXME: Figure out whether to even create a KFuncSpec +// const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc____tcp_enter_loss"}, {"kretfunc__tcp_sendmsg"}}); +const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc__tcp_sendmsg"}, {"kretfunc__tcp_sendmsg"}}); + void HandleTcpEvent(void* cb_cookie, void* data, int /*data_size*/) { auto* connector = reinterpret_cast<TCPStatsConnector*>(cb_cookie); auto* event = reinterpret_cast<struct tcp_event_t*>(data); @@ -82,8 +84,14 @@ Status TCPStatsConnector::InitImpl() { "was removed in Kernel 6.5.", sendpage_attach_status.msg(), kernel_version.ToString()); } + + // FIXME: Add comment here explaining the probe conflict with socket tracer. + PX_RETURN_IF_ERROR(bcc_->AttachKFuncs(kFuncSpecs)); + PX_RETURN_IF_ERROR(bcc_->OpenPerfBuffers(perf_buffer_specs)); LOG(INFO) << absl::Substitute("Successfully deployed $0 kprobes.", kProbeSpecs.size()); + + // FIXME: Add a log to print the number of deployed kfunc probes. return Status::OK(); } From 147e052d92866195e28052802776ac09e57e0c1b Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Wed, 18 Dec 2024 22:38:49 +0000 Subject: [PATCH 2/8] Rename func to probe in bpf code Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c index 72e9f251e93..ccfbbf7e765 100644 --- a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c +++ b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c @@ -227,11 +227,11 @@ int probe_entry_tcp_retransmit_skb(struct pt_regs* ctx, struct sock* skp, struct return 0; } -KFUNC_PROBE(kfunc__tcp_sendmsg, struct sock* sk) { +KFUNC_PROBE(tcp_sendmsg, struct sock* sk) { return tcp_send_entry(sk); } -KRETFUNC_PROBE(kretfunc__tcp_sendmsg, struct pt_regs* regs) { +KRETFUNC_PROBE(tcp_sendmsg, struct pt_regs* regs) { int size = PT_REGS_RC(regs); if (size > 0) { uint64_t id = bpf_get_current_pid_tgid(); From 535722a208ec75719c12d875f391b9755e56f66f Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Thu, 19 Dec 2024 21:23:49 +0000 Subject: [PATCH 3/8] Fix kfunc probe name, add unload methods to bcc wrapper, comments Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/bpf_tools/bcc_wrapper.cc | 23 +++++++++---- src/stirling/bpf_tools/bcc_wrapper.h | 4 ++- .../tcp_stats/bcc_bpf/tcp_stats.c | 33 +++++++++---------- .../tcp_stats/tcp_stats_connector.cc | 8 ++--- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/stirling/bpf_tools/bcc_wrapper.cc b/src/stirling/bpf_tools/bcc_wrapper.cc index 9bb3c9ec964..798838a4dcf 100644 --- a/src/stirling/bpf_tools/bcc_wrapper.cc +++ b/src/stirling/bpf_tools/bcc_wrapper.cc @@ -244,7 +244,7 @@ Status BCCWrapperImpl::AttachSamplingProbe(const SamplingProbeSpec& probe) { } Status BCCWrapperImpl::AttachKFunc(const KFuncSpec& probe) { - LOG(INFO) << "Deploying kfunc: " << probe.ToString(); + VLOG(1) << absl::Substitute("Deploying kfunc $0", probe.ToString()); int prog_fd; auto res = bpf_.load_func(probe.kfunc, BPF_PROG_TYPE_TRACING, prog_fd); @@ -349,14 +349,14 @@ Status BCCWrapperImpl::DetachTracepoint(const TracepointSpec& probe) { return Status::OK(); } -// Status BCCWrapperImpl::DetachKFunc(const KFuncSpec& probe) { -// LOG(INFO) << "Detaching kfunc " << probe.ToString(); +Status BCCWrapperImpl::UnloadKFunc(const KFuncSpec& probe) { + VLOG(1) << absl::Substitute("Unloading kfunc $0", probe.ToString()); -// PX_RETURN_IF_ERROR(bpf_.detach_kfunc(probe.kfunc)); + PX_RETURN_IF_ERROR(bpf_.unload_func(probe.kfunc)); -// --num_attached_kfuncs_; -// return Status::OK(); -// } + --num_attached_kfuncs_; + return Status::OK(); +} void BCCWrapperImpl::DetachKProbes() { for (const auto& p : kprobes_) { @@ -382,6 +382,14 @@ void BCCWrapperImpl::DetachTracepoints() { tracepoints_.clear(); } +void BCCWrapperImpl::UnloadKFuncs() { + for (const auto& p : kfuncs_) { + auto res = UnloadKFunc(p); + LOG_IF(ERROR, !res.ok()) << res.msg(); + } + kfuncs_.clear(); +} + int BCCWrapperImpl::CommonPerfBufferSetup(const PerfBufferSpec& perf_buffer_spec) { DCHECK(perf_buffer_spec.cb_cookie != nullptr) << "perf_buffer_spec.cb_cookie must be non-null."; DCHECK(perf_buffer_spec.size_bytes > 0) << "perf_buffer_spec.cb_cookie must greater than zero."; @@ -500,6 +508,7 @@ void BCCWrapperImpl::Close() { DetachKProbes(); DetachUProbes(); DetachTracepoints(); + UnloadKFuncs(); } Status RecordingBCCWrapperImpl::OpenPerfBuffer(const PerfBufferSpec& perf_buffer_spec) { diff --git a/src/stirling/bpf_tools/bcc_wrapper.h b/src/stirling/bpf_tools/bcc_wrapper.h index 53909258c8f..031665c8152 100644 --- a/src/stirling/bpf_tools/bcc_wrapper.h +++ b/src/stirling/bpf_tools/bcc_wrapper.h @@ -333,14 +333,16 @@ class BCCWrapperImpl : public BCCWrapper { Status DetachUProbe(const UProbeSpec& probe); Status DetachTracepoint(const TracepointSpec& probe); Status DetachPerfEvent(const PerfEventSpec& perf_event); + Status UnloadKFunc(const KFuncSpec& probe); - // Detaches all kprobes/uprobes/perf buffers/perf events that were attached by the wrapper. + // Detaches all kprobes/uprobes/perf buffers/perf events/kfuncs that were attached by the wrapper. // If any fails to detach, an error is logged, and the function continues. void DetachKProbes(); void DetachUProbes(); void DetachTracepoints(); void ClosePerfBuffers(); void DetachPerfEvents(); + void UnloadKFuncs(); // Returns the name that identifies the target to attach this k-probe. std::string GetKProbeTargetName(const KProbeSpec& probe); diff --git a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c index ccfbbf7e765..fc93c1b01b4 100644 --- a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c +++ b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c @@ -98,16 +98,16 @@ static int tcp_sendstat(struct pt_regs* ctx, uint32_t tgid, uint32_t id, int siz return 0; } -int probe_ret_tcp_sendmsg(struct pt_regs* ctx) { - int size = PT_REGS_RC(ctx); - if (size > 0) { - uint64_t id = bpf_get_current_pid_tgid(); - uint32_t tgid = id >> 32; - return tcp_sendstat(ctx, tgid, id, size); - } else { - return 0; - } -} +// int probe_ret_tcp_sendmsg(struct pt_regs* ctx) { +// int size = PT_REGS_RC(ctx); +// if (size > 0) { +// uint64_t id = bpf_get_current_pid_tgid(); +// uint32_t tgid = id >> 32; +// return tcp_sendstat(ctx, tgid, id, size); +// } else { +// return 0; +// } +// } int probe_ret_tcp_sendpage(struct pt_regs* ctx) { int size = PT_REGS_RC(ctx); @@ -131,9 +131,9 @@ int probe_entry_tcp_sendpage(struct pt_regs* ctx, struct sock* sk, struct page* return tcp_send_entry(sk); } -int probe_entry_tcp_sendmsg(struct pt_regs* ctx, struct sock* sk, struct msghdr* msg, size_t size) { - return tcp_send_entry(sk); -} +// int probe_entry_tcp_sendmsg(struct pt_regs* ctx, struct sock* sk, struct msghdr* msg, size_t size) { +// return tcp_send_entry(sk); +// } int probe_entry_tcp_cleanup_rbuf(struct pt_regs* ctx, struct sock* sk, int copied) { if (copied <= 0) { @@ -231,12 +231,11 @@ KFUNC_PROBE(tcp_sendmsg, struct sock* sk) { return tcp_send_entry(sk); } -KRETFUNC_PROBE(tcp_sendmsg, struct pt_regs* regs) { - int size = PT_REGS_RC(regs); - if (size > 0) { +KRETFUNC_PROBE(tcp_sendmsg, struct sock* sk, struct msghdr* msg, size_t size, int ret) { + if (ret > 0) { uint64_t id = bpf_get_current_pid_tgid(); uint32_t tgid = id >> 32; - return tcp_sendstat(regs, tgid, id, size); + return tcp_sendstat(ctx, tgid, id, ret); } else { return 0; } diff --git a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc index 9ffd53881f3..010f9333ba7 100644 --- a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc +++ b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc @@ -49,8 +49,7 @@ const auto kSendPageProbeSpecs = MakeArray<bpf_tools::KProbeSpec>( {"tcp_sendpage", ProbeType::kReturn, "probe_ret_tcp_sendpage", /*is_syscall*/ false}}); // FIXME: Figure out whether to even create a KFuncSpec -// const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc____tcp_enter_loss"}, {"kretfunc__tcp_sendmsg"}}); -const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc__tcp_sendmsg"}, {"kretfunc__tcp_sendmsg"}}); +const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc__vmlinux__tcp_sendmsg"}, {"kretfunc__vmlinux__tcp_sendmsg"}}); void HandleTcpEvent(void* cb_cookie, void* data, int /*data_size*/) { auto* connector = reinterpret_cast<TCPStatsConnector*>(cb_cookie); @@ -85,13 +84,14 @@ Status TCPStatsConnector::InitImpl() { sendpage_attach_status.msg(), kernel_version.ToString()); } - // FIXME: Add comment here explaining the probe conflict with socket tracer. + // We deploy kfunc probes for the tcp_sendmsg function to side step a probe insertion conflict with + // the socket tracer since it already deploys kprobes for the tcp_sendmsg function. PX_RETURN_IF_ERROR(bcc_->AttachKFuncs(kFuncSpecs)); PX_RETURN_IF_ERROR(bcc_->OpenPerfBuffers(perf_buffer_specs)); LOG(INFO) << absl::Substitute("Successfully deployed $0 kprobes.", kProbeSpecs.size()); + LOG(INFO) << absl::Substitute("Successfully deployed $0 kfunc probes.", kFuncSpecs.size()); - // FIXME: Add a log to print the number of deployed kfunc probes. return Status::OK(); } From 3d4140ee17c98bec3375d6488673c0aa6e6dc547 Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Sat, 4 Jan 2025 01:10:11 +0000 Subject: [PATCH 4/8] Tidy. Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- .../tcp_stats/bcc_bpf/tcp_stats.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c index fc93c1b01b4..0d0248aeb1a 100644 --- a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c +++ b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c @@ -98,17 +98,6 @@ static int tcp_sendstat(struct pt_regs* ctx, uint32_t tgid, uint32_t id, int siz return 0; } -// int probe_ret_tcp_sendmsg(struct pt_regs* ctx) { -// int size = PT_REGS_RC(ctx); -// if (size > 0) { -// uint64_t id = bpf_get_current_pid_tgid(); -// uint32_t tgid = id >> 32; -// return tcp_sendstat(ctx, tgid, id, size); -// } else { -// return 0; -// } -// } - int probe_ret_tcp_sendpage(struct pt_regs* ctx) { int size = PT_REGS_RC(ctx); if (size > 0) { @@ -131,10 +120,6 @@ int probe_entry_tcp_sendpage(struct pt_regs* ctx, struct sock* sk, struct page* return tcp_send_entry(sk); } -// int probe_entry_tcp_sendmsg(struct pt_regs* ctx, struct sock* sk, struct msghdr* msg, size_t size) { -// return tcp_send_entry(sk); -// } - int probe_entry_tcp_cleanup_rbuf(struct pt_regs* ctx, struct sock* sk, int copied) { if (copied <= 0) { return 0; @@ -235,7 +220,7 @@ KRETFUNC_PROBE(tcp_sendmsg, struct sock* sk, struct msghdr* msg, size_t size, in if (ret > 0) { uint64_t id = bpf_get_current_pid_tgid(); uint32_t tgid = id >> 32; - return tcp_sendstat(ctx, tgid, id, ret); + return tcp_sendstat((struct pt_regs*)(ctx), tgid, id, ret); } else { return 0; } From ffcb6740cfcefff125c355288ca276745e85cb2d Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Sat, 4 Jan 2025 04:19:08 +0000 Subject: [PATCH 5/8] Tidy Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc index 010f9333ba7..d03aa935f92 100644 --- a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc +++ b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc @@ -48,7 +48,6 @@ const auto kSendPageProbeSpecs = MakeArray<bpf_tools::KProbeSpec>( {{"tcp_sendpage", ProbeType::kEntry, "probe_entry_tcp_sendpage", /*is_syscall*/ false}, {"tcp_sendpage", ProbeType::kReturn, "probe_ret_tcp_sendpage", /*is_syscall*/ false}}); -// FIXME: Figure out whether to even create a KFuncSpec const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc__vmlinux__tcp_sendmsg"}, {"kretfunc__vmlinux__tcp_sendmsg"}}); void HandleTcpEvent(void* cb_cookie, void* data, int /*data_size*/) { From edf06fa41f5041f7524fae408a67639c8a98f887 Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Sun, 5 Jan 2025 21:11:51 +0000 Subject: [PATCH 6/8] lint Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/bpf_tools/probe_specs/probe_specs.h | 4 +--- .../source_connectors/tcp_stats/bcc_bpf/tcp_stats.c | 4 +--- .../source_connectors/tcp_stats/tcp_stats_connector.cc | 7 ++++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/stirling/bpf_tools/probe_specs/probe_specs.h b/src/stirling/bpf_tools/probe_specs/probe_specs.h index b14b649f65f..d5eb833bc8f 100644 --- a/src/stirling/bpf_tools/probe_specs/probe_specs.h +++ b/src/stirling/bpf_tools/probe_specs/probe_specs.h @@ -144,9 +144,7 @@ struct TracepointSpec { struct KFuncSpec { std::string kfunc; - std::string ToString() const { - return absl::Substitute("[kfunc=$0]", kfunc); - } + std::string ToString() const { return absl::Substitute("[kfunc=$0]", kfunc); } }; /** diff --git a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c index 0d0248aeb1a..d25f1eb9dbe 100644 --- a/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c +++ b/src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c @@ -212,9 +212,7 @@ int probe_entry_tcp_retransmit_skb(struct pt_regs* ctx, struct sock* skp, struct return 0; } -KFUNC_PROBE(tcp_sendmsg, struct sock* sk) { - return tcp_send_entry(sk); -} +KFUNC_PROBE(tcp_sendmsg, struct sock* sk) { return tcp_send_entry(sk); } KRETFUNC_PROBE(tcp_sendmsg, struct sock* sk, struct msghdr* msg, size_t size, int ret) { if (ret > 0) { diff --git a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc index d03aa935f92..e8ea22ee5c2 100644 --- a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc +++ b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc @@ -48,7 +48,8 @@ const auto kSendPageProbeSpecs = MakeArray<bpf_tools::KProbeSpec>( {{"tcp_sendpage", ProbeType::kEntry, "probe_entry_tcp_sendpage", /*is_syscall*/ false}, {"tcp_sendpage", ProbeType::kReturn, "probe_ret_tcp_sendpage", /*is_syscall*/ false}}); -const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>({{"kfunc__vmlinux__tcp_sendmsg"}, {"kretfunc__vmlinux__tcp_sendmsg"}}); +const auto kFuncSpecs = MakeArray<bpf_tools::KFuncSpec>( + {{"kfunc__vmlinux__tcp_sendmsg"}, {"kretfunc__vmlinux__tcp_sendmsg"}}); void HandleTcpEvent(void* cb_cookie, void* data, int /*data_size*/) { auto* connector = reinterpret_cast<TCPStatsConnector*>(cb_cookie); @@ -83,8 +84,8 @@ Status TCPStatsConnector::InitImpl() { sendpage_attach_status.msg(), kernel_version.ToString()); } - // We deploy kfunc probes for the tcp_sendmsg function to side step a probe insertion conflict with - // the socket tracer since it already deploys kprobes for the tcp_sendmsg function. + // We deploy kfunc probes for the tcp_sendmsg function to side step a probe insertion conflict + // with the socket tracer since it already deploys kprobes for the tcp_sendmsg function. PX_RETURN_IF_ERROR(bcc_->AttachKFuncs(kFuncSpecs)); PX_RETURN_IF_ERROR(bcc_->OpenPerfBuffers(perf_buffer_specs)); From e478ecfb19ebdb3f1ad15ffa7d39112e65818b6a Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Mon, 6 Jan 2025 22:58:00 +0000 Subject: [PATCH 7/8] Add kfunc methods to ReplayingBCCWrapperImpl Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- src/stirling/bpf_tools/bcc_wrapper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/stirling/bpf_tools/bcc_wrapper.h b/src/stirling/bpf_tools/bcc_wrapper.h index 031665c8152..6d39a798cef 100644 --- a/src/stirling/bpf_tools/bcc_wrapper.h +++ b/src/stirling/bpf_tools/bcc_wrapper.h @@ -415,11 +415,13 @@ class ReplayingBCCWrapperImpl : public BCCWrapper { Status AttachUProbe(const UProbeSpec&) override { return Status::OK(); } Status AttachTracepoint(const TracepointSpec&) override { return Status::OK(); } Status AttachSamplingProbe(const SamplingProbeSpec&) override { return Status::OK(); } + Status AttachKFunc(const KFuncSpec&) override { return Status::OK(); } Status AttachPerfEvent(const PerfEventSpec&) override { return Status::OK(); } Status AttachKProbes(const ArrayView<KProbeSpec>&) override { return Status::OK(); } Status AttachTracepoints(const ArrayView<TracepointSpec>&) override { return Status::OK(); } Status AttachUProbes(const ArrayView<UProbeSpec>&) override { return Status::OK(); } Status AttachSamplingProbes(const ArrayView<SamplingProbeSpec>&) override { return Status::OK(); } + Status AttachKFuncs(const ArrayView<KFuncSpec>&) override { return Status::OK(); } Status AttachXDP(const std::string&, const std::string&) override { return Status::OK(); } Status AttachPerfEvents(const ArrayView<PerfEventSpec>&) override { return Status::OK(); } Status PopulateBPFPerfArray(const std::string&, const uint32_t, const uint64_t) override { From 8d77b866744d09636471ec7a4ceb5d544d6c4e87 Mon Sep 17 00:00:00 2001 From: kpattaswamy <kpattaswamy@pixielabs.ai> Date: Tue, 7 Jan 2025 03:25:32 +0000 Subject: [PATCH 8/8] Log when kfunc fails to attach on an older kernel Signed-off-by: kpattaswamy <kpattaswamy@pixielabs.ai> --- .../tcp_stats/tcp_stats_connector.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc index e8ea22ee5c2..425996f9fc7 100644 --- a/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc +++ b/src/stirling/source_connectors/tcp_stats/tcp_stats_connector.cc @@ -86,11 +86,19 @@ Status TCPStatsConnector::InitImpl() { // We deploy kfunc probes for the tcp_sendmsg function to side step a probe insertion conflict // with the socket tracer since it already deploys kprobes for the tcp_sendmsg function. - PX_RETURN_IF_ERROR(bcc_->AttachKFuncs(kFuncSpecs)); + const auto kfunc_attach_status = bcc_->AttachKFuncs(kFuncSpecs); + if (!kfunc_attach_status.ok()) { + const auto kernel_version = system::GetCachedKernelVersion(); + LOG(INFO) << absl::Substitute( + "Could not attach kfunc probes for tcp_sendmsg: $0, detected kernel version: $1. Note: " + "kfunc probes are supported on kernel version 5.5 and newer.", + kfunc_attach_status.msg(), kernel_version.ToString()); + } else { + LOG(INFO) << absl::Substitute("Successfully deployed $0 kfunc probes.", kFuncSpecs.size()); + } PX_RETURN_IF_ERROR(bcc_->OpenPerfBuffers(perf_buffer_specs)); LOG(INFO) << absl::Substitute("Successfully deployed $0 kprobes.", kProbeSpecs.size()); - LOG(INFO) << absl::Substitute("Successfully deployed $0 kfunc probes.", kFuncSpecs.size()); return Status::OK(); }