Tcp long connections metrics #1249

yp969803 · 2025-02-24T13:01:24Z

What type of PR is this?

/kind feature

What this PR does / why we need it:
The pr introduces new feature of tcp_long_conn metrics
Which issue(s) this PR fixes:

Fixes #1211

Special notes for your reviewer:

Does this PR introduce a user-facing change?:
Yes

Tcp long connections metrics

kmesh-bot · 2025-02-24T13:01:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nlgwcy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

yp969803 · 2025-02-24T13:26:39Z

@nlgwcy @LemmyHuang @xiangxinyong can you review the pr, have i writen correct ebpf code, i have done the changes which has been told in previous comments in the proposal.

LiZhenCheng9527

Please present the results of the long connection metrics
The original destination address problem is still unhandle. If you plan to optimize later, you can create a sub-issue under the existing lfx issue to track the task.

LiZhenCheng9527 · 2025-02-26T06:22:34Z

pkg/controller/telemetry/metric.go

@@ -55,6 +55,7 @@ type MetricController struct {
 	EnableAccesslog      atomic.Bool
 	EnableMonitoring     atomic.Bool
 	EnableWorkloadMetric atomic.Bool
+	EnableLongTCPMetric  atomic.Bool


I don't think it's necessary to distinguish between metrics for long connections and short connections.
As long as metrics is turned on, we should handle all types of connections.

LiZhenCheng9527 · 2025-02-26T06:29:28Z

bpf/kmesh/workload/include/config.h

+#define kmesh_perf_map           km_perf_map
+#define kmesh_perf_info          km_perf_info
+#define map_of_long_tcp_conns    km_longtcpconns_map
+#define long_tcp_conns_events    km_longtcpconns_events


BPF_OBJ_NAME_LEN = 16
So I think, this name of map is too long.

Signed-off-by: Yash Patel <yp969803@gmail.com>

…userspace Signed-off-by: Yash Patel <yp969803@gmail.com>

Signed-off-by: Yash Patel <yp969803@gmail.com>

…km_longconn_ev ringbuff Signed-off-by: Yash Patel <yp969803@gmail.com>

Signed-off-by: Yash Patel <yp969803@gmail.com>

Signed-off-by: Yash Patel <yp969803@gmail.com> rfac: sendmsg.c Signed-off-by: Yash Patel <yp969803@gmail.com> chore: generated bpf2go Signed-off-by: Yash Patel <yp969803@gmail.com> rfac: added is_managed_by_kmesh to sendmsg.c Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 · 2025-03-22T07:33:43Z

    bpf_map.go:60: bpf init failed: bpf Load failed: load program: permission denied:
        	; int sockops_active_prog(struct bpf_sock_ops *skops)
        	0: (bf) r6 = r1
        	; __u64 sock_cookie = bpf_get_socket_cookie(skops);
        	1: (85) call bpf_get_socket_cookie#46
        	2: (bf) r7 = r0
        	; if (skops->family != AF_INET && skops->family != AF_INET6)
        	3: (61) r1 = *(u32 *)(r6 +20)
        	; if (skops->family != AF_INET && skops->family != AF_INET6)
        	4: (bf) r2 = r1
        	5: (47) r2 |= 8
        	6: (15) if r2 == 0xa goto pc+1

@LiZhenCheng9527 @nlgwcy can you look at the above error during bpf_compaitliblity test, why is this happening, i have also shorten the program length so that number of instruction sets not exceeds

yp969803 · 2025-03-22T13:45:25Z

so the problem arrises in the line of code, with the error

	303: (18) r1 = 0xffff8ff1b4577000     ; R1_w=map_ptr(map=km_orig_dst,ks=8,vs=36)
        	305: (85) call bpf_map_lookup_elem#1
        	R2 type=sock_or_null expected=fp, pkt, pkt_meta, map_key, map_value, mem, ringbuf_mem, buf, trusted_ptr_
        	processed 390 insns (limit 1000000) max_states_per_insn 1 total_states 33 peak_states 33 mark_read 5

when i try to comment these lines and run unit tests, verifier accept it. these lines are written previously in the sockops_prog

     __u64 *current_sk = (__u64 *)skops->sk;
        struct bpf_sock_tuple *dst = bpf_map_lookup_elem(&map_of_orig_dst, current_sk);

in the program

SEC("sockops_active")
int sockops_active_prog(struct bpf_sock_ops *skops)
{
    __u64 sock_cookie = bpf_get_socket_cookie(skops);

    if (skops->family != AF_INET && skops->family != AF_INET6)
        return 0;

    switch (skops->op) {
    case BPF_SOCK_OPS_TCP_CONNECT_CB:
        skops_handle_kmesh_managed_process(skops);
        break;

    case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
        if (!is_managed_by_kmesh(skops))
            break;
        observe_on_connect_established(skops->sk, sock_cookie, OUTBOUND);
        if (bpf_sock_ops_cb_flags_set(skops, BPF_SOCK_OPS_STATE_CB_FLAG) != 0
            || bpf_sock_ops_cb_flags_set(skops, BPF_SOCK_OPS_RETRANS_CB_FLAG) != 0
            || bpf_sock_ops_cb_flags_set(skops, BPF_SOCK_OPS_RTT_CB_FLAG) != 0) {
            BPF_LOG(ERR, SOCKOPS, "set sockops cb failed!\n");
        }
        __u64 *current_sk = (__u64 *)skops->sk;
        struct bpf_sock_tuple *dst = bpf_map_lookup_elem(&map_of_orig_dst, current_sk);
        if (dst != NULL)
            enable_encoding_metadata(skops);
        break;

    default:
        break;
    }
    return 0;
}

@nlgwcy @LiZhenCheng9527

Signed-off-by: Yash Patel <yp969803@gmail.com> chore: generated bpf2go Signed-off-by: Yash Patel <yp969803@gmail.com>

Signed-off-by: Yash Patel <yp969803@gmail.com>

hzxuzhonghu · 2025-03-27T08:54:04Z

bpf/kmesh/probes/performance_probe.h

@@ -8,6 +8,7 @@

 #include "bpf_common.h"
 #include "config.h"
+#include "workload.h"


I think probe should not include workload to prevent cicular include

hzxuzhonghu · 2025-03-27T08:55:56Z

bpf/kmesh/probes/tcp_probe.h

    }

-    construct_orig_dst_info(sk, info);
+    construct_orig_dst_info(sk, &info_vals);
+    __builtin_memcpy(info, &info_vals, sizeof(struct tcp_probe_info));


i am not sure why memcpy here

hzxuzhonghu · 2025-03-27T08:59:02Z

bpf/kmesh/probes/tcp_probe.h

+    __u32 *index = bpf_map_lookup_elem(&map_of_soc_id_counter, &zero);
+    if (index && *index < MAP_SIZE_OF_TCP_CONNS) {
+        bpf_map_update_elem(&map_of_soc_id, index, &storage->sock_cookie, BPF_ANY);
+        *index += 1;
+        bpf_map_update_elem(&map_of_soc_id_counter, &zero, index, BPF_ANY);


can you add some comments about these two maps operation

hzxuzhonghu · 2025-03-27T09:04:19Z

bpf/kmesh/probes/tcp_probe.h

+}
+
+static inline void
+refresh_tcp_conn_info_on_state_change(struct bpf_tcp_sock *tcp_sock, struct sock_storage_data *storage, __u32 state)


can u make it use refresh_tcp_conn_info

hzxuzhonghu · 2025-03-27T09:05:09Z

bpf/kmesh/workload/sendmsg.c

+    __uint(max_entries, 1);
+    __type(key, int);
+    __type(value, __u64);
+} tcp_conn_last_flush SEC(".maps");


please obey the kmesh mao naming convention

hzxuzhonghu · 2025-03-27T09:07:34Z

bpf/kmesh/workload/sendmsg.c

+        // Initialize last flush time if not set
+        bpf_map_update_elem(&tcp_conn_last_flush, &key, &init_time, BPF_ANY);
+    } else if ((now - *last_time) >= TIMER_INTERVAL_NS) {
+        flush_tcp_conns();


IIRC, this is flushing all the connections probe info, i think it is a little bit heavy.

Can you do that for this particular socket?

You may get the bpf_tcp_sock from struct sk_msg_md

struct bpf_sock *sk = msg->sk; struct bpf_tcp_sock *tcp_sk = bpf_skc_to_tcp_sock(sk);

Signed-off-by: Yash Patel <yp969803@gmail.com>

hzxuzhonghu · 2025-03-27T11:39:46Z

@yp969803 Pleas rely if you have resolved that or have some diff idea

yp969803 · 2025-03-27T11:43:25Z

@hzxuzhonghu i have seen your comments now, i will resolve it asap !!

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 · 2025-03-28T11:17:06Z

@hzxuzhonghu done the changes, most of the ebpf code is complete, just adding another hook for recv_bytes is left i will do this in end, from now i will start writing test.
@LiZhenCheng9527 @nlgwcy

Signed-off-by: Yash Patel <yp969803@gmail.com>

codecov · 2025-03-30T11:00:39Z

Codecov Report

Attention: Patch coverage is 10.16949% with 53 lines in your changes missing coverage. Please review.

Project coverage is 45.28%. Comparing base (e5ead63) to head (c0a044a).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/controller/telemetry/metric.go	1.92%	51 Missing ⚠️
pkg/controller/telemetry/accesslog.go	71.42%	2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (10.16%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Files with missing lines	Coverage Δ
pkg/controller/telemetry/accesslog.go	`90.32% <71.42%> (ø)`
pkg/controller/telemetry/metric.go	`48.36% <1.92%> (-3.07%)`	⬇️

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fc5794...c0a044a. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Yash Patel <yp969803@gmail.com>

kmesh-bot added the kind/feature label Feb 24, 2025

kmesh-bot requested review from hzxuzhonghu and nlgwcy February 24, 2025 13:01

kmesh-bot added the size/L label Feb 24, 2025

LiZhenCheng9527 reviewed Feb 26, 2025

View reviewed changes

yp969803 added 17 commits February 27, 2025 14:21

feat: added tcp_long_conn_maps

d16e491

Signed-off-by: Yash Patel <yp969803@gmail.com>

feat: added obeserve_long_conn_tcp func in probe.h

13021d5

Signed-off-by: Yash Patel <yp969803@gmail.com>

feat: added perf/event-flush to get conn-metrics fron kernelspace to …

0b112e3

…userspace Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added now param to observe and report funcs

f6c4ef5

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: flush_long_conns

ac45d37

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added bounded loop to flush_long_conns func

5493d52

Signed-off-by: Yash Patel <yp969803@gmail.com>

feat: added timer_func at trafic_control to flush long_conn

38e820a

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: init_tcp_long_conn_flush_timer

bf49aca

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: removed inline from timer-callback func

52a3d58

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added long_conn_metrics ringbuf reader in userspace

60491cb

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: shorten the map names

04a4405

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: removed enableLongTcp metric from metric controller

2b483ad

Signed-off-by: Yash Patel <yp969803@gmail.com>

feat: created longConnectionDataV4 longConnectionDataV6 to read from …

63215bb

…km_longconn_ev ringbuff Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added start_ns to long_conn_tcp_event

b48c50e

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added start_ns and last_report_ns in tcp_probe_info

e19f57d

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added startNs and lastReportNs in requestMetric

4355de8

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: run func in workload_controller

a698cac

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 force-pushed the tcp_long_connection_code branch from b79a168 to a698cac Compare February 27, 2025 08:53

yp969803 added 5 commits February 27, 2025 14:37

rfac: removed tcp_long_conn_env ringbuff and tcp_probe ringbuff instead

c2a46fd

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: tcp_report report_tcp_conn

66d26fd

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added funcs of tcp_long_conn ile in tcp_probe.h

b92b6c6

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: changed maps name in conn_report_time.c

b550bf6

Signed-off-by: Yash Patel <yp969803@gmail.com>

chore: added some ebpf logs for tcp report func

1f9cb92

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 force-pushed the tcp_long_connection_code branch 2 times, most recently from c274149 to 3df32ec Compare March 21, 2025 06:39

yp969803 force-pushed the tcp_long_connection_code branch from 3df32ec to e26073e Compare March 21, 2025 06:44

yp969803 added 2 commits March 22, 2025 20:43

rfac: merge 3 sockops prog into 1 due to overhead issue

372e628

Signed-off-by: Yash Patel <yp969803@gmail.com> chore: generated bpf2go Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac:info_vals in refresh_tcp_conn_info_on_state_change for map update

1c9bedc

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 marked this pull request as ready for review March 24, 2025 08:51

kmesh-bot removed the do-not-merge/work-in-progress label Mar 24, 2025

kmesh-bot requested review from LiZhenCheng9527 and supercharge-xsy March 24, 2025 08:51

yp969803 force-pushed the tcp_long_connection_code branch 2 times, most recently from 9a01a5a to 1ebbcb3 Compare March 24, 2025 09:04

hzxuzhonghu reviewed Mar 27, 2025

View reviewed changes

yp969803 added 2 commits March 27, 2025 16:55

rfac: &current_sk in sockops_prog

96f5c1e

Signed-off-by: Yash Patel <yp969803@gmail.com>

Merge branch 'main' into tcp_long_connection_code

249fde9

yp969803 force-pushed the tcp_long_connection_code branch from 1ebbcb3 to 249fde9 Compare March 27, 2025 11:29

yp969803 added 2 commits March 27, 2025 17:45

chore: added design diagram in tcp_long_conn proposal

3333d91

Signed-off-by: Yash Patel <yp969803@gmail.com>

chore: observe_on_send and reporting of conn in send_msg

c9b2790

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 force-pushed the tcp_long_connection_code branch from 5883b74 to c9b2790 Compare March 28, 2025 11:15

rfac: Test_buildAccesslog

6b512c5

Signed-off-by: Yash Patel <yp969803@gmail.com>

yp969803 added 4 commits March 31, 2025 16:59

rfac: removed msg len check in metric.go

1fa626e

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added conn_id to access log

0238780

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: set cb flags in sockops

4ae76b8

Signed-off-by: Yash Patel <yp969803@gmail.com>

rfac: added logs in tcp_probe funcs

c0a044a

Signed-off-by: Yash Patel <yp969803@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tcp long connections metrics #1249

Tcp long connections metrics #1249

yp969803 commented Feb 24, 2025 •

edited

Loading

kmesh-bot commented Feb 24, 2025

yp969803 commented Feb 24, 2025 •

edited

Loading

LiZhenCheng9527 left a comment

LiZhenCheng9527 Feb 26, 2025

LiZhenCheng9527 Feb 26, 2025

yp969803 commented Mar 22, 2025

yp969803 commented Mar 22, 2025 •

edited

Loading

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu Mar 27, 2025

hzxuzhonghu commented Mar 27, 2025

yp969803 commented Mar 27, 2025

yp969803 commented Mar 28, 2025 •

edited

Loading

codecov bot commented Mar 30, 2025 •

edited

Loading

Tcp long connections metrics #1249

Are you sure you want to change the base?

Tcp long connections metrics #1249

Conversation

yp969803 commented Feb 24, 2025 • edited Loading

kmesh-bot commented Feb 24, 2025

yp969803 commented Feb 24, 2025 • edited Loading

LiZhenCheng9527 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yp969803 commented Mar 22, 2025

yp969803 commented Mar 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzxuzhonghu commented Mar 27, 2025

yp969803 commented Mar 27, 2025

yp969803 commented Mar 28, 2025 • edited Loading

codecov bot commented Mar 30, 2025 • edited Loading

Codecov Report

yp969803 commented Feb 24, 2025 •

edited

Loading

yp969803 commented Feb 24, 2025 •

edited

Loading

yp969803 commented Mar 22, 2025 •

edited

Loading

yp969803 commented Mar 28, 2025 •

edited

Loading

codecov bot commented Mar 30, 2025 •

edited

Loading