-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
separation bpf and binary in Makefile&provides bpf2go outputs files #971
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to separate workload bpf2go as well
bpf/kmesh/bpf2go/bpf2go.go
Outdated
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshSockopsWorkload ../workload/sockops.c -- -I../workload/include -I../../include -I../probes -DKERNEL_VERSION_HIGHER_5_13_0=1 | ||
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshXDPAuth ../workload/xdp.c -- -I../workload/include -I../../include -I../../../api/v2-c -DKERNEL_VERSION_HIGHER_5_13_0=1 | ||
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshSendmsg ../workload/sendmsg.c -- -I../workload/include -I../../include -DKERNEL_VERSION_HIGHER_5_13_0=1 | ||
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -output-dir normal/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshCgroupSock ../ads/cgroup_sock.c -- -I../ads/include -I../../include -I../../../api/v2-c -DCGROUP_SOCK_MANAGE -DKERNEL_VERSION_HIGHER_5_13_0=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/bpf2go seems redudant
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -output-dir normal/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshCgroupSock ../ads/cgroup_sock.c -- -I../ads/include -I../../include -I../../../api/v2-c -DCGROUP_SOCK_MANAGE -DKERNEL_VERSION_HIGHER_5_13_0=1 | |
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -output-dir normal -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshCgroupSock ../ads/cgroup_sock.c -- -I../ads/include -I../../include -I../../../api/v2-c -DCGROUP_SOCK_MANAGE -DKERNEL_VERSION_HIGHER_5_13_0=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If “/bpf2go“” does not exist, the package cannot be imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? I couldnot quite understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont know cleanly, but i delete /bpf2go ,it dosen't work, I guess "package bpf2go" in go files, so the file need to be same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: If i understand correctly package name bpf2go
can be different from dir
name
bpf2go tool provide flags to set different pkg names
https://github.com/cilium/ebpf/blob/main/cmd/bpf2go/main.go#L108-L109
ok, i will separate workload bpf2go
|
Makefile
Outdated
.PHONY: all data daemon | ||
all: data daemon | ||
|
||
data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name is confusing, this seems building bpf-prog, how about bpf-prog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not only bpf-prog, also deserialization.so kmesh.ko and bpf-prog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think any name is better and concrete than data.
How about separating build ko and bpf
Makefile
Outdated
$(call printlog, BUILD, "kernel") | ||
$(QUIET) make -C kernel/ko_src | ||
|
||
daemon: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not only daemon, it also includes other I would still call this all
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about call this controller?
b781932
to
35472cd
Compare
Signed-off-by: lec-bit <[email protected]>
Signed-off-by: lec-bit <[email protected]>
Makefile
Outdated
$(call printlog, BUILD, "kernel") | ||
$(QUIET) make -C kernel/ko_src | ||
|
||
controller: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not controller, we have cni, mda, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use gen-check to detect any generated file correctly generated
Please update the pr title as well |
Signed-off-by: lec-bit <[email protected]>
The xxx.go file is fine, mainly because it needs to embed an .o file, but we didn't provide the .o file into the code repository. |
What type of PR is this?
/kind enhancement
What this PR does / why we need it:
1.Separate compilation of data and control plane
2.Provide the bpf2go file to the code warehouse and adjust the directory so that subsequent open source packages can be imported.Since the directory has been adjusted, all files will have some changes.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: