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

http2: send HEADER & DATA together if possible #11420

Closed
wants to merge 3 commits into from
Closed

http2: send HEADER & DATA together if possible #11420

wants to merge 3 commits into from

Conversation

peppapighs
Copy link
Contributor

This PR is based on the conversation here.

When sending an HTTP/2 POST request, curl will always defer request body to a different TLS record (if SSL/TLS is enabled) and TCP packet. This problem is previously addressed in #6363.

Belows are network packets captured by Wireshark by running SSLKEYLOGFILE=/tmp/sslkeylog.log src/curl -k "https://nghttp2.org" -d "moo":

image

We can combine the TCP packet by disabling TCP_NODELAY. Belows is a TCP packet sent by running SSLKEYLOGFILE=/tmp/sslkeylog.log src/curl -k "https://nghttp2.org" -d "moo" --no-tcp-nodelay:

image

As can be seen from the image above, curl still generates 2 TLS records for HEADER and DATA frames, respectively (ignore the first TLS record from MAGIC frame).

Therefore, this PR provides an optimization that allows request body to be sent together with request headers in a single TLS record and TCP packet if the body size is small enough. This is achieved by attaching request body to header data so that h2_submit() can write the body directly after it has created a stream.

Belows is a TCP packet sent by running SSLKEYLOGFILE=/tmp/sslkeylog.log src/curl -k "https://nghttp2.org" -d "moo" with this change:

image

@github-actions github-actions bot added the HTTP label Jul 11, 2023
@bagder bagder requested a review from icing July 11, 2023 08:35
Copy link
Contributor

@icing icing left a comment

Choose a reason for hiding this comment

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

LGTM! Nicely done.

lib/http2.c Show resolved Hide resolved
lib/http2.c Outdated Show resolved Hide resolved
@bagder bagder added the HTTP/2 label Jul 11, 2023
@bagder bagder closed this in c9ec851 Jul 11, 2023
@bagder
Copy link
Member

bagder commented Jul 11, 2023

Thanks!

@peppapighs peppapighs deleted the http2-coalesce-frames branch July 12, 2023 00:35
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants