-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
HTTP/3: Windows build sends lots of empty UDP datagrams #9086
Comments
That's an awful lot of send()s... |
They were added for performance reasons (but not tested for h3 I bet). What exactly happens if we don't send() there? |
If we don't send, then we won't get another write-ready event from select if we don't send in between two calls, effectively blocking till the next readiness event. I guess we could try to just use this workaround with TCP, as UDP should always be ready for writing/sending, Please see the message of the commit mentioned above for more details why this was necessary. |
From WSAEventSelect then I presume? select() and poll() are not edge-trigged, they should return writable for as long as it is writable.
Yes we should, because send()ing zero bytes might be an almost no-op with TCP but as @tatsuhiro-t figured out: it clearly is much more than that for UDP. |
It doesn't look very easy to add that check though... Can we back-peddle the use of WSAEventSelect() there ? |
Yes, sorry, I meant WSAEventSelect in this context of course, but since I was responding via the mobile app I didn't have time and context to fully type it out. |
We could try to use getsockopt to get a SO_PROTOCOL_INFOW which contains the protocol attached to the socket. But I guess we would then need to cache this somehow to not hurt the performance too much. Back-peddling the use of WSAEventSelect would bring the return of emulating socketpair with local TCP sockets which was the original reason to implement it this way, see the history of multi.c up to that point and especially #5397. Edit: we could also try SO_PROTOCOL_INFO or just SO_TYPE. |
UDP send() could actually return EAGAIN/EWOULDBLOCK. It is not always write ready. |
I will try to work on a solution soon. |
@tatsuhiro-t would you be able to share the commands you used to build curl with HTTP/3 support for/using MinGW-w64? |
I tried to replicate this issue locally using msh3 by @nibanks as HTTP/3 backend, but unfortunately I always get this error for now:
|
Look in the Event Log for messages like: Occurs if your Win-10 version is too old or some Registry setting is wrong. |
You might need to specify CPPFLAGS and LDFLAGS to prioritize the header file locations for ngtcp2/nghttp3/openssl libraries compiled for mingw. |
@tatsuhiro-t thanks, but I also meant the dependencies. |
Basically, follow the instruction https://github.com/ngtcp2/ngtcp2#build-from-git |
Any tip what needs to be changed to make this work on Win10 then?
Thanks, but unfortunately this results in the following error message while compiling curl:
./configure found all the dependencies as can be seen here:
It seems to be quite hard to get HTTP/3 support compiled for Windows 😞 |
It is because compiler sees different installation of openssl when actually compiling curl code. As I wrote earlier:
You need to specify CPPFLAGS and LDFLAGS to the path of quictls to tell compiler to use the correct library header files. |
Thanks, I finally got it to build using the following config options: |
I can finally confirm that I can reproduce the issue and am now starting to tackle a possible solution. |
I need a test server that supports uploading maybe 100 MB using HTTP1/2 and 3 to time the impact of my fix. Any suggestions? |
Right now I can only confirm the expected. The empty-send()-workaround is definitely needed to reset the state of FD_WRITE between multi_wait calls in case no other send() was done. If it is no in place then you can clearly see curl pausing on uploads every now and then in the console output. With the existing workaround in place, the upload is done smoothly using HTTP/1.1. But unfortunately my upload attempts using HTTP/3 are never concluding with or without the workaround active. I just get an endless stream of Any idea why this might be? |
Which server are you using for HTTP/3 upload? I use curl -T and it completes successfully. |
I used |
It looks like postfield is set, upload progress indicator goes to 100% instantly, but the actual upload is done much slower than that. |
Right, so the lots of IP_RECVERR is set only for linux, so this probably is not directly related to mingw build. I'll file an another issue for this. |
@tatsuhiro-t I get the same issue with |
Are there any cURL Windows build configurations that are not affected by this? We'd like to get Windows + HTTP/3 CI for mitmproxy going, but this is breaking my tests locally. :) |
#9203 contains efforts of a fix |
- Limit the 0-sized send procedure that is used to reset a SOCKET's FD_WRITE to TCP sockets only. Prior to this change the reset was used on UDP sockets as well, but unlike TCP sockets a 0-sized send actually sends out out a datagram. Assisted-by: Marc Hörsken Ref: curl#9203 Fixes curl#9086 Closes #xxxx
- Limit the 0-sized send procedure that is used to reset a SOCKET's FD_WRITE to TCP sockets only. Prior to this change the reset was used on UDP sockets as well, but unlike TCP sockets a 0-sized send actually sends out out a datagram. Assisted-by: Marc Hörsken Ref: curl#9203 Fixes curl#9086 Closes #xxxx
- Limit the 0-sized send procedure that is used to reset a SOCKET's FD_WRITE to TCP sockets only. Prior to this change the reset was used on UDP sockets as well, but unlike TCP sockets a 0-sized send actually sends out a datagram. Assisted-by: Marc Hörsken Ref: curl#9203 Fixes curl#9086 Closes curl#10430
I did this
Compile curl windows build (mingw64 works) and contact HTTP/3 server with --http3 option.
You need to either check server log or capture the packets to see the empty UDP datagrams.
The number of UDP datagrams varies, but I observed ~500.
It looks like the empty UDP datagram is sent by this call:
curl/lib/multi.c
Line 1238 in 0defae2
which was introduced via b36442b
If it is TCP specific workaround, maybe we should skip it on UDP sockets?
I expected the following
curl should not send empty UDP datagrams on HTTP/3 connection because such UDP datagram is invalid on QUIC specification.
curl/libcurl version
master branch
[curl -V output]
operating system
The text was updated successfully, but these errors were encountered: