Add TCP Fast Open support #1328
Conversation
Forward proxy has been removed since we cannot control its socket options. Signed-off-by: Syrone Wong <[email protected]>
|
It hasn't been tested on Windows 7; I don't have such devices. |
|
The Appveyor url in readme.md is broken ( https://ci.appveyor.com/project/wongsyrone/shadowsocks-windows-yqdou ) Unable test. (Too lazy for compile 😅 ) |
|
On Windows 7,Log print: |
|
It's expected. #1045 |
|
I talked with @madeye before, he agreed to remove Forward proxy. |
|
Is it possible to implement forward proxy as a SIP003 plugin instead? If so, we can keep our core code base as clean as possible. |
|
IMO, the priority list of pull requests is SIP003 > TFO > Forward proxy. |
|
I choose not to modify my PR. |
|
@madeye @wongsyrone Why is this in conflict with my pull request ? I don't touch any of the core tcp / udp relay code (I implemented it in a far simpler way from a higher level). I see no reason why we can't have tcp fast open at least for the supported cases (no forward proxy, no sip003 plugin, supported OS). In case of forward proxy or sip003, it is the responsibility of the proxy or plugin to also implement tcp fast open but no reason why we can't have it too... |
|
Yes, if there is no conflict, I think we can try to merge this PR next. @rwasef1830 Any interest to help merge this one? |
|
Yes, should I open a new pull request and cherry pick the commit to my fork ? I want to maintain credits to @wongsyrone |
|
I think we'd better open a new pull request and rebase the latest changes then. @wongsyrone What do you think? |
|
There are two must-have for TFO.
Under forward proxy, I have no idea on fulfilling these two requirements. Currently, I'm investigating c-ares, cannot find much time to rebase. If someone has interest, he can mention my ID in the commit message. As I said before, it serves as a reference implementation, everyone can use without notice. |
|
@wongsyrone ok, thanks. I will cherry pick and open a new pull request. I will mention you in the commit message. I will leave forward proxy option, just that when it is used, fast open will be disabled. |
|
Sorry I have not get sufficient time to review all the code of this pull request and do a full assessment of code impacts. |
|
Forgot to mention, the two must-have above is specific to the Socket.ConnectAsync() way. If you want to pass parameters to the plugin like: "param=tcp-fast-open;blah-blah" to notify other layers to enable TFO. That layer should use ConnectEx on Windows, otherwise, it won't work. |
|
@wongsyrone Yeah I noticed this. Currently, I am trying to make the changes as small as possible to implement this, but I really have the urge of rewriting that layer.. The ancient BeginXXX EndXXX style code is very scattered and hard to follow. |
|
Do you want to use TAP? I already implemented it with SAEA for the whole networking layer. |
|
I want to change IProxy interface to expose TAP and wrap SAEA into TAP interface, it is much cleaner. Even better if I can make it expose Stream interface, so unit testing becomes possible. Then I want to have all code using TAP async/await. It will make the flow much cleaner. Anyway, I am working on this. |
|
In fact, there is no need to convert all code into TAP. If you use SAEA, there is no need to keep connect timer, just let the socket layer handle it. |
Forward proxy has been removed since we cannot control
its socket options.
Signed-off-by: Syrone Wong [email protected]