optimize(bpf): Alternative way to avoid parsing packet at dae0 #600
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 222dfcf. Fixes: daeuniverse#586
|
用了这个PR Build国内外(dae本机和局域网客户端)都上不了网了，通过抓包发现只有 环境debian@Copy-of-VM-debian:~$ uname -a
Linux Copy-of-VM-debian 6.1.0-23-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.99-1 (2024-07-15) x86_64 GNU/Linux
debian@Copy-of-VM-debian:~$ dae -v
dae version unstable-20240807.pr-600.r2.307338
go runtime go1.22.5 linux/amd64
Copyright (c) 2022-2024 @daeuniverse
License GNU AGPLv3 <https://github.com/daeuniverse/dae/blob/main/LICENSE>测试debian@Copy-of-VM-debian:~$ curl -v one.one.one.one
* Could not resolve host: one.one.one.one
* Closing connection 0
curl: (6) Could not resolve host: one.one.one.one
debian@Copy-of-VM-debian:~$ curl -v 1.1.1.1
* Trying 1.1.1.1:80...
* Connected to 1.1.1.1 (1.1.1.1) port 80 (#0)
> GET / HTTP/1.1
> Host: 1.1.1.1
> User-Agent: curl/7.88.1
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
< Server: cloudflare
< Date: Thu, 08 Aug 2024 08:15:30 GMT
< Content-Type: text/html
< Content-Length: 167
< Connection: keep-alive
< Location: https://1.1.1.1/
< CF-RAY: 8afe0f17edaf2f7a-LAX
<
<html>
<head><title>301 Moved Permanently</title></head>
<body>
<center><h1>301 Moved Permanently</h1></center>
<hr><center>cloudflare</center>
</body>
</html>
* Connection #0 to host 1.1.1.1 left intact |
Now we can load l3 addr free from heavy duty of parse_transport and get_tuples.
jschwinger233
force-pushed
the
gray/revert-direct-access
branch
from
3073383
to
4d18ed6
Compare
|
现在正常了 |
|
Is there something preventing this PR from being merged? The latest main branch cannot run on the OpenWrt 6.6 kernel without this PR. @jschwinger233 @mzz2017 I have tested this PR on OpenWrt, and it works well. Is there anything else that needs testing? |
mzz2017
approved these changes
Aug 24, 2024
![dae-prow[bot]](https://avatars.githubusercontent.com/in/345419?s=60&v=4){kind=small}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
#562 caused more issues than benefits, such as #586. Unfortunately I failed to reproduce issues after several attempts. This PR reverts the changes, but introduced an alternative solution to avoid parsing packet at dae0_ingress.
The key idea is skbs at dae0_ingress are bound to be built from a local process (control plane process, dae), so we should be confident in usingThis doesn't work because udp listener socket always has zero value as sk->dst_ip4.skb->sk->{local_ip4,dst_ip4,local_ip6,dst_ip6}to get tuples, instead of callingparse_transport()followed byget_tuples().The key idea is to parse L3 by manually calling
bpf_skb_load_bytesfor twice only. The instruction number can be greatly reduced from 913 to 333 (jitted x64, kernel 6.5.0-1027-oem), -63% comparing to current main (using direct access). Before #562 the insn number can be as many as 1715, then it's -80%.The subtle part is this PR deletes
l4protofrom redirect_tuple. This looks harmless as a TCP connection and a UDP "connection" with same src/dst ip should have same source, therefore they can safely share one redirect_entry.One followup on me: review and fix ci kernel-test.
Checklist
Full Changelogs
Issue Reference
Closes #586
Test Result