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

Add support for 0RTT-BDP extension #1073

Closed
wants to merge 1 commit into from

Conversation

francklinSIMO
Copy link
Collaborator

@francklinSIMO francklinSIMO commented Oct 26, 2020

SUMMARY
This implements 0RTT-BDP extension: a generic method to exchange path parameters related to transport. This aims at improving traffic delivery by means of allowing the connection to short-cut slow RTT-based processes that grow connection parameters. If necessary, please see here for futher details about this ietf draft.

CHANGES
Following components have been modified:

  • Core library (picoquic)
  • Test program (picoquicfirts)

DEPENDENCIES
You will need a specific version of Picotls for testing this feature. This PicoTLS version allows to dynamically add bdp parameters as TLS extension in New Session Ticket message which now can be sent multiple times during a session. This version is currently under review and available here .

@huitema
Copy link
Collaborator

huitema commented Oct 26, 2020

The "special version of picotls" makes this extension not practical as is. It is the reason why all checks are failing. It goes against the general design of QUIC to not use a special version of TLS. In the general QUIC design, the TLS session ticket is entirely managed by the TLS stack, and is generated at the end of the handshake. It would be much simpler to NOT use the TLS session ticket, and use a new QUIC frame, modeled for example after the "NEW TOKEN" frame. The value would then be stored in a "congestion parameter store", modeled after the "token store". (We may think of unifying token store, ticket store and congestion data store in a subsequent PR.)

@huitema
Copy link
Collaborator

huitema commented Oct 26, 2020

I am looking at https://tools.ietf.org/html/draft-kuhn-quic-0rtt-bdp-07. It documents three parameters:

  • recon_bytes_in_flight
  • recon_min_rtt
  • recon_max_pkt_number

Your PR adds CWIN, RTT variant and MAX ACK DELAY. This is questionable. CWIN is very much the same concept as "bytes in flight", and there is already a MAX ACK DELAY transport parameter. I think we should have a discussion with authors of the draft before adding these extra parameters.

@huitema
Copy link
Collaborator

huitema commented Oct 26, 2020

I am also looking at the interaction between the transport parameters and the congestion control algorithms. I would express to see some additional code in the implementations of Cubic or BBR to process the additional initial parameters, but I don't see that. How will that work?

This brings another question, regarding testing. A general requirement is that we add tests in the "picoquic test" code to check that the extensions behave as expected. This includes basic tests to verifying parsing and logging, as well as functional tests. If you look at the current test suite, there are functional tests of satellite transmission with different congestion control algorithms. We should have variants of these tests when the extensions are used, to demonstrate that the implementation uses the new parameters as intended.

@NicoKos
Copy link

NicoKos commented Oct 27, 2020

The "special version of picotls" makes this extension not practical as is. It is the reason why all checks are failing. It goes against the general design of QUIC to not use a special version of TLS. In the general QUIC design, the TLS session ticket is entirely managed by the TLS stack, and is generated at the end of the handshake. It would be much simpler to NOT use the TLS session ticket, and use a new QUIC frame, modeled for example after the "NEW TOKEN" frame. The value would then be stored in a "congestion parameter store", modeled after the "token store". (We may think of unifying token store, ticket store and congestion data store in a subsequent PR.)

I followed the discussion that took place on the QUIC mailing list ; thank you for clarifying the general design of QUIC that does not use a special version of TLS. This explains why the proposed approach may not be relevant for large scale deployment - but can still be useful for experimenting congestion controls exploiting 0-RTT-BDP.

@NicoKos
Copy link

NicoKos commented Oct 27, 2020

I am looking at https://tools.ietf.org/html/draft-kuhn-quic-0rtt-bdp-07. It documents three parameters:

* recon_bytes_in_flight

* recon_min_rtt

* recon_max_pkt_number

Your PR adds CWIN, RTT variant and MAX ACK DELAY. This is questionable. CWIN is very much the same concept as "bytes in flight", and there is already a MAX ACK DELAY transport parameter. I think we should have a discussion with authors of the draft before adding these extra parameters.

Adding these parameters illustrates the flexibility of the BDP_METADA approach but all listed parameters here may not be actually relevant.

@NicoKos
Copy link

NicoKos commented Oct 27, 2020

I am also looking at the interaction between the transport parameters and the congestion control algorithms. I would express to see some additional code in the implementations of Cubic or BBR to process the additional initial parameters, but I don't see that. How will that work?

There is an appendix in the 0-RTT-BDP a strawman algorithm to safely jump to previously measured BDP ; AFAIU this implementation jumps direclty to previously measured BDP. @francklinSIMO ?

This brings another question, regarding testing. A general requirement is that we add tests in the "picoquic test" code to check that the extensions behave as expected. This includes basic tests to verifying parsing and logging, as well as functional tests. If you look at the current test suite, there are functional tests of satellite transmission with different congestion control algorithms. We should have variants of these tests when the extensions are used, to demonstrate that the implementation uses the new parameters as intended.

There is indeed a test required to see whether the implementation works as expected. The implementation has been validated through the usage of https://qvis.edm.uhasselt.be

We are working on an updated version of the quic-4-sat draft to include tests with 0-RTT and its extensions.

@huitema
Copy link
Collaborator

huitema commented Oct 28, 2020

OK, so I think I understand a way forward. I have two issues with the proposed approach, the reliance on TLS, and the lack of integration with the general architecture. I would propose that we split the work in several parts:

  1. memorize key path characteristics in the QUIC context, maybe in an LRU list. That can be done on both client and server. I think the list should be indexed by IP address of the peer (port number is too variable). Write test to verify that these values can be stored and retrieved. Requires adding a BDP metadata callback for selected congestion algorithms, to obtain the current value of the data.

  2. add code to path creation to get initial variables from the path-characteristic LRU, and add code in at least Cubic and BBR to retrieve these values when the path is initiated -- maybe add a congestion control callback to signal that initial values are available. Write test to verify that if these values are present, a download goes faster.

  3. add code on the client side to store or retrieve the values from a file. Write test to verify that the values are properly stored and retrieved.

  4. add code on the server side to store the value in a "NEW TOKEN" (NOT a TLS ticket). This is in line with the discussion in the WG. Add code on the client side to only keep the last value of the NEW TOKEN if several are presented during a connection. Add test to verify that the TOKEN stored by the client matches the last value pushed by the server.

  5. add code on the server side to retrieve parameters from a NEW TOKEN if presented during the connection, and update the congestion control algorithm for the default path if the NEW TOKEN is accepted (i.e., not too old, and specific to the correct IP address). Add or update test to verify that the connection goes faster in these cases.

I still do not understand why you need something specific for 0-RTT -- I think the approach above solves the issue for both client uploads and client downloads. But if that's actually an issue, consider the extensions then.

Note that there is no actual need to negotiate transport parameters. Caching of connection data is an unilateral action by each implementation, client and server do not need to coordinate on that.

@NicoKos
Copy link

NicoKos commented Oct 29, 2020

We try to sum up the discussions and what we need the QUIC WG to discuss before actual implementation.

@francklinSIMO francklinSIMO deleted the branch private-octopus:master April 22, 2021 18:10
@francklinSIMO francklinSIMO deleted the master branch April 22, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants