kernel-native mode support restart #987

lec-bit · 2024-10-26T10:28:37Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Signed-off-by: lec-bit <glfhmzmy@126.com>

pkg/bpf/ads/sock_connection.go

codecov · 2024-10-28T01:49:50Z

Codecov Report

Attention: Patch coverage is 36.98630% with 46 lines in your changes missing coverage. Please review.

Project coverage is 49.34%. Comparing base (6bb2ae0) to head (43f8129).
Report is 51 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/bpf/ads/sock_connection.go	50.00%	6 Missing and 4 partials ⚠️
pkg/cache/v2/cluster.go	18.18%	8 Missing and 1 partial ⚠️
pkg/cache/v2/listener.go	18.18%	8 Missing and 1 partial ⚠️
pkg/cache/v2/route.go	18.18%	8 Missing and 1 partial ⚠️
pkg/bpf/utils/bpf_helper.go	33.33%	4 Missing and 2 partials ⚠️
pkg/bpf/workload/sock_connection.go	0.00%	0 Missing and 2 partials ⚠️
pkg/bpf/workload/sock_ops.go	50.00%	0 Missing and 1 partial ⚠️

Files with missing lines	Coverage Δ
pkg/bpf/bpf.go	`41.75% <100.00%> (+1.61%)`	⬆️
pkg/bpf/workload/sendmsg.go	`54.90% <ø> (+1.74%)`	⬆️
pkg/controller/ads/ads_processor.go	`77.13% <100.00%> (+0.20%)`	⬆️
pkg/bpf/workload/sock_ops.go	`54.90% <50.00%> (-0.55%)`	⬇️
pkg/bpf/workload/sock_connection.go	`52.94% <0.00%> (-0.74%)`	⬇️
pkg/bpf/utils/bpf_helper.go	`57.14% <33.33%> (-42.86%)`	⬇️
pkg/cache/v2/cluster.go	`43.24% <18.18%> (-2.41%)`	⬇️
pkg/cache/v2/listener.go	`51.85% <18.18%> (-5.90%)`	⬇️
pkg/cache/v2/route.go	`52.77% <18.18%> (-6.90%)`	⬇️
pkg/bpf/ads/sock_connection.go	`55.90% <50.00%> (-0.62%)`	⬇️

... and 11 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 288f6fa...43f8129. Read the comment docs.

LiZhenCheng9527 · 2024-10-28T01:42:27Z

pkg/bpf/ads/sock_ops.go

+	// pin bpf_link and bpf_tail_call map
+	// pin bpf_link, after restart, update prog in bpf_link
+	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independent
+	mapPinPath := filepath.Join(sc.Info.BpfFsPath, "sockops_tail_call_map")


You can define the path name as a constant in the constant.go file

LiZhenCheng9527 · 2024-10-28T01:52:15Z

pkg/bpf/utils/bpf_helper.go

+func BpfMapDeleteByPinPath(bpfFsPath string) error {
+	progMap, err := ebpf.LoadPinnedMap(bpfFsPath, nil)
+	if err != nil {
+		return fmt.Errorf("loadPinnedProgram failed for %s: %v", bpfFsPath, err)


LoadPinnedProgram or LoadPinnedMap?

LoadPinnedMap here， a type of ebpf_map of BPF_MAP_TYPE_PROG_ARRAY ，I am wrong in log, will fix

LiZhenCheng9527 · 2024-10-28T01:55:08Z

pkg/bpf/utils/bpf_helper.go

+func BpfMapDeleteByPinPath(bpfFsPath string) error {
+	progMap, err := ebpf.LoadPinnedMap(bpfFsPath, nil)
+	if err != nil {
+		return fmt.Errorf("loadPinnedProgram failed for %s: %v", bpfFsPath, err)


Perhaps your error message should carry more information, such as the circumstances under which this error occurred.

fmt.Errorf("loadPinnedProgram failed for %s: %v, when kmesh delete by pin path", bpfFsPath, err)

LiZhenCheng9527 · 2024-10-28T01:56:42Z

pkg/bpf/ads/trace_point.go

-	sc.Link = lk
+	pinPath := filepath.Join(sc.Info.BpfFsPath, "trace_point_link")
+	if restart.GetStartType() == restart.Restart {
+		lk, err := link.LoadPinnedLink(pinPath, &ebpf.LoadPinOptions{})


Suggested change

lk, err := link.LoadPinnedLink(pinPath, &ebpf.LoadPinOptions{})

sc.Link, err := link.LoadPinnedLink(pinPath, &ebpf.LoadPinOptions{})

YaoZengzeng · 2024-10-28T02:13:37Z

pkg/bpf/ads/loader_enhanced.go

 	sc := &BpfAds{}
-	sc.TracePoint.NewBpf(cfg)
+	if err := sc.TracePoint.NewBpf(cfg); err != nil {
+		return sc, err


By convention, return nil, err.

YaoZengzeng · 2024-10-28T02:25:26Z

pkg/bpf/ads/sock_ops.go

-	if err != nil {
+	// pin bpf_link and bpf_tail_call map
+	// pin bpf_link, after restart, update prog in bpf_link
+	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independent


Suggested change

// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independent

// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independently

LiZhenCheng9527 · 2024-10-28T02:48:55Z

Does ads has restart e2e test？

LiZhenCheng9527 · 2024-10-28T02:54:34Z

pkg/constants/constants.go

 	VersionPath         = "/bpf_kmesh/map/"
 	WorkloadVersionPath = "/bpf_kmesh_workload/map/"
+
+	Tail_call_map = "tail_call_map"


Suggested change

Tail_call_map = "tail_call_map"

TailCallMap = "tail_call_map"

In go, everyone uses camel naming by default.

YaoZengzeng · 2024-10-28T03:11:40Z

Does ads has restart e2e test？

Ads doesn't have E2E tests.

Signed-off-by: lec-bit <glfhmzmy@126.com>

kmesh-bot · 2024-11-05T14:10:10Z

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: lec-bit <glfhmzmy@126.com>

nlgwcy · 2024-11-21T15:18:12Z

pkg/bpf/bpf_test.go

-	t.Run("new start", func(t *testing.T) {
-		runTestNormal(t)
+	t.Run("new start DualEengine", func(t *testing.T) {
+		runTestNormalDualEengine(t)


Suggested change

runTestNormalDualEengine(t)

runTestNormalDualEngine(t)

Spelling mistakes. Check them.

nlgwcy · 2024-11-21T15:30:47Z

pkg/controller/client.go

+			NameToLds: *c.AdsController.Processor.Cache.ListenerCache.GetListenerHashPtr(),
+			NameToRds: *c.AdsController.Processor.Cache.RouteCache.GetRouteHashPtr(),
+		}
+		ads.ResetPersistFile()


Where is the hash persistence code in dual-engine mode?

kmesh/pkg/controller/workload/workload_processor.go

Line 808 in 288f6fa

func (p *Processor) handleRemovedAddressesDuringRestart() {

Signed-off-by: lec-bit <glfhzmy@126.com>

Signed-off-by: lec-bit <glfhmzmy@126.com>

nlgwcy · 2024-11-25T01:15:17Z

pkg/bpf/utils/bpf_helper.go

+	return sclink, nil
+}
+
+// func BpfMapDeleteByPinPath(bpfFsPath string) error {


nlgwcy · 2024-11-25T01:16:03Z

/lgtm

Signed-off-by: lec-bit <glfhzmy@126.com>

hzxuzhonghu · 2024-11-25T03:26:52Z

pkg/controller/ads/ads_processor.go

 	lastNonce *lastNonce
 	// the channel used to send domains to dns resolver. key is domain name and value is refreshrate
 	DnsResolverChan chan []*config_cluster_v3.Cluster
+	once            [3]sync.Once


This looks tricky.

hzxuzhonghu · 2024-11-25T03:27:40Z

pkg/controller/ads/ads_hash.go

+}
+
+// Should only be used by test
+func ResetPersistFile() {


Feel remove is clearer than reset

hzxuzhonghu · 2024-11-25T07:55:04Z

pkg/bpf/bpf.go

 func (l *BpfLoader) Stop() {
 	var err error
-	if restart.GetExitType() == restart.Restart && l.config.DualEngineEnabled() {
-		C.deserial_uninit()


why remove C.deserial_uninit()

The deserial logic is updated, and now the default C.deserial_uninit().

Signed-off-by: lec-bit <glfhzmy@126.com>

hzxuzhonghu · 2024-11-28T03:38:06Z

pkg/bpf/ads/sock_connection.go

-	if err != nil {
+	// pin bpf_tail_call map
+	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independently
+	mapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)


Suggested change

mapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)

tailCallMapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)

hzxuzhonghu · 2024-11-28T03:38:25Z

pkg/bpf/ads/sock_ops.go

+	// pin bpf_link and bpf_tail_call map
+	// pin bpf_link, after restart, update prog in bpf_link
+	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independently
+	mapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)


Signed-off-by: lec-bit <glfhmzmy@126.com>

hzxuzhonghu · 2024-11-28T06:45:49Z

/lgtm
/approve

kmesh-bot · 2024-11-28T06:45:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hzxuzhonghu]

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

kernel-native mode support restart

6bb0b03

Signed-off-by: lec-bit <glfhmzmy@126.com>

kmesh-bot added the kind/feature label Oct 26, 2024

kmesh-bot requested review from Okabe-Rintarou-0 and YaoZengzeng October 26, 2024 10:28

kmesh-bot added the size/L label Oct 26, 2024

lec-bit added 2 commits October 26, 2024 18:46

make gen

6193ad2

Signed-off-by: lec-bit <glfhmzmy@126.com>

update

753953c

Signed-off-by: lec-bit <glfhmzmy@126.com>

LiZhenCheng9527 reviewed Oct 28, 2024

View reviewed changes

pkg/bpf/ads/sock_connection.go Show resolved Hide resolved

LiZhenCheng9527 reviewed Oct 28, 2024

View reviewed changes

YaoZengzeng reviewed Oct 28, 2024

View reviewed changes

LiZhenCheng9527 reviewed Oct 28, 2024

View reviewed changes

update constants.Tail_call_map

5a1aee9

Signed-off-by: lec-bit <glfhmzmy@126.com>

lec-bit force-pushed the kernel_native_restart branch from 5a1aee9 to ac73def Compare November 1, 2024 01:36

kmesh-bot added the do-not-merge/contains-merge-commits label Nov 4, 2024

lec-bit added 4 commits November 5, 2024 11:05

Configuration recovery after restart in kernel-native mode

af523f6

Signed-off-by: lec-bit <glfhmzmy@126.com>

typo

9e9f682

Signed-off-by: lec-bit <glfhmzmy@126.com>

Merge branch 'main' into kernel_native_restart

9e311fc

gofmt update

976cdd2

Signed-off-by: lec-bit <glfhmzmy@126.com>

lec-bit force-pushed the kernel_native_restart branch from 1add177 to 9e9f682 Compare November 5, 2024 14:02

kmesh-bot added do-not-merge/contains-merge-commits and removed do-not-merge/contains-merge-commits labels Nov 5, 2024

lec-bit force-pushed the kernel_native_restart branch 2 times, most recently from 8e514ec to 976cdd2 Compare November 6, 2024 02:15

lec-bit added 2 commits November 6, 2024 20:43

add ut test

5446dd1

Signed-off-by: lec-bit <glfhmzmy@126.com>

update golanglint

1cd03bf

Signed-off-by: lec-bit <glfhmzmy@126.com>

nlgwcy reviewed Nov 21, 2024

View reviewed changes

lec-bit and others added 7 commits November 22, 2024 01:48

optimize logic of Unpin tail_call map

8be1dfe

Signed-off-by: lec-bit <glfhzmy@126.com>

type

8d9808c

Signed-off-by: lec-bit <glfhzmy@126.com>

optimize filepath

1de44ab

Signed-off-by: lec-bit <glfhmzmy@126.com>

fix ut golanglint failed

e3e1066

Signed-off-by: lec-bit <glfhmzmy@126.com>

Merge branch 'main' into kernel_native_restart

64f0092

fix ut

8f43e65

Signed-off-by: lec-bit <glfhmzmy@126.com>

typo

55c04f8

Signed-off-by: lec-bit <glfhmzmy@126.com>

nlgwcy reviewed Nov 25, 2024

View reviewed changes

pkg/bpf/utils/bpf_helper.go Outdated

return sclink, nil

}

// func BpfMapDeleteByPinPath(bpfFsPath string) error {

Copy link

Contributor

nlgwcy Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

kmesh-bot assigned nlgwcy Nov 25, 2024

kmesh-bot added lgtm and removed lgtm labels Nov 25, 2024

typo, unnecessary code

5a0e629

Signed-off-by: lec-bit <glfhzmy@126.com>

lec-bit force-pushed the kernel_native_restart branch from 55b1935 to 5a0e629 Compare November 25, 2024 01:56

hzxuzhonghu reviewed Nov 25, 2024

View reviewed changes

Refactor configuration recovery

f316c05

Signed-off-by: lec-bit <glfhzmy@126.com>

lec-bit force-pushed the kernel_native_restart branch from 27eaacd to f316c05 Compare November 28, 2024 01:29

hzxuzhonghu reviewed Nov 28, 2024

View reviewed changes

typo

43f8129

Signed-off-by: lec-bit <glfhmzmy@126.com>

kmesh-bot assigned hzxuzhonghu Nov 28, 2024

kmesh-bot added the lgtm label Nov 28, 2024

kmesh-bot added the approved label Nov 28, 2024

kmesh-bot merged commit 071bfd8 into kmesh-net:main Nov 28, 2024
12 checks passed

	lk, err := link.LoadPinnedLink(pinPath, &ebpf.LoadPinOptions{})
	sc.Link, err := link.LoadPinnedLink(pinPath, &ebpf.LoadPinOptions{})

	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independent
	// tail_call map cannot pin in SetMapPinType->LoadAndAssign, we pin them independently

	Tail_call_map = "tail_call_map"
	TailCallMap = "tail_call_map"

	mapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)
	tailCallMapPinPath := filepath.Join(sc.Info.BpfFsPath, constants.TailCallMap)

kernel-native mode support restart #987

kernel-native mode support restart #987

Uh oh!

Conversation

lec-bit commented Oct 26, 2024

Uh oh!

Uh oh!

codecov bot commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lec-bit Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LiZhenCheng9527 commented Oct 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng commented Oct 28, 2024

Uh oh!

kmesh-bot commented Nov 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nlgwcy commented Nov 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu commented Nov 28, 2024

Uh oh!

kmesh-bot commented Nov 28, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Oct 28, 2024 •

edited

Loading

lec-bit Oct 28, 2024 •

edited

Loading