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

Keepalive component after QUIC connection is established #6

Closed
5 of 6 tasks
CMCDragonkai opened this issue Apr 11, 2023 · 6 comments
Closed
5 of 6 tasks

Keepalive component after QUIC connection is established #6

CMCDragonkai opened this issue Apr 11, 2023 · 6 comments
Assignees
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Apr 11, 2023

Specification

After the QUIC connection is established, there's a max idle timeout. Nothing actually keeps the connection alive. If there's no activity on the connection either by stream data or datagram data, then the connection will timeout and be destroyed. Once destroyed, the QUICClient is destroyed, or the QUICServer removes it from the connection map.

We should have a interval loop that keeps the connection alive. This will be important for any connection that we need to maintain the NAT mappings. Which usually activity under 20 seconds.

One way to do this is with the send_ack_eliciting method. This method will queue up a PING frame if it detects there's nothing in the send queue that will keep the connection alive (and by nothing we mean something that will trigger an acknowledgement from the remote end).

I imagine that this can be done from both ends, the client and server. So both sides are capable of sending a PING frame and elicit an acknowledgement.

The function call is a noop if there's stuff ihttps://github.com//issues/1#issuecomment-1501236254n the queue that will keep the connection alive anyway.

We should add in a keepaliveIntervalTime that maintains the connection liveness to QUICConnection. This can be added to the config, although it's not actually used by the buildQuicheConfig.

Alternatively it can be an extra parameter to the QUICConnection itself.

I think this loop will be rather easy. It can be a setTimeout loop that just calls that function repeatedly. However this loop can only start once the connection is established, and the loop must not be running when the connection is closing or draining.

Additional context

Tasks

  • 1. Introduce keepAliveTime option to QUICConnection, propagate from both QUICClient and QUICServer.
  • 2. Ensure that the loop is only running after being established and before it is closed or draining.
  • 3. Test that a connection is kept alive even during the idle timeout.
  • 4. Note that by specifying such a thing this does not conflict with the idle timeout. This is because, the remote end might still be broken or not responding, in that case, the idle timeout will still be hit and the connection will timeout.
  • 5. Use it in PK and make sure it's less than 5 seconds.
  • 6. Test what happens if it is enabled on both client and server, what do we see on the wireshark monitor?
@tegefaulkes
Copy link
Contributor

I recall something in the quic RFC about ping frames being sent if a timeout were to happen soon. So I think keep alive is built into quic? It's something I'll have to check.

If we do need to implement keep alive logic here are a few ideas,

  1. Quic has ping frames, if it's not automatic then there may be a way to manually trigger them?
  2. If we had to implement something then we can create a single stream to send our own ping/pong messages.

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented May 5, 2023 via email

@tegefaulkes
Copy link
Contributor

Looking over the quiche docs, the only mention of the ping frame is for the send_ack_eliciting method https://docs.rs/quiche/latest/quiche/struct.Connection.html#method.send_ack_eliciting.

If I understand it correctly, After calling send_ack_eliciting the next connection.send() attempt will generate a ping frame packet if no other packets will be generated. If a packet was already waiting to be sent then it will do nothing. If it is called twice, only 1 new packet would be generated.

So to implement the keep-alive we just need to call send_ack_eliciting on an interval. We could get fancy and check if x delay since a packet has been sent, but I don't think we need that.

So changes that need to be made.

  1. Create a keep-alive interval timer when the connection is created during construction.
  2. Every interval we call send_ack_eliciting and send.
  3. When cleaning up the connection we need to clear the interval timer.

tegefaulkes added a commit that referenced this issue May 10, 2023
* Done adding the feature,
* Still need to write tests

* Related #6

[ci skip]
@tegefaulkes
Copy link
Contributor

Instead of a keepaliveIntervalTime parameter for construction, I'm adding a setKeepAlive method so we can set the keep alive at any time. Client and server will use this to set the keep alive after establishment.

@tegefaulkes
Copy link
Contributor

Both sides doing timeouts looks something like this.

1 0.000000000 CLIENT Initial, PKN: 0, CRYPTO
2 0.003445980 SERVER Retry
3 0.004450904 CLIENT Initial, PKN: 1, CRYPTO
4 0.007895284 SERVER Handshake, PKN: 0, CRYPTO
5 0.008070888 SERVER Handshake, PKN: 1, CRYPTO
6 0.009016510 CLIENT Handshake, PKN: 0, ACK
7 0.010089535 CLIENT Handshake, PKN: 1, ACK, CRYPTO
8 0.011349064 SERVER Protected Payload (KP0), PKN: 0, DONE, CRYPTO
9 0.012237485 CLIENT Protected Payload (KP0), PKN: 0, ACK
10 1.007485100 SERVER Protected Payload (KP0), PKN: 1, PING, PADDING
11 1.008462923 CLIENT Protected Payload (KP0), PKN: 1, ACK
12 1.011268288 CLIENT Protected Payload (KP0), PKN: 2, PING, PADDING
13 1.012332313 SERVER Protected Payload (KP0), PKN: 2, ACK
14 2.008451188 SERVER Protected Payload (KP0), PKN: 3, PING, PADDING
15 2.009454311 CLIENT Protected Payload (KP0), PKN: 3, ACK
16 2.011583460 CLIENT Protected Payload (KP0), PKN: 4, PING, PADDING
17 2.012327978 SERVER Protected Payload (KP0), PKN: 4, ACK
18 3.009607920 SERVER Protected Payload (KP0), PKN: 5, PING, PADDING
19 3.010677245 CLIENT Protected Payload (KP0), PKN: 5, ACK
20 3.012587189 CLIENT Protected Payload (KP0), PKN: 6, PING, PADDING
21 3.013224903 SERVER Protected Payload (KP0), PKN: 6, ACK
22 4.011331505 SERVER Protected Payload (KP0), PKN: 7, PING, PADDING
23 4.012639335 CLIENT Protected Payload (KP0), PKN: 7, ACK
24 4.012986143 CLIENT Protected Payload (KP0), PKN: 8, PING, PADDING
25 4.014197071 SERVER Protected Payload (KP0), PKN: 8, ACK
26 5.012244510 SERVER Protected Payload (KP0), PKN: 9, PING, PADDING
27 5.012907725 CLIENT Protected Payload (KP0), PKN: 9, PING, PADDING
28 5.013517739 CLIENT Protected Payload (KP0), PKN: 10, ACK
29 5.014344358 SERVER Protected Payload (KP0), PKN: 10, ACK
30 6.012996752 SERVER Protected Payload (KP0), PKN: 11, PING, PADDING
31 6.013341960 CLIENT Protected Payload (KP0), PKN: 11, PING, PADDING
32 6.014423184 CLIENT Protected Payload (KP0), PKN: 12, ACK
33 6.014943796 SERVER Protected Payload (KP0), PKN: 12, ACK
34 6.312145693 CLIENT Protected Payload (KP0), PKN: 13, CC

@tegefaulkes
Copy link
Contributor

This was fixed by d542dcb but I put the wrong issue id in the commit message,

Resolving this.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices label Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices
Development

No branches or pull requests

2 participants