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

TCP Fast Open support #660

Closed
wants to merge 4 commits into from

Conversation

Projects
None yet
2 participants
@ghedo
Copy link
Member

ghedo commented Feb 16, 2016

This adds support for TFO (RFC7413) to libcurl and curl. Right now it only supports recent OS X versions (>= El Capitan), but I plan to add Linux support as well (it's a bit trickier though). Still need to write documentation as well.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Feb 16, 2016

Added documentation.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Feb 16, 2016

Lovely!

But where does the CONNECT_DATA_IDEMPOTENT symbol come from?

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Feb 16, 2016

Looks like it's defined in sys/socket.h, no idea how it gets into lib/url.c though.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Feb 17, 2016

Hmm, the Travis test failure seems unrelated.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Feb 17, 2016

Rebased and pushed. Looks like everything's ok now :)

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Feb 17, 2016

I guess maybe it's too early, but as far as I'm concerned this is ready to be merged. I'll open a different PR for the Linux support if this gets merged before I can push the patch.

@bagder bagder self-assigned this Feb 22, 2016

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Feb 22, 2016

I wouldn't mind having the Linux version in its own separate PR anyway so I don't think they need to be very tightly associated. I just feel that by waiting for the Linux version too before merging we can get them in for the same release (probably the next one then since we close the feature window in two days) and get a bigger momentum for users to actually try it out and get it used.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Apr 2, 2016

Hey @ghedo, do you think we're ripe to merge this?

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Apr 3, 2016

I rebased on master and finally added support for Linux, though it seems that newer Linux versions (on the client side) are not compatible with older versions (on the server side) so keep that in mind while testing.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Apr 3, 2016

One thing missing from the Linux support is making this work with SSL, but I'll leave that for another time.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Apr 11, 2016

@bagder ping?

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Apr 12, 2016

Not forgotten, I promise! Just a busy week here in my end.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Apr 17, 2016

error: lib/url.c: patch does not apply

Can you please rebase this?

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Apr 17, 2016

Done.

@ghedo

This comment has been minimized.

Copy link
Member Author

ghedo commented Apr 18, 2016

Rebased again (there was a conflict in include/curl/curl.h).

@bagder bagder closed this in 03de4e4 Apr 18, 2016

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Apr 18, 2016

thanks a lot!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.