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

ngtcp2: Allow curl to send larger UDP datagrams #8883

Closed
wants to merge 2 commits into from
Closed

Conversation

tatsuhiro-t
Copy link
Contributor

@tatsuhiro-t tatsuhiro-t commented May 19, 2022

Allow curl to send larger UDP datagram if Path MTU Discovery finds the
availability of larger path MTU. To make it work and not to send
fragmented packet, we need to set DF bit. That makes send(2) fail
with EMSGSIZE if UDP datagram is too large. In that case, just let it
lost. This patch enables DF bit for Linux only.

Allow curl to send larger UDP datagram if Path MTU Discovery finds the
availability of larger path MTU.  To make it work and not to send
fragmented packet, we need to set DF bit.  That makes send(2) fail
with EMSGSIZE if UDP datagram is too large.  In that case, just let it
lost.  This patch enables DF bit for Linux only.
/* Send larger UDP datagram if we can set DF bit. */
#if defined(__linux__) && defined(IP_MTU_DISCOVER) && \
defined(IPV6_MTU_DISCOVER)
uint8_t out[1452];
Copy link
Member

@bagder bagder May 19, 2022

Choose a reason for hiding this comment

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

Shouldn't this value rather have an NGTCP2_-name as well?

Copy link
Member

@bagder bagder May 19, 2022

Choose a reason for hiding this comment

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

And maybe, since the smaller value is 1200 (right?), maybe you could just use the larger value unconditionally there and save the code from a complicated #ifdef setup?

Copy link
Contributor Author

@tatsuhiro-t tatsuhiro-t May 20, 2022

Choose a reason for hiding this comment

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

Fixed in 1c66f5b
NGTCP2_ name is now defined in ngtcp2 library header, and now always use larger buffer.

@bagder bagder added the HTTP/3 label May 19, 2022
bagder
bagder approved these changes May 20, 2022
@bagder bagder closed this in 8ea851b May 20, 2022
@bagder
Copy link
Member

@bagder bagder commented May 20, 2022

Thanks!

@bagder bagder deleted the ngtcp2-pmtud branch May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTTP/3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants