Add an option to disable QPACK for HTTP/3. #38775
Conversation
Signed-off-by: Haoyue Wang <haoyuewang@google.com>
|
Hi @wang178c, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/assign @RyanTheOptimist |
|
wang178c is not allowed to assign users. |
|
/assign @RyanTheOptimist |
RyanTheOptimist
assigned RyanTheOptimist and mattklein123 and unassigned mattklein123 and RyanTheOptimist
There was a problem hiding this comment.
Looks great! Thanks for doing this. Any chance we could add some sort of test in envoy_quic_server_stream_test.cc and envoy_quic_client_stream_test.cc? Might be hard to tell if it "works" but if we could at least test it out. Perhaps we could add something to quic_http_integration_test.cc which enables this in the config and just makes sure that HTTP works as expected?
|
|
||
| // Disables QPACK compression related features for HTTP/3 including: | ||
| // No huffman encoding, zero dynamic table capacity and no cookie crumbing. | ||
| bool disable_qpack = 7; |
There was a problem hiding this comment.
You'll need to update the the changelog to mention this new API changelogs/current.yaml.
There was a problem hiding this comment.
It would also be nice to mention why someone might want to do this?
…pack Signed-off-by: Haoyue Wang <haoyuewang@google.com>
Signed-off-by: Haoyue Wang <haoyuewang@google.com>
Signed-off-by: Haoyue Wang <haoyuewang@google.com>
wang178c
force-pushed
the
option_to_disable_qpack
branch
from
8c8091c to
d63dbfd
Compare
Thanks. Added two session uint tests. For the integration test, I only add the client coverage as it seems quite involved to plumb the Http3PtotocolOptions. These tests nicely catch bugs from previous commit. |
| // | ||
| // Disables QPACK compression related features for HTTP/3 including: | ||
| // No huffman encoding, zero dynamic table capacity and no cookie crumbing. | ||
| // This can be useful when an upstream HTTP/3 connection multiplexes multiple downstream connections. |
There was a problem hiding this comment.
I think I would mention here that this is useful for trading off CPU vs bandwidth.
changelogs/current.yaml
Outdated
| @@ -280,5 +280,9 @@ new_features: | |||
| change: | | |||
| Added an :ref:`io_uring <envoy_v3_api_field_extensions.network.socket_interface.v3.DefaultSocketInterface.io_uring_options>` option in | |||
| default socket interface to support io_uring. | |||
| - area: http3 | |||
| change: | | |||
| Added experimental support for disabling QPACK compression. | |||
There was a problem hiding this comment.
Please link to the new API field.
Signed-off-by: Haoyue Wang <haoyuewang@google.com>
Signed-off-by: Haoyue Wang <haoyuewang@google.com>
|
Updated the PR as |
Commit Message: Add an option to disable QPACK for HTTP/3.
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]