-
Notifications
You must be signed in to change notification settings - Fork 3
Allow Bidirectional Hole Punching from QUICClient and QUICServer #4
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
Comments
From what I recall we only need to check and fix the weird dialling pattern before this issue can be resolved. Is that correct @CMCDragonkai ? |
Ok, so the following is still left for this issue.
|
Ok, so looking at the dialling procedure, I can see it retrying with a delay that is roughly doubling until it times out. That is expected. Every attempt after the first two sends two packets, the initial handshake crypto frame and a 2nd packet with a ping frame. This may have been what you were seeing before? This seems intentional and not something like multiple connections being created and dialling.
|
Did you see my reported delay pattern?
|
I do see the pattern but my point is, since the packets are clearly pairs of handshake + ping packets at the expected delays, I think this is intentional. If it was repeating the handshake frames with no delay then I'd think it was a problem. |
Digging through the code I can see that this dialling procedure is fully driven by Looking at the logs I can see that the additional packet is generated by In any case, this isn't something like multiple clients dialling at the same time like we originally though. I'm going to chalk this up as intentional behaviour on the part of |
Some odd behaviour when providing an existing socket when starting a client. It's making calls to the Ah, when providing the socket to the client then it doesn't like a target host address in the IPv4 mapped IPv6 address I'm not providing a |
* Added a `keepAliveDelay` parameter to client, server and connections. * Connections will send a ping frame after each interval, this should reset the timeout for both sides, given that the ping is received and responded to. * Added tests for this feature. * Fixes #4 [ci skip]
* Added a `keepAliveDelay` parameter to client, server and connections. * Connections will send a ping frame after each interval, this should reset the timeout for both sides, given that the ping is received and responded to. * Added tests for this feature. * Fixes #4 [ci skip]
* Added a `keepAliveDelay` parameter to client, server and connections. * Connections will send a ping frame after each interval, this should reset the timeout for both sides, given that the ping is received and responded to. * Added tests for this feature. * Fixes #4 [ci skip]
Specification
PK needs to use QUIC system for bidirectional hole punching.
QUICClient
The QUICClient starts a "dialing" phase when it is being created and awaiting the
establishedP
.This results in it sending the
Initial
frame with an initial delay of 1, and doubling between.This will repeat until the
config.maxIdleTimeout
is exhausted, which defaults to 5 seconds.Each time it sends the initial frame, this is stuffed into a UDP packet which is sent to the server.
From this point of view, it is pretty much already doing a "hole punching".
The delay is currently not configurable, and currently I believe there's a bug in the
QUICConnection.send
that results in a delay pattern of1, 2, 0, 4, 0, 8, 0...
. I believe that even though we don't have more frames to be sent, there's a repeat of sending the UDP packet. This should be fixed.This will occur until a connection is established or that the
QUICConnection
times out. If it times out, theQUICClient
async creation should fail.Once a connection is established there should be a keep alive mechanism to ensure that the NAT mappings are maintained.
QUICServer
The QUICServer on the other hand cannot just try to start a connection to the client side.
Instead we need to expose the ability to just send UDP packets to the client side. Right now the
QUICServer.socket
is a protected property... and in PK usage, the socket is likely to be created separately and shared between the client and server in the same peer.But I think it would be ideal to expose the socket somehow... like being able to access
QUICServer.socket
.So one can do
server.socket.send()
.This is actually an application concern, but we just want to support the interface to make this possible.
Tests should test that it's possible to craft random packets and send it via
server.socket.send()
.Random packets may be preferred over simply empty packets as we suspect that empty packets might be thrown away by routers.
Doing this will then result in the reverse hole punch, that will enable the QUIC client to connect.
Now when these packets are received by the client side, these packets are neither associated to an existing connection, nor can they initiate a new connection, so these packets will simply be thrown away. Tests should prove that this is the case.
At the same time once the connection is setup, there should be a keep alive component added to #6.
Additional context
dgram
module #1 (comment) - demonstrates the delay pattern with wireshark screenshotTasks
Fix the weird delay pattern during the "dialing" phase ofNot a bug, seems to be intentional onQUICClient
quiche
's part.Integrate the keep alive component Keepalive component after QUIC connection is established #6 when a connection is live. The interval should only be created after the connection is already established, not before it is established.Change to be made as part of Keepalive component after QUIC connection is established #6The text was updated successfully, but these errors were encountered: