Skip to content
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

optimize(udp)/fix(quicSniffer): optimize performance of udp and fix a potential panic of quic #301

Merged
merged 26 commits into from
Nov 15, 2023

Conversation

mzz2017
Copy link
Contributor

@mzz2017 mzz2017 commented Aug 25, 2023

Background

Fix panic

11212

When a quic first packet is insufficient, the re-assembler will panic due to being out of range. This PR fixes it.

Support cross-packet reassembling for QUIC (SNI Sniff)

After this quic cross-packet reassembling, we can get SNI in more cases. However, quic connecting to google servers seems to be refused if the destination is overridden. Therefore, we can only use the quic sniffed domain for routing instead of dest overriding.

UDP Performance

And this PR improves UDP performance.

Before:

image

After:

image

Checklist

Full Changelogs

  • Check range before copy.
  • Upgrade quic-go to v0.38.1.
  • GSO prototype on dae side (NOT READY).
  • Use AnyFromPool to reduce syscall consumption.

Issue Reference

Closes #343

Test Result

@mzz2017 mzz2017 requested a review from a team as a code owner August 25, 2023 15:35
piyoki
piyoki previously approved these changes Aug 26, 2023
dae-prow[bot]
dae-prow bot previously approved these changes Aug 26, 2023
Copy link
Contributor

@dae-prow dae-prow bot left a comment

Choose a reason for hiding this comment

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

🧪 Since the PR has been fully tested, please consider merging it.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Aug 26, 2023

Hold it. We need to improve the stream sniffer instead of just skip the problem.

I'll convert it into a draft.

@mzz2017 mzz2017 marked this pull request as draft August 26, 2023 05:44
@mzz2017 mzz2017 dismissed stale reviews from dae-prow[bot] and piyoki via f22b148 August 26, 2023 09:55
@dae-prow
Copy link
Contributor

dae-prow bot commented Aug 26, 2023

❌ Your branch is currently out-of-sync to main. No worry, I will fix it for you.

@mzz2017 mzz2017 marked this pull request as ready for review August 26, 2023 11:42
@mzz2017 mzz2017 requested a review from a team as a code owner August 26, 2023 11:42
@mzz2017 mzz2017 marked this pull request as draft August 26, 2023 11:44
@mzz2017 mzz2017 changed the title fix(quicSnifferReassemble): a potential panic optimize(udp)/fix(quicSniffer): optimize performance of udp and fix a potential panic of quic Aug 27, 2023
@mzz2017 mzz2017 marked this pull request as ready for review August 27, 2023 10:25
@mzz2017
Copy link
Contributor Author

mzz2017 commented Aug 27, 2023

Ready for review.

@mzz2017 mzz2017 marked this pull request as draft September 1, 2023 08:37
@dae-prow
Copy link
Contributor

dae-prow bot commented Oct 21, 2023

❌ Your branch is currently out-of-sync to main. No worry, I will fix it for you.

@mzz2017 mzz2017 marked this pull request as ready for review October 21, 2023 07:57
@mzz2017
Copy link
Contributor Author

mzz2017 commented Oct 21, 2023

Tested again earlier in the day. No more panic issues occurred.

I observed some abnormal issues earlier in the day. DNS is not working quite well. Also, the embed video on YouTube was not able to load and play automatically. We need further investigation on this.

Sniffing should work fine now. Please test it again, thanks.

@mzz2017 mzz2017 mentioned this pull request Oct 21, 2023
3 tasks
@dae-prow
Copy link
Contributor

dae-prow bot commented Oct 21, 2023

❌ Your branch is currently out-of-sync to main. No worry, I will fix it for you.

@piyoki
Copy link
Contributor

piyoki commented Oct 24, 2023

TCP is working as expected. However, there is a noticeable speed discrepancy in UDP connection. Let’s gather more feedbacks from the end-users.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Nov 14, 2023

https://t.me/daeuniverse/119500

It is recommended to merge in advance.
And the bug should be fixed later.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Nov 14, 2023

cc @yqlbu

@piyoki
Copy link
Contributor

piyoki commented Nov 14, 2023

cc @yqlbu

Ok, let's do so. However, we shall make an announcement in the next release as this is a known issue.

@piyoki
Copy link
Contributor

piyoki commented Nov 14, 2023

Link to #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] 代理 HTTP/3 异常
2 participants