Proposal for tcp_long_connection_metrics #1224

yp969803 · 2025-02-06T16:56:34Z

What type of PR is this?
Proposal for "Metrics for TCP Long Connection"
LFX 2025 term-1

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #1211

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

yp969803 · 2025-02-06T21:42:50Z

Some questions:

Do we also need acceess logs for tcp-long-connection or just metrics works
Does current logs support open-telemetry format
If not, do we need opentelemetry compatible logs

@LiZhenCheng9527 @nlgwcy @hzxuzhonghu

hzxuzhonghu · 2025-02-07T02:39:58Z

Do we also need acceess logs for tcp-long-connection or just metrics works

The accesslog is printed after connection closed. Can keep it as now

Integrate with OTEL sounds reasonable to me.

LiZhenCheng9527 · 2025-02-07T02:41:34Z

Some questions:

Do we also need acceess logs for tcp-long-connection or just metrics works

Does current logs support open-telemetry format

If not, do we need opentelemetry compatible logs

@LiZhenCheng9527 @nlgwcy @hzxuzhonghu

In proposal, You need to include the design of the accesslog and metrics. In code, you can only focus on metrics.
Now, accesslog of Kmesh is not support OTEL format. We will support it later.
If you want to do this work. : )

LiZhenCheng9527 · 2025-02-10T07:27:09Z

docs/proposal/tcp_long_connection_metrics.md

+
+- Reporting of metrics and access logs, at periodic time or based on throughput (e.g. after transfer of 1mb of data).
+
+- User can fine tune the time and throughput using yaml during kmesh deployment or can use CLI tool kmeshctl anytime.


What's this mean? How to fine tune the time and throughput?

I was thinking to, give users options to set their prefered periodic time and threshold values during the start of kmesh daemon, be setting the values in values.yaml file

understand.
You can change the description

LiZhenCheng9527 · 2025-02-10T07:29:40Z

docs/proposal/tcp_long_connection_metrics.md

+}
+```
+
+The value of the period or the threshold is provided by the user, if not a default value of 5 seconds and 1 mb is chosen.


Can you able to explain in the proposal why 1MB is the threshold?

If the threshold were set too low, the system might generate too many reports, leading to noise and increased processing overhead, 1 mb threshold sounds appropriate to me. We are also giving users options to set their own threshold if he is not satisfied with 1mb

LiZhenCheng9527 · 2025-02-10T07:30:51Z

@nlgwcy PTAL

yp969803 · 2025-02-10T23:18:44Z

Sorry for inactivity i am sick from last 6 days, i will reply to all your queries and complete my proposal asap.

hzxuzhonghu · 2025-02-13T02:31:11Z

docs/proposal/tcp_long_connection_metrics.md

+authors: 
+ - "yp969803"
+reviewers:
+- "nglwcy"


Suggested change

- "nglwcy"

- "nlgwcy"

nlgwcy · 2025-02-13T04:13:29Z

docs/proposal/tcp_long_connection_metrics.md

+know that this has succeeded?
+-->
+
+- Collect detailed traffic metrics (e.g. bytes send/recieved, direction, throughput, round-trip time, latency , state-change) continously during the lifetime of long TCP connections using ebpf.


It is recommended that TCP retransmission and packet loss measurement indicators be added.

yp969803 · 2025-02-14T04:39:02Z

@LiZhenCheng9527 can you review the ebpf code in the proposal!

LiZhenCheng9527

Please fix typo in your proposal
In Kmesh's dual-engine mode, traffic passes through a waypoint. In the original metrics, the origin destination would be preserved before modifying the TCP metadata. However, I noticed that the current proposal does not address this aspect.

LiZhenCheng9527 · 2025-02-22T03:19:35Z

docs/proposal/tcp_long_connection_metrics.md

+};
+
+struct ipv6_addr {
+    __u8 addr[16];


If ipv4 and ipv6 are represented together, why not use __u32 addr[4]. Is there a difference?

Using union for ipv4 or ipv6,

union { struct ipv4_addr v4; struct ipv6_addr v6; } saddr; union { struct ipv4_addr v4; struct ipv6_addr v6; } daddr;

This approach enhance code readiblity, i can use a single "__u32 addr[4]", which might require conversion logic (e.g., mapping IPv4 into an IPv6-like format), adding complexity

nlgwcy · 2025-02-22T06:16:52Z

docs/proposal/tcp_long_connection_metrics.md

+
+Using various ebpf tracepoints hooks to collects metrics of tcp_long_collection, a ring buffer is also decleared to send data from kernel space to userspace
+
+Code update in tracepoint.c file


It is best to add a separate file.

nlgwcy · 2025-02-22T06:20:49Z

docs/proposal/tcp_long_connection_metrics.md

+    key.dport = ctx->dport;
+
+    if (ctx->newstate == TCP_ESTABLISHED) {
+        struct long_tcp_metrics m = {};


Kernel-native mode has implemented some observation capabilities, such as on_cluster_sock_connect and on_cluster_sock_close. It is recommended to enhance them based on the existing implementation;

nlgwcy · 2025-02-22T06:45:02Z

docs/proposal/tcp_long_connection_metrics.md

+
+    struct long_tcp_metrics *m = bpf_map_lookup_elem(&conn_metrics_map, &key);
+    if (m) {
+        __sync_fetch_and_add(&m->bytes_sent, bytes);


The socket structure already stores link information such as the number of bytes sent and received. Why do we need to calculate it? You can refer to tcp_report

nlgwcy · 2025-02-22T06:51:55Z

docs/proposal/tcp_long_connection_metrics.md

+
+
+SEC("tracepoint/tcp/tcp_set_state")
+int trace_tcp_set_state(struct trace_event_raw_tcp_set_state *ctx)


All newly added BPF observation hook points need to use is_managed_by_kmesh to determine whether the current link is taken over by Kmesh

nlgwcy · 2025-02-22T07:12:01Z

docs/proposal/tcp_long_connection_metrics.md

+// Flush Function: Periodically invoked via a perf event.
+// Iterates over the conn_metrics_map and submits events for connections
+// that have been open longer than LONG_CONN_THRESHOLD_NS.
+SEC("perf_event/flush")


Can the periodic reporting of the indicator information of the long link be realized based on bpf_timer? For reference: https://github.com/Asphaltt/learn-by-example/blob/main/ebpf/timer/tcp-connecting.c

yp969803 · 2025-02-23T06:37:34Z

what is the difference between kmesh_map64, kmesh_map192, kmesh_map296, kmesh_map1600 ebpf maps, they all look similar?
@nlgwcy

kmesh-bot · 2025-02-23T13:16:37Z

[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

hzxuzhonghu · 2025-02-24T02:05:19Z

what is the difference between kmesh_map64, kmesh_map192, kmesh_map296, kmesh_map1600 ebpf maps, they all look similar?

These are inner implementions, you should never use them directly

LiZhenCheng9527 · 2025-02-24T06:22:11Z

what is the difference between kmesh_map64, kmesh_map192, kmesh_map296, kmesh_map1600 ebpf maps, they all look similar? @nlgwcy

you can refer to #1029 and https://github.com/kmesh-net/kmesh/blob/main/docs/proposal/map-in-map_management_enhancement-en.md

hzxuzhonghu · 2025-03-03T07:17:46Z

docs/proposal/tcp_long_connection_metrics.md

+
+Currently kmesh provides access logs during termination and establisment of a TCP connection with more detailed information about the connection.
+
+Kmesh also provides metrics during connection establishment, completion and deny apturing a variety of details about the connection.


Suggested change

Kmesh also provides metrics during connection establishment, completion and deny apturing a variety of details about the connection.

Kmesh also provides metrics during connection establishment, completion and deny capturing a variety of details about the connection.

hzxuzhonghu

Please update once the solution settle down

hzxuzhonghu · 2025-03-03T07:24:52Z

docs/proposal/tcp_long_connection_metrics.md

+nitty-gritty.
+-->
+
+Metrics will be collected using eBPF tracepoint hooks, and a eBPF map will be used to transfer metrics from kernel space to userspace.


tracepoint hooks is not consistent with impl

I have to update the proposal with the new implementations:
#1249
this is the pr with the latest work

feat: created proposal doc for tcp_long_connection_metrics

92769b8

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

kmesh-bot requested review from kevin-wangzefeng and nlgwcy February 6, 2025 16:56

kmesh-bot added the size/S label Feb 6, 2025

yp969803 marked this pull request as draft February 6, 2025 16:58

kmesh-bot added the do-not-merge/work-in-progress label Feb 6, 2025

rfac: created doc structure

1a90d68

kmesh-bot added size/L and removed size/S labels Feb 6, 2025

added summary section in the proposal doc

ba253b2

yp969803 added 7 commits February 8, 2025 22:54

motivation section added in the doc

61be50d

added goals section in the doc

b5e5625

added non-goals section

9f66139

proposal section added

82ef4e4

rfac: goals

c080949

update metric controller

9a3cde8

rfac: metric controller

0b80e5a

LiZhenCheng9527 reviewed Feb 10, 2025

View reviewed changes

hzxuzhonghu reviewed Feb 13, 2025

View reviewed changes

nlgwcy reviewed Feb 13, 2025

View reviewed changes

yp969803 added 4 commits February 13, 2025 11:17

added threshold 1mb reason

b1aa07d

added ebpf code

0b8e608

rfac: ebpf code

5776f6b

changed ring buffer name

5e89e3e

yp969803 added 2 commits February 14, 2025 09:25

rfac: changed metric controller code

77f9b6d

rfac: ebpf code

f659bea

yp969803 added 2 commits February 14, 2025 14:48

added ipv6 support

2a1eb0b

added metric controller run

b70d3f8

yp969803 marked this pull request as ready for review February 14, 2025 23:25

kmesh-bot added size/XL and removed size/L do-not-merge/work-in-progress labels Feb 14, 2025

kmesh-bot requested a review from supercharge-xsy February 14, 2025 23:25

LiZhenCheng9527 reviewed Feb 22, 2025

View reviewed changes

nlgwcy reviewed Feb 22, 2025

View reviewed changes

change ip_address struct

793bac4

kmesh-bot added size/L and removed size/XL labels Feb 23, 2025

hzxuzhonghu reviewed Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal for tcp_long_connection_metrics #1224

Proposal for tcp_long_connection_metrics #1224

yp969803 commented Feb 6, 2025

yp969803 commented Feb 6, 2025

hzxuzhonghu commented Feb 7, 2025

LiZhenCheng9527 commented Feb 7, 2025

LiZhenCheng9527 Feb 10, 2025

yp969803 Feb 13, 2025

LiZhenCheng9527 Feb 13, 2025

LiZhenCheng9527 Feb 10, 2025

yp969803 Feb 13, 2025

LiZhenCheng9527 commented Feb 10, 2025

yp969803 commented Feb 10, 2025

hzxuzhonghu Feb 13, 2025

nlgwcy Feb 13, 2025

yp969803 commented Feb 14, 2025

LiZhenCheng9527 left a comment

LiZhenCheng9527 Feb 22, 2025

yp969803 Feb 22, 2025

nlgwcy Feb 22, 2025

nlgwcy Feb 22, 2025 •

edited

Loading

nlgwcy Feb 22, 2025

nlgwcy Feb 22, 2025

nlgwcy Feb 22, 2025

yp969803 commented Feb 23, 2025

kmesh-bot commented Feb 23, 2025

hzxuzhonghu commented Feb 24, 2025

LiZhenCheng9527 commented Feb 24, 2025

hzxuzhonghu Mar 3, 2025

hzxuzhonghu left a comment

hzxuzhonghu Mar 3, 2025

yp969803 Mar 3, 2025 •

edited

Loading


		- Reporting of metrics and access logs, at periodic time or based on throughput (e.g. after transfer of 1mb of data).

		- User can fine tune the time and throughput using yaml during kmesh deployment or can use CLI tool kmeshctl anytime.


		Using various ebpf tracepoints hooks to collects metrics of tcp_long_collection, a ring buffer is also decleared to send data from kernel space to userspace

		Code update in tracepoint.c file



		SEC("tracepoint/tcp/tcp_set_state")
		int trace_tcp_set_state(struct trace_event_raw_tcp_set_state *ctx)


		Currently kmesh provides access logs during termination and establisment of a TCP connection with more detailed information about the connection.

		Kmesh also provides metrics during connection establishment, completion and deny apturing a variety of details about the connection.

Proposal for tcp_long_connection_metrics #1224

Are you sure you want to change the base?

Proposal for tcp_long_connection_metrics #1224

Conversation

yp969803 commented Feb 6, 2025

yp969803 commented Feb 6, 2025

hzxuzhonghu commented Feb 7, 2025

LiZhenCheng9527 commented Feb 7, 2025

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

LiZhenCheng9527 commented Feb 10, 2025

yp969803 commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yp969803 commented Feb 14, 2025

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

Choose a reason for hiding this comment

nlgwcy Feb 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

yp969803 commented Feb 23, 2025

kmesh-bot commented Feb 23, 2025

hzxuzhonghu commented Feb 24, 2025

LiZhenCheng9527 commented Feb 24, 2025

Choose a reason for hiding this comment

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yp969803 Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

nlgwcy Feb 22, 2025 •

edited

Loading

yp969803 Mar 3, 2025 •

edited

Loading