Skip to content

Commit 3a7e3fe

Browse files
ddelnanoddelnano
authored
Fix Go 1.24 tracing issues caused by http2bufferedWriter struct changes (#2223)
Summary: Fix Go 1.24 tracing issues caused by `http2bufferedWriter` struct changes This PR addresses the Go 1.24 http2 tracing issues caused by the removal of the http2bufferedWriter w (io.Writer) member. This change updates the BPF tracing code to handle the structural changes in Go 1.24's HTTP/2 implementation, adds test coverage for Go 1.24, and bumps the Go minor versions across the codebase. Relevant Issues: N/A Type of change: /kind bugfix Test Plan: Newly added Go 1.24 test cases pass with changes from commit 3 Changelog Message: Fixed compatibility issues with Go 1.24's internal HTTP/2 implementation changes. --------- Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
1 parent 3cc2cc1 commit 3a7e3fe

File tree

18 files changed

+264
-28
lines changed

18 files changed

+264
-28
lines changed

‎WORKSPACE‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pl_go_overrides()
3333

3434
go_download_sdk(
3535
name = "go_sdk",
36-
version = "1.24.0",
36+
version = "1.24.4",
3737
)
3838

3939
go_rules_dependencies()
@@ -238,7 +238,7 @@ go_download_sdk(
238238

239239
go_download_sdk(
240240
name = "go_sdk_1_23",
241-
version = "1.23.6",
241+
version = "1.23.10",
242242
)
243243

244244
# The go_sdk_boringcrypto SDK is used for testing boringcrypto specific functionality (TLS tracing).
@@ -251,7 +251,7 @@ go_download_sdk(
251251
go_download_sdk(
252252
name = "go_sdk_boringcrypto",
253253
experiments = ["boringcrypto"],
254-
version = "1.23.5",
254+
version = "1.23.9",
255255
)
256256

257257
pip_parse(

‎bazel/pl_build_system.bzl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
2323
load("@rules_python//python:defs.bzl", "py_test")
2424
load("//bazel:toolchain_transitions.bzl", "qemu_interactive_runner")
2525

26-
pl_boringcrypto_go_sdk = ["1.23.5"]
27-
pl_supported_go_sdk_versions = ["1.18", "1.19", "1.20", "1.21", "1.22", "1.23"]
26+
pl_boringcrypto_go_sdk = ["1.23.9"]
27+
pl_supported_go_sdk_versions = ["1.18", "1.19", "1.20", "1.21", "1.22", "1.23", "1.24"]
2828

2929
# The last version in this list corresponds to the boringcrypto go sdk version.
3030
pl_all_supported_go_sdk_versions = pl_supported_go_sdk_versions + pl_boringcrypto_go_sdk

‎docker.properties‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
DOCKER_IMAGE_TAG=202504142133
2-
LINTER_IMAGE_DIGEST=0129dd524203f95a25f4343ec4499919db4434752375624a4cdbd51d463acdaf
3-
DEV_IMAGE_DIGEST=f669bf0bc9db3ce03a48365a41e87de1a8e3e9be01bc5a1e10816412c671665e
4-
DEV_IMAGE_WITH_EXTRAS_DIGEST=65535207f2fb805d45bb7997cf0a71abbd756cf8763db02c57838f8ee18f0c66
1+
DOCKER_IMAGE_TAG=202506270326
2+
LINTER_IMAGE_DIGEST=05aeeb210dec4b5753e55376aef7df013bb136a3ad70524fa1dff24f832b5c4e
3+
DEV_IMAGE_DIGEST=f0a0c803733d6ad8c9104f745378ab9d7a95263a3f8882fc28f11c79c1200d1f
4+
DEV_IMAGE_WITH_EXTRAS_DIGEST=17c8ce9c4bd4dba40cdd6e40bdee726237336dbd6581deca864ebe9302cff569

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module px.dev/pixie
22

3-
go 1.24.0
3+
go 1.24.4
44

55
require (
66
cloud.google.com/go v0.81.0

‎src/stirling/source_connectors/socket_tracer/BUILD.bazel‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,13 +310,15 @@ pl_cc_bpf_test(
310310
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_21_grpc_client",
311311
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_22_grpc_client",
312312
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_23_grpc_client",
313+
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_24_grpc_client",
313314
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_boringcrypto_grpc_client",
314315
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_18_grpc_server_with_certs",
315316
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs",
316317
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_20_grpc_server_with_certs",
317318
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_21_grpc_server_with_certs",
318319
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_22_grpc_server_with_certs",
319320
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_23_grpc_server_with_certs",
321+
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_24_grpc_server_with_certs",
320322
"//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_boringcrypto_grpc_server_with_certs",
321323
],
322324
flaky = True,
@@ -364,6 +366,8 @@ pl_cc_bpf_test(
364366
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_22_grpc_server_container",
365367
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_23_grpc_client_container",
366368
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_23_grpc_server_container",
369+
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_24_grpc_client_container",
370+
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_24_grpc_server_container",
367371
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_boringcrypto_grpc_client_container",
368372
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_boringcrypto_grpc_server_container",
369373
"//src/stirling/source_connectors/socket_tracer/testing/container_images:product_catalog_client_container",
@@ -586,6 +590,8 @@ pl_cc_bpf_test(
586590
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_22_tls_server_container",
587591
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_23_tls_client_container",
588592
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_23_tls_server_container",
593+
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_24_tls_client_container",
594+
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_1_24_tls_server_container",
589595
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_boringcrypto_tls_client_container",
590596
"//src/stirling/source_connectors/socket_tracer/testing/container_images:go_boringcrypto_tls_server_container",
591597
"//src/stirling/testing:cc_library",

‎src/stirling/source_connectors/socket_tracer/bcc_bpf/go_http2_trace.c‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,19 @@ static __inline int32_t get_fd_from_http2_Framer(const void* framer_ptr,
141141
static __inline int32_t get_fd_from_http_http2Framer(const void* framer_ptr,
142142
const struct go_http2_symaddrs_t* symaddrs) {
143143
REQUIRE_SYMADDR(symaddrs->http2Framer_w_offset, kInvalidFD);
144-
REQUIRE_SYMADDR(symaddrs->http2bufferedWriter_w_offset, kInvalidFD);
144+
int32_t inner_intf_offset = symaddrs->http2bufferedWriter_w_offset;
145+
bool conn_intf = false;
146+
// Go 1.24 dropped the io.Writer w member from http2bufferedWriter,
147+
// in favor of a conn member (net.Conn interface).
148+
// Go 1.23 struct:
149+
// https://github.com/golang/go/blob/d375ae50633cdf1cd8536f2a199c382f9053b638/src/net/http/h2_bundle.go#L3552-L3556
150+
// Go 1.24 struct:
151+
// https://github.com/golang/go/blob/3901409b5d0fb7c85a3e6730a59943cc93b2835c/src/net/http/h2_bundle.go#L3733-L3739
152+
if (inner_intf_offset == -1) {
153+
inner_intf_offset = symaddrs->http2bufferedWriter_conn_offset;
154+
conn_intf = true;
155+
}
156+
REQUIRE_SYMADDR(inner_intf_offset, kInvalidFD);
145157

146158
struct go_interface io_writer_interface;
147159
BPF_PROBE_READ_VAR(io_writer_interface, framer_ptr + symaddrs->http2Framer_w_offset);
@@ -152,11 +164,13 @@ static __inline int32_t get_fd_from_http_http2Framer(const void* framer_ptr,
152164
return kInvalidFD;
153165
}
154166

155-
struct go_interface inner_io_writer_interface;
156-
BPF_PROBE_READ_VAR(inner_io_writer_interface,
157-
io_writer_interface.ptr + symaddrs->http2bufferedWriter_w_offset);
167+
struct go_interface inner_intf;
168+
BPF_PROBE_READ_VAR(inner_intf, io_writer_interface.ptr + inner_intf_offset);
158169

159-
return get_fd_from_io_writer_intf(inner_io_writer_interface.ptr);
170+
if (conn_intf) {
171+
return get_fd_from_conn_intf(inner_intf);
172+
}
173+
return get_fd_from_io_writer_intf(inner_intf.ptr);
160174
}
161175

162176
//-----------------------------------------------------------------------------
@@ -525,7 +539,7 @@ int probe_http2_server_operate_headers(struct pt_regs* ctx) {
525539
// Symbol:
526540
// net/http.(*http2serverConn).processHeaders
527541
//
528-
// Verified to be stable from go1.?? to t go.1.13.
542+
// Verified to be stable from go1.?? to go.1.24..
529543
int probe_http_http2serverConn_processHeaders(struct pt_regs* ctx) {
530544
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
531545
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -629,7 +643,7 @@ int probe_http_http2serverConn_processHeaders(struct pt_regs* ctx) {
629643
// Symbol:
630644
// golang.org/x/net/http2/hpack.(*Encoder).WriteField
631645
//
632-
// Verified to be stable from at least go1.6 to t go.1.13.
646+
// Verified to be stable from at least go1.6 to go.1.24.
633647
int probe_hpack_header_encoder(struct pt_regs* ctx) {
634648
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
635649
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -679,7 +693,7 @@ int probe_hpack_header_encoder(struct pt_regs* ctx) {
679693
// Symbol:
680694
// net/http.(*http2writeResHeaders).writeFrame
681695
//
682-
// Verified to be stable from go1.?? to t go.1.13.
696+
// Verified to be stable from go1.?? to go.1.24.
683697
int probe_http_http2writeResHeaders_write_frame(struct pt_regs* ctx) {
684698
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
685699
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -857,7 +871,7 @@ static __inline void go_http2_submit_data(struct pt_regs* ctx, enum http2_probe_
857871
// Symbol:
858872
// golang.org/x/net/http2.(*Framer).checkFrameOrder
859873
//
860-
// Verified to be stable from at least go1.6 to t go.1.13.
874+
// Verified to be stable from at least go1.6 to go.1.24.
861875
int probe_http2_framer_check_frame_order(struct pt_regs* ctx) {
862876
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
863877
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -962,7 +976,7 @@ int probe_http2_framer_check_frame_order(struct pt_regs* ctx) {
962976
// Symbol:
963977
// net/http.(*http2Framer).checkFrameOrder
964978
//
965-
// Verified to be stable from at least go1.?? to go.1.13.
979+
// Verified to be stable from at least go1.?? to go.1.24.
966980
int probe_http_http2framer_check_frame_order(struct pt_regs* ctx) {
967981
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
968982
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -1066,7 +1080,7 @@ int probe_http_http2framer_check_frame_order(struct pt_regs* ctx) {
10661080
// Symbol:
10671081
// golang.org/x/net/http2.(*Framer).WriteDataPadded
10681082
//
1069-
// Verified to be stable from go1.7 to t go.1.13.
1083+
// Verified to be stable from go1.7 to go.1.24.
10701084
int probe_http2_framer_write_data(struct pt_regs* ctx) {
10711085
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
10721086
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);
@@ -1134,7 +1148,7 @@ int probe_http2_framer_write_data(struct pt_regs* ctx) {
11341148
// Symbol:
11351149
// net/http.(*http2Framer).WriteDataPadded
11361150
//
1137-
// Verified to be stable from go1.?? to t go.1.13.
1151+
// Verified to be stable from go1.?? to go.1.24.
11381152
int probe_http_http2framer_write_data(struct pt_regs* ctx) {
11391153
uint32_t tgid = bpf_get_current_pid_tgid() >> 32;
11401154
struct go_http2_symaddrs_t* symaddrs = http2_symaddrs_map.lookup(&tgid);

‎src/stirling/source_connectors/socket_tracer/bcc_bpf/go_trace_common.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,25 @@ static __inline int32_t get_fd_from_conn_intf_core(struct go_interface conn_intf
174174
REQUIRE_SYMADDR(symaddrs->FD_Sysfd_offset, kInvalidFD);
175175

176176
if (conn_intf.type == symaddrs->internal_syscallConn) {
177+
// TODO(ddelnano): The 4.14 verifier has stricter bounds checking limits when reading memory
178+
// offsets. Without this check, the verifier rejects the program. This can be removed when 4.14
179+
// kernel support is dropped.
180+
if (symaddrs->syscallConn_conn_offset < 0 || symaddrs->syscallConn_conn_offset > 1024) {
181+
return kInvalidFD;
182+
}
177183
REQUIRE_SYMADDR(symaddrs->syscallConn_conn_offset, kInvalidFD);
178184
const int kSyscallConnConnOffset = 0;
179185
bpf_probe_read(&conn_intf, sizeof(conn_intf),
180186
conn_intf.ptr + symaddrs->syscallConn_conn_offset);
181187
}
182188

183189
if (conn_intf.type == symaddrs->tls_Conn) {
190+
// TODO(ddelnano): The 4.14 verifier has stricter bounds checking limits when reading memory
191+
// offsets. Without this check, the verifier rejects the program. This can be removed when 4.14
192+
// kernel support is dropped.
193+
if (symaddrs->tlsConn_conn_offset < 0 || symaddrs->tlsConn_conn_offset > 1024) {
194+
return kInvalidFD;
195+
}
184196
REQUIRE_SYMADDR(symaddrs->tlsConn_conn_offset, kInvalidFD);
185197
bpf_probe_read(&conn_intf, sizeof(conn_intf), conn_intf.ptr + symaddrs->tlsConn_conn_offset);
186198
}

‎src/stirling/source_connectors/socket_tracer/bcc_bpf_intf/symaddrs.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ struct go_http2_symaddrs_t {
235235

236236
// Members of net/http.http2bufferedWriter
237237
int32_t http2bufferedWriter_w_offset; // 0
238+
// Go 1.24 switched from a w io.Writer member to a conn net.Conn one.
239+
int32_t http2bufferedWriter_conn_offset;
238240
};
239241

240242
struct go_tls_symaddrs_t {

‎src/stirling/source_connectors/socket_tracer/go_tls_trace_bpf_test.cc‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_1_22_tls_server_container.h"
3333
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_1_23_tls_client_container.h"
3434
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_1_23_tls_server_container.h"
35+
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_1_24_tls_client_container.h"
36+
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_1_24_tls_server_container.h"
3537
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_boringcrypto_tls_client_container.h"
3638
#include "src/stirling/source_connectors/socket_tracer/testing/container_images/go_boringcrypto_tls_server_container.h"
3739
#include "src/stirling/source_connectors/socket_tracer/testing/protocol_checkers.h"
@@ -101,6 +103,11 @@ struct Go1_23TLSClientServerContainers {
101103
using GoTLSClientContainer = ::px::stirling::testing::Go1_23_TLSClientContainer;
102104
};
103105

106+
struct Go1_24TLSClientServerContainers {
107+
using GoTLSServerContainer = ::px::stirling::testing::Go1_24_TLSServerContainer;
108+
using GoTLSClientContainer = ::px::stirling::testing::Go1_24_TLSClientContainer;
109+
};
110+
104111
struct GoBoringCryptoTLSClientServerContainers {
105112
using GoTLSServerContainer = ::px::stirling::testing::GoBoringCryptoTLSServerContainer;
106113
using GoTLSClientContainer = ::px::stirling::testing::GoBoringCryptoTLSClientContainer;
@@ -109,7 +116,7 @@ struct GoBoringCryptoTLSClientServerContainers {
109116
typedef ::testing::Types<GoBoringCryptoTLSClientServerContainers, Go1_18TLSClientServerContainers,
110117
Go1_19TLSClientServerContainers, Go1_20TLSClientServerContainers,
111118
Go1_21TLSClientServerContainers, Go1_22TLSClientServerContainers,
112-
Go1_23TLSClientServerContainers>
119+
Go1_23TLSClientServerContainers, Go1_24TLSClientServerContainers>
113120
GoVersions;
114121
TYPED_TEST_SUITE(GoTLSTraceTest, GoVersions);
115122

‎src/stirling/source_connectors/socket_tracer/grpc_trace_bpf_test.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ INSTANTIATE_TEST_SUITE_P(SecurityModeTest, GRPCTraceTest,
196196
TestParams{"1_21", true, true}, TestParams{"1_21", true, false},
197197
TestParams{"1_22", true, true}, TestParams{"1_22", true, false},
198198
TestParams{"1_23", true, true}, TestParams{"1_23", true, false},
199+
TestParams{"1_24", true, true}, TestParams{"1_24", true, false},
199200
TestParams{"boringcrypto", true, true}));
200201

201202
class PyGRPCTraceTest : public testing::SocketTraceBPFTestFixture</* TClientSideTracing */ false> {

0 commit comments

Comments
 (0)