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

feat: Add support for hysteria2 dialer and protocol #533

Merged
merged 9 commits into from
Jun 16, 2024

Conversation

mnixry
Copy link
Contributor

@mnixry mnixry commented Jun 10, 2024

Background

ref: daeuniverse/outbound#9

This PR is currently a draft and needs user feedback to move forward. Users interested in testing this feature are encouraged to use the GitHub Action build.

Checklist

Full Changelogs

  • Implement Hysteria2 protocol support.

Issue Reference

Closes #48
Closes #450

Test Result

This feature should be tested by end user.

@mnixry mnixry marked this pull request as ready for review June 10, 2024 18:31
@mnixry mnixry requested a review from a team as a code owner June 10, 2024 18:31
@sumire88 sumire88 changed the title wip: feat: Add support for hysteria2 dialer and protocol feat: Add support for hysteria2 dialer and protocol Jun 11, 2024
@mnixry mnixry mentioned this pull request Jun 11, 2024
@douglarek
Copy link
Contributor

This is a highly anticipated feature support, extremely impressive. It has passed the test on my Linux (kernel 6.9), although there seem to be some warning logs, I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

@xmapst
Copy link

xmapst commented Jun 11, 2024

This is a highly anticipated feature support, extremely impressive. It has passed the test on my Linux (kernel 6.9), although there seem to be some warning logs, I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

是的 在openwrt上运行也出现这种情况 但不影响正常使用

@mnixry
Copy link
Contributor Author

mnixry commented Jun 11, 2024

I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

@douglarek @xmapst Thank you for your feedback. I plan to address this issue shortly, likely by Wednesday or Thursday. Should you encounter any additional problems during extended use, please don't hesitate to follow up.

During my personal usage, I've encountered a few issues:

  • Subpar HTTP/3 performance is expected with UDP-over-TCP proxy methods; however, it shouldn't be the case for protocols like Hysteria2 that utilize QUIC and connection multiplexing. The source of this problem is unclear to me, and I lack the expertise to diagnose and debug it. (I'll later compare this with the official client to determine whether the issue is on our end or a more widespread problem.)

@Integral-Tech
Copy link
Contributor

It works on Arch Linux, kernel 6.9.3-arch1-1.

@mzz2017
Copy link
Contributor

mzz2017 commented Jun 12, 2024

@mnixry For the udp over tcp problem, could you please revert my patch on your local and test it again?

@douglarek
Copy link
Contributor

This is a highly anticipated feature support, extremely impressive. It has passed the test on my Linux (kernel 6.9), although there seem to be some warning logs, I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

The current implementation has too high of a delay. The same server is simultaneously configured with tuic (sb implementation) and hy2 (original version). The udp4(DNS) delay results for dae, the former is between 1~4s, while tuic is around 200ms.

@mnixry
Copy link
Contributor Author

mnixry commented Jun 12, 2024

For the udp over tcp problem, could you please revert my patch on your local and test it again?

@mzz2017 Reverting the patch did not resolve the issue for me. Further testing indicates that both the official client and the sing-box implementation exhibit similar problems, suggesting that the issue may not lie with the Hysteria2 dialer.

Here's my benchmarking method:

  • Install a curl build with HTTP/3 support, such as the one available at https://github.com/stunnel/static-curl.
  • Use the following command for benchmarking:
    ./curl -L4v --http3-only "https://h3.speed.cloudflare.com/__down?bytes=25000000" > /dev/null
  • The following 4 benchmarks were conducted:
    1. Running on dae with the built-in Hysteria2 dialer provided in this PR. The download speed peaks at 30kB/s, then drops to 0b/s, and continues to fluctuate between 30kB/s and 0b/s.
    2. Running on dae with an external sing-box, connected via socks5 protocol. The behavior is the same as above.
    3. Running on dae with an external apernet/hysteria client, connected via socks5 protocol. The behavior is the same as above.
    4. Running on dae with an external apernet/hysteria client with bandwidth provided (i.e., enabling the brutal cc algorithm), the speed remains at 30kB/s without dropping or increasing.

Since curl does not support directly using a socks5 proxy with the HTTP/3 protocol, I am currently unable to provide examples without using dae. Additionally, testing under various network conditions is also needed.

@mzz2017
Copy link
Contributor

mzz2017 commented Jun 12, 2024

@mnixry You can use curl -x http://YOUR_PROXY or curl -x socks5://YOUR_PROXY to achieve

@mnixry
Copy link
Contributor Author

mnixry commented Jun 12, 2024

You can use curl -x http://YOUR_PROXY or curl -x socks5://YOUR_PROXY to achieve

@mzz2017 Sadly, this approach does not work for curl with HTTP/3 protocol.

$ ./curl -L4vx socks5://localhost:2080 --http3-only "https://h3.speed.cloudflare.com/__down?bytes=25000000" > /dev/null
* HTTP/3 is not supported over a SOCKS proxy
* Closing connection
curl: (3) HTTP/3 is not supported over a SOCKS proxy

I am also wondering if there is any networking tools like curl supports h3 over socks5 proxy.

ref: https://github.com/curl/curl/blob/267c3b31e911dc96c2445c0342bab7b827f9c0a8/lib/vquic/vquic.c#L653-L656

@mzz2017
Copy link
Contributor

mzz2017 commented Jun 12, 2024

@mnixry What about sing-box tun inbound with auto-route?

@mnixry
Copy link
Contributor Author

mnixry commented Jun 12, 2024

What about sing-box tun inbound with auto-route?

@mzz2017 The TUN mode in Sing-box appears to function well with HTTP/3 connections, progressively increasing speeds up to approximately 1 MB/s. Interestingly, even when using UDP-over-TCP proxy solutions, HTTP/3 downloads perform significantly faster than dae (also about 1 MB/s).

Could you reproduce these results on your end? Since this appears to be unrelated to the current pull request, we should probably open a new issue for tracking purposes.

@mnixry
Copy link
Contributor Author

mnixry commented Jun 12, 2024

The current implementation has too high of a delay. The same server is simultaneously configured with tuic (sb implementation) and hy2 (original version). The udp4(DNS) delay results for dae, the former is between 1~4s, while tuic is around 200ms.

@douglarek Apologies for the delayed response. I'm unable to replicate the issue on my side; it might stem from network jitter or a delay due to initial protocol handshaking.

Could you try using tools such as 'dig' from BIND to assess any DNS resolution delays?

@mzz2017
Copy link
Contributor

mzz2017 commented Jun 12, 2024

@mnixry I'll look into it as long as i'm free.

@mnixry
Copy link
Contributor Author

mnixry commented Jun 14, 2024

I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

是的 在openwrt上运行也出现这种情况 但不影响正常使用

@douglarek @xmapst The issue has been fixed in f2d5302. Could you check out the latest build and confirm whether the warning still persists?

@douglarek
Copy link
Contributor

I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

是的 在openwrt上运行也出现这种情况 但不影响正常使用

@douglarek @xmapst The issue has been fixed in f2d5302. Could you check out the latest build and confirm whether the warning still persists?

It's indeed fixed, the warn log has disappeared. This is so awesome.

@xmapst
Copy link

xmapst commented Jun 14, 2024

I'm not sure if it matters: level=warning msg="handleConn: handleTCP relay error: deadline exceeded". However, it doesn't affect the basic proxy experience.

是的 在openwrt上运行也出现这种情况 但不影响正常使用

@douglarek @xmapst The issue has been fixed in f2d5302. Could you check out the latest build and confirm whether the warning still persists?

太棒了 确实修复了

@mzz2017
Copy link
Contributor

mzz2017 commented Jun 14, 2024

The TUN mode in Sing-box appears to function well with HTTP/3 connections, progressively increasing speeds up to approximately 1 MB/s. Interestingly, even when using UDP-over-TCP proxy solutions, HTTP/3 downloads perform significantly faster than dae (also about 1 MB/s).

@mnixry I fixed a possibly related problem that caused download speed zeroing. The problem dies away on my local. Could you please try this PR? #539

@xmapst
Copy link

xmapst commented Jun 16, 2024

进展如何了? 大概什么时候合并?

@mnixry
Copy link
Contributor Author

mnixry commented Jun 16, 2024

进展如何了? 大概什么时候合并?

@xmapst I believe there are two things blocking:

  1. Wait for the merge of fix: incidental packet drop and weird UDP state maintaining #539 to confirm that stateful UDP connections function properly.
  2. Require more user testing. I'd like to wait ~1 week to ensure that this build operates without any additional issues.

However, members of the daeuniverse who determine if this PR will ultimately be merged may hold different opinions.

@xmapst
Copy link

xmapst commented Jun 16, 2024

进展如何了? 大概什么时候合并?

@xmapst I believe there are two things blocking:

  1. Wait for the merge of fix: incidental packet drop and weird UDP state maintaining #539 to confirm that stateful UDP connections function properly.
  2. Require more user testing. I'd like to wait ~1 week to ensure that this build operates without any additional issues.

However, members of the daeuniverse who determine if this PR will ultimately be merged may hold different opinions.

好的 感谢告知

@mzz2017

This comment was marked as resolved.

@mzz2017 mzz2017 requested a review from a team as a code owner June 16, 2024 12:33
Copy link
Contributor

@mzz2017 mzz2017 left a comment

Choose a reason for hiding this comment

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

Ready to merge

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.

@sumire88 sumire88 merged commit cb61bf9 into daeuniverse:main Jun 16, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

支持 Hysteria1/2 协议 支持hysteria协议
6 participants