http3: prepare for HTTP Datagrams support #25925

jeongseokson · 2023-03-06T00:41:23Z

Commit Message:
This commit prepares Envoy for HTTP Datagrams and the Capsule Protocol (RFC 9297) support. The added CapsuleProtocolHandler is used to decode received HTTP/3 Datagrams into Capsules, and encode Capsules into HTTP/3 Datagrams in the EnvoyQuicClientStream and EnvoyQuicServerStream.

This commit does not enable HTTP Datagrams support in Envoy, so it does not change the behavior of Envoy under any circumstances. A subsequent CL will enable the feature with CONNECT-UDP support.

Additional Description:
Risk Level: Low. The current commit does not change the behavior of Envoy.
Testing: unit tests
Docs Changes: N/A
Release Notes: Add a build feature envoy_http_datagrams to allow disabling HTTP Datagram support.

CapsuleProtocolHandler decodes received HTTP/3 Datagrams into Capsules, and encodes Capsules into HTTP/3 Datagrams. This commit does not change the behavior of Envoy. See the comments in EnvoyQuicClientSession and EnvoyQuicServerSession classes for more details. Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson · 2023-03-06T00:47:03Z

/assign @DavidSchinazi

Hi David, can you review the commit in this PR? I left a few questions you could answer in some TODOs.

alyssawilk · 2023-03-06T16:33:29Z

Exciting! I'll let @DavidSchinazi do first pas.
Also interested in his and @RyanTheOptimist 's take on if this will eventually be a large enough code path we may want to have a compile-out option for EM binary size

RyanTheOptimist · 2023-03-06T20:32:39Z

Looks like clang-tidy is failing: https://dev.azure.com/cncf/envoy/_build/results?buildId=129883&view=logs&jobId=b7634614-24f3-5416-e791-4f3affaaed6c&j=b7634614-24f3-5416-e791-4f3affaaed6c&t=21e6aa7d-f369-5abd-5e4e-e888cac18e9c

jeongseokson · 2023-03-06T21:44:47Z

Thanks for pointing it out, Ryan. I will fix the clang-tidy errors as well as the msan/asan ones.

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

DavidSchinazi

Thanks for this! First pass complete

source/common/quic/capsule_protocol_handler.h

source/common/quic/envoy_quic_server_session.h

source/common/quic/envoy_quic_server_stream.cc

source/common/quic/envoy_quic_server_stream.h

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

alyssawilk · 2023-03-14T17:11:02Z

/wait on CI (though you can ping David if you think it's ready for another review pass)

jeongseokson · 2023-03-14T17:43:49Z

Thanks for the reminder, Alyssa. David will take a look at it soon. I will fix the CI errors in the meantime.

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

DavidSchinazi

Nice progress!

source/common/quic/capsule_protocol_handler.h

source/common/quic/capsule_protocol_handler.cc

source/common/quic/envoy_quic_client_session.h

source/common/quic/envoy_quic_server_session.h

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

alyssawilk · 2023-03-15T20:54:56Z

/wait on CI

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson · 2023-03-15T22:33:29Z

/wait on CI

RyanTheOptimist · 2023-03-15T22:46:01Z

@jeongseokson GitHub UI/Usability is kinda terrible. But if you reply to comments from the "Files Changed" view instead of the "Conversation" view, you can do "Start Review" in which case all of your comments will be sent in a single email instead of one email per comment. Just FYI

jeongseokson · 2023-03-15T23:05:58Z

@RyanTheOptimist Ah! I didn't know that. Thanks for the information, Ryan.

RyanTheOptimist · 2023-03-15T23:10:00Z

@RyanTheOptimist Ah! I didn't know that. Thanks for the information, Ryan.

Sure thing! It's completely undiscoverable. sigh I'm glad we have GitHub but sometimes I find it really annoying.

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

RyanTheOptimist

Looks great. Just a few small comments.

source/common/quic/envoy_quic_client_stream.cc

source/common/quic/http_datagram_handler.h

source/common/http/header_utility.cc

source/common/quic/envoy_quic_client_stream.h

source/common/quic/envoy_quic_server_stream.cc

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

RyanTheOptimist

Looks good to me! @alyssawilk to give this a final review.

source/common/quic/envoy_quic_server_stream.cc

jeongseokson · 2023-03-18T05:15:42Z

/retest

repokitteh-read-only · 2023-03-18T05:15:48Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #25925 (comment) was created by @jeongseokson.

see: more, trace.

alyssawilk · 2023-03-20T13:29:44Z

@mattklein123 you Ok with datagram support 1) being added and 2) being treated like HTTP/3 and getting google-only sign off rather than requiring cross-company LGTMs?

alyssawilk

Looks great overall, just a few final comments
/wait

.github/workflows/compile_time_options.yml

bazel/envoy_select.bzl

alyssawilk · 2023-03-20T13:23:37Z

mobile/.bazelrc

 build --define envoy_mobile_listener=disabled
 build --define=library_autolink=disabled
 build --define=envoy_mobile_swift_cxx_interop=enabled
+build --define=envoy_enable_http_datagram=disabled


cc @RenjieTang we'll want to do this for android/iOS when this is imported

source/common/http/header_utility.cc

source/common/http/header_utility.h

mattklein123 · 2023-03-20T14:27:30Z

@mattklein123 you Ok with datagram support 1) being added and 2) being treated like HTTP/3 and getting google-only sign off rather than requiring cross-company LGTMs?

Sure SGTM!

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson · 2023-03-21T03:07:20Z

/retest

repokitteh-read-only · 2023-03-21T03:07:27Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #25925 (comment) was created by @jeongseokson.

see: more, trace.

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson

Thank you for the comments, Alyssa. Please take a look at the commits and let me know if you have any further comments.

.github/workflows/compile_time_options.yml

source/common/http/header_utility.cc

bazel/envoy_select.bzl

source/common/http/header_utility.h

jeongseokson · 2023-03-21T06:25:00Z

/retest

repokitteh-read-only · 2023-03-21T06:25:08Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #25925 (comment) was created by @jeongseokson.

see: more, trace.

alyssawilk

yeah thanks for the rename and with one more offline round wiht Ryan on multiple headers I've got no objections. I'll defer to him for merge :-)

RyanTheOptimist

Looks great! Thanks for doing this!

repokitteh-read-only bot assigned DavidSchinazi Mar 6, 2023

mattklein123 assigned alyssawilk Mar 6, 2023

Fix clang-tidy and msan/asan errors.

4ed53ac

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

DavidSchinazi reviewed Mar 7, 2023

View reviewed changes

alyssawilk added the waiting label Mar 7, 2023

jeongseokson added 2 commits March 8, 2023 00:31

Fix additional msan error.

72e32f9

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

Resolve code comments.

6151024

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

repokitteh-read-only bot removed the waiting label Mar 13, 2023

Fix clang tidy errors.

a35f9a4

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

Add a bytes sent tracker.

59db689

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

DavidSchinazi reviewed Mar 15, 2023

View reviewed changes

jeongseokson added 2 commits March 15, 2023 18:22

Add a test case for IsCapsuleProtocol.

0759d10

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

Merge branch 'main' into h3_datagram_capsule_support

3df9c11

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

repokitteh-read-only bot added the waiting label Mar 15, 2023

Resolve comments.

3297093

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

repokitteh-read-only bot removed the waiting label Mar 15, 2023

repokitteh-read-only bot added the waiting label Mar 15, 2023

jeongseokson changed the title ~~http3: add support for HTTP Datagrams~~ http3: prepare for HTTP Datagrams support Mar 15, 2023

repokitteh-read-only bot removed the waiting label Mar 17, 2023

Merge branch 'main' into h3_datagram_capsule_support

7ab3151

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

RyanTheOptimist reviewed Mar 17, 2023

View reviewed changes

Resolve comments.

bfdc360

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

RyanTheOptimist previously approved these changes Mar 18, 2023

View reviewed changes

source/common/quic/envoy_quic_server_stream.cc Outdated Show resolved Hide resolved

alyssawilk reviewed Mar 20, 2023

View reviewed changes

repokitteh-read-only bot added the waiting label Mar 20, 2023

jeongseokson added 2 commits March 21, 2023 00:36

Resolve comments.

6ce61d3

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

Merge branch 'main' into h3_datagram_capsule_support

766d9b1

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson dismissed RyanTheOptimist’s stale review via 766d9b1 March 21, 2023 00:41

repokitteh-read-only bot removed the waiting label Mar 21, 2023

Fix build errors.

51a8058

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson added 2 commits March 21, 2023 03:30

Fix error.

d557281

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

Fix YAML comment issue.

5efe1e1

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>

jeongseokson commented Mar 21, 2023

View reviewed changes

.github/workflows/compile_time_options.yml Outdated Show resolved Hide resolved

source/common/http/header_utility.cc Show resolved Hide resolved

bazel/envoy_select.bzl Outdated Show resolved Hide resolved

source/common/http/header_utility.h Outdated Show resolved Hide resolved

alyssawilk reviewed Mar 21, 2023

View reviewed changes

alyssawilk removed their assignment Mar 21, 2023

RyanTheOptimist approved these changes Mar 21, 2023

View reviewed changes

RyanTheOptimist merged commit 7755d31 into envoyproxy:main Mar 21, 2023

DavidSchinazi mentioned this pull request Apr 26, 2023

connect-udp support (RFC 9298) #23564

Closed

jeongseokson mentioned this pull request Jun 8, 2023

http: add CONNECT-UDP support #27714

Merged

http3: prepare for HTTP Datagrams support #25925

http3: prepare for HTTP Datagrams support #25925

Uh oh!

Conversation

jeongseokson commented Mar 6, 2023 • edited by RyanTheOptimist Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeongseokson commented Mar 6, 2023

Uh oh!

alyssawilk commented Mar 6, 2023

Uh oh!

RyanTheOptimist commented Mar 6, 2023

Uh oh!

jeongseokson commented Mar 6, 2023

Uh oh!

DavidSchinazi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alyssawilk commented Mar 14, 2023

Uh oh!

jeongseokson commented Mar 14, 2023

Uh oh!

DavidSchinazi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alyssawilk commented Mar 15, 2023

Uh oh!

jeongseokson commented Mar 15, 2023

Uh oh!

RyanTheOptimist commented Mar 15, 2023

Uh oh!

jeongseokson commented Mar 15, 2023

Uh oh!

RyanTheOptimist commented Mar 15, 2023

Uh oh!

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeongseokson commented Mar 18, 2023

Uh oh!

repokitteh-read-only bot commented Mar 18, 2023

Uh oh!

alyssawilk commented Mar 20, 2023

Uh oh!

alyssawilk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alyssawilk Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeongseokson commented Mar 6, 2023 •

edited by RyanTheOptimist

Loading