Skip to content

Commit 222342b

Browse files
mmat11mmat11
committed
debug
1 parent ff86594 commit 222342b

File tree

6 files changed

+133
-27
lines changed

6 files changed

+133
-27
lines changed

‎.github/workflows/cx-internal-build-push.yml‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
name: CX build and push
2+
3+
on:
4+
push:
5+
branches: [ 'main', 'matt/bpf-loop-fallback' ]
6+
pull_request:
7+
branches: [ 'matt/bpf-loop-fallback' ]
8+
9+
env:
10+
AWS_REGION: eu-west-1
11+
AWS_ACCOUNT_ID: 897729105761 # tacotaco-research
12+
AWS_ROLE_TO_ASSUME: arn:aws:iam::897729105761:role/AnemiaAgentRole
13+
14+
jobs:
15+
build-and-push:
16+
runs-on: ubuntu-latest
17+
if: github.repository == 'coralogix/opentelemetry-ebpf-instrumentation'
18+
permissions:
19+
id-token: write
20+
packages: write
21+
contents: read
22+
23+
steps:
24+
- name: Checkout
25+
uses: actions/checkout@v3
26+
27+
- name: Configure AWS credentials
28+
uses: aws-actions/configure-aws-credentials@v4
29+
with:
30+
role-to-assume: ${{ env.AWS_ROLE_TO_ASSUME }}
31+
aws-region: ${{ env.AWS_REGION }}
32+
33+
- name: Login to Amazon ECR
34+
id: login-ecr
35+
run: |
36+
aws ecr get-login-password --region ${{ env.AWS_REGION }} | docker login --username AWS --password-stdin ${{ env.AWS_ACCOUNT_ID }}.dkr.ecr.${{ env.AWS_REGION }}.amazonaws.com
37+
38+
- name: Create ECR Repositories
39+
run: |
40+
aws ecr create-repository --repository-name "obi-internal" --region ${{ env.AWS_REGION }} || echo "Repository obi-internal already exists."
41+
42+
- name: Set up buildx
43+
uses: docker/setup-buildx-action@v3
44+
with:
45+
driver: docker-container
46+
47+
- name: Cache images
48+
uses: actions/cache@v3
49+
with:
50+
path: /tmp/.buildx-cache
51+
key: ${{ runner.os }}-buildx-${{ github.sha }}
52+
restore-keys: |
53+
${{ runner.os }}-buildx-
54+
55+
- name: Set Git Commit Hash
56+
run: echo "COMMIT_HASH=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
57+
58+
- name: Build and push
59+
uses: docker/build-push-action@v6
60+
with:
61+
platforms: linux/amd64
62+
context: .
63+
file: ./Dockerfile
64+
push: true
65+
tags: |
66+
${{ env.AWS_ACCOUNT_ID }}.dkr.ecr.${{ env.AWS_REGION }}.amazonaws.com/obi-internal:${{ env.COMMIT_HASH }}
67+
${{ env.AWS_ACCOUNT_ID }}.dkr.ecr.${{ env.AWS_REGION }}.amazonaws.com/obi-internal:latest
68+
cache-from: type=local,src=/tmp/.buildx-cache
69+
cache-to: type=local,dest=/tmp/.buildx-cache,new=true

‎bpf/common/trace_common.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ static __always_inline void trace_key_from_pid_tid(trace_key_t *t_key) {
5555
t_key->extra_id = extra_id;
5656
}
5757

58-
static __hidden int tp_match(u32 index, void *data) {
58+
static int tp_match(u32 index, void *data) {
5959
if (index >= (TRACE_BUF_SIZE - TRACE_PARENT_HEADER_LEN)) {
6060
return 1;
6161
}
@@ -89,14 +89,15 @@ static __always_inline unsigned char *bpf_strstr_tp_loop(unsigned char *buf, int
8989
return NULL;
9090
}
9191

92-
static __always_inline unsigned char *bpf_strstr_tp_loop__legacy(unsigned char *buf) {
92+
static __always_inline unsigned char *bpf_strstr_tp_loop__legacy(unsigned char *buf, int buf_len) {
93+
(void)buf_len;
94+
9395
if (!k_bpf_traceparent_enabled) {
9496
return NULL;
9597
}
9698

97-
const u16 k_besteffort_max_loops = 450; // Limit search to avoid burning too many instructions
99+
const u16 k_besteffort_max_loops = 450; // Limited best-effort search to stay within insns limit
98100
for (u16 i = 0; i + TRACE_PARENT_HEADER_LEN < k_besteffort_max_loops; i++) {
99-
// Only check at offset=0 or after '\n' to save instructions
100101
if (i != 0 && buf[i - 1] != '\n') {
101102
continue;
102103
}

‎bpf/common/trace_util.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,6 @@ static __always_inline bool is_traceparent(const unsigned char *p) {
6868
(p[8] == 'e') && (p[9] == 'n') && (p[10] == 't') && (p[11] == ':') && (p[12] == ' ')) {
6969
return true;
7070
}
71+
7172
return false;
7273
}

‎bpf/generictracer/protocol_http.h‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,16 @@ static __always_inline u32 trace_type_from_meta(http_connection_metadata_t *meta
4040
return TRACE_TYPE_SERVER;
4141
}
4242

43-
static __always_inline void http_get_or_create_trace_info(http_connection_metadata_t *meta,
44-
u32 pid,
45-
connection_info_t *conn,
46-
void *u_buf,
47-
int bytes_len,
48-
s32 capture_header_buffer,
49-
u8 ssl,
50-
u16 orig_dport) {
43+
static __always_inline void
44+
http_get_or_create_trace_info(http_connection_metadata_t *meta,
45+
u32 pid,
46+
connection_info_t *conn,
47+
void *u_buf,
48+
int bytes_len,
49+
s32 capture_header_buffer,
50+
u8 ssl,
51+
u16 orig_dport,
52+
unsigned char *(*tp_loop_fn)(unsigned char *, int)) {
5153
//TODO use make_key
5254
egress_key_t e_key = {
5355
.d_port = conn->d_port,
@@ -148,13 +150,7 @@ static __always_inline void http_get_or_create_trace_info(http_connection_metada
148150

149151
bpf_probe_read(buf, buf_len, u_buf);
150152

151-
unsigned char *res = NULL;
152-
if (bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_loop)) {
153-
res = bpf_strstr_tp_loop(buf, buf_len);
154-
} else {
155-
res = bpf_strstr_tp_loop__legacy(buf);
156-
}
157-
153+
unsigned char *res = tp_loop_fn(buf, buf_len);
158154
if (res) {
159155
bpf_dbg_printk("Found traceparent in headers [%s] overriding what was before", res);
160156
unsigned char *t_id = extract_trace_id(res);
@@ -392,13 +388,8 @@ static __always_inline void handle_http_response(unsigned char *small_buf,
392388
cleanup_http_request_data(pid_conn, info);
393389
}
394390

395-
// k_tail_protocol_http
396-
SEC("kprobe/http")
397-
int obi_protocol_http(void *ctx) {
398-
(void)ctx;
399-
391+
static __always_inline int __obi_protocol_http(unsigned char *(*tp_loop_fn)(unsigned char *, int)) {
400392
call_protocol_args_t *args = protocol_args();
401-
402393
if (!args) {
403394
return 0;
404395
}
@@ -450,7 +441,8 @@ int obi_protocol_http(void *ctx) {
450441
args->bytes_len,
451442
capture_header_buffer,
452443
args->ssl,
453-
args->orig_dport);
444+
args->orig_dport,
445+
tp_loop_fn);
454446

455447
if (meta) {
456448
u32 type = trace_type_from_meta(meta);
@@ -483,3 +475,17 @@ int obi_protocol_http(void *ctx) {
483475

484476
return 0;
485477
}
478+
479+
// k_tail_protocol_http
480+
SEC("kprobe/http")
481+
static int obi_protocol_http(void *ctx) {
482+
(void)ctx;
483+
return __obi_protocol_http(bpf_strstr_tp_loop);
484+
}
485+
486+
// k_tail_protocol_http
487+
SEC("kprobe/http")
488+
static int obi_protocol_http_legacy(void *ctx) {
489+
(void)ctx;
490+
return __obi_protocol_http(bpf_strstr_tp_loop__legacy);
491+
}

‎pkg/components/ebpf/common/common.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"unsafe"
1818

1919
"github.com/cilium/ebpf"
20+
"github.com/cilium/ebpf/asm"
2021
"github.com/cilium/ebpf/link"
2122
lru "github.com/hashicorp/golang-lru/v2"
2223
"github.com/hashicorp/golang-lru/v2/expirable"
@@ -301,6 +302,7 @@ func SupportsContextPropagationWithProbe(log *slog.Logger) bool {
301302
return false
302303
}
303304

305+
// TODO(matt): probe for BPF_FUNC_loop instead of relying on kernel version?
304306
func SupportsEBPFLoops(log *slog.Logger, overrideKernelVersion bool) bool {
305307
if overrideKernelVersion {
306308
log.Debug("Skipping kernel version check for bpf_loop functionality: user supplied confirmation of support")
@@ -310,6 +312,26 @@ func SupportsEBPFLoops(log *slog.Logger, overrideKernelVersion bool) bool {
310312
return kernelMajor > 5 || (kernelMajor == 5 && kernelMinor >= 17)
311313
}
312314

315+
func FixupSpec(spec *ebpf.CollectionSpec, overrideKernelVersion bool) {
316+
if !SupportsEBPFLoops(ptlog(), overrideKernelVersion) {
317+
// Hack: instead of redefining bpf2go generated struct for mutually exclusive conditional programs,
318+
// use one predefined field name to store either of them.
319+
spec.Programs["obi_protocol_http"] = spec.Programs["obi_protocol_http_legacy"]
320+
spec.Programs["obi_protocol_http"].Name = "obi_protocol_http"
321+
}
322+
// Hack: insert a dummy unused program in order to be able to use bpf2go generated struct to load
323+
// the collection.
324+
spec.Programs["obi_protocol_http_legacy"] = &ebpf.ProgramSpec{
325+
Name: "obi_dummy",
326+
Type: ebpf.Kprobe,
327+
Instructions: asm.Instructions{
328+
asm.Mov.Imm(asm.R0, 0),
329+
asm.Return(),
330+
},
331+
License: "MIT",
332+
}
333+
}
334+
313335
// Injectable for tests
314336
var lockdownPath = "/sys/kernel/security/lockdown"
315337

‎pkg/components/ebpf/generictracer/generictracer.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,14 @@ func (p *Tracer) Load() (*ebpf.CollectionSpec, error) {
149149
p.log.Info("Enabling trace information parsing", "bpf_loop_enabled", ebpfcommon.SupportsEBPFLoops(p.log, p.cfg.EBPF.OverrideBPFLoopEnabled))
150150
}
151151

152-
return loader()
152+
spec, err := loader()
153+
if err != nil {
154+
return nil, fmt.Errorf("can't load bpf collection from reader: %v", err)
155+
}
156+
157+
ebpfcommon.FixupSpec(spec, p.cfg.EBPF.OverrideBPFLoopEnabled)
158+
159+
return spec, err
153160
}
154161

155162
func (p *Tracer) SetupTailCalls() {

0 commit comments

Comments
 (0)