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

docs: unhiding HTTP/3 and adding docs #15926

Merged
merged 27 commits into from
May 7, 2021
Merged

Conversation

alyssawilk
Copy link
Contributor

@alyssawilk alyssawilk commented Apr 12, 2021

Unhiding HTTP/3 upstream and downstream configuration, linking to example configs, and updating docs for HTTP/3 alpha.

Risk Level: n/a
Testing: n/a
Docs Changes: yes
Release Notes: inline
#14829 #2557 #15845
Fixes #12923

Co-Authored-By: Michael Payne michael@sooper.org

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #15926 was opened by alyssawilk.

see: more, trace.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@mattklein123
Copy link
Member

Thanks in general this looks amazing! So excited to see this land, especially with example configs and maybe even a sandbox? (cc @moderation @phlax)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Contributor

@danzh2010 danzh2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's a lot of text! Thanks for drafting it!

@@ -13,7 +13,8 @@ The gRPC bridge sandbox is an example usage of Envoy's
This is an example of a key-value store where an ``http``-based client CLI, written in ``Python``,
updates a remote store, written in ``Go``, using the stubs generated for both languages.

The client send messages through a proxy that upgrades the HTTP requests from ``http/1.1`` to ``http/2``.
The client send messages through a proxy that upgrades the HTTP requests from ``http/1.1`` to ``http/2`` or
``http/3``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP/3 does not support the HTTP Upgrade mechanism https://tools.ietf.org/id/draft-ietf-quic-http-23.html#http-upgrade

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe we need better phrasing here overall, but basically you use Upgrade for http/1 and CONNECT for H2 and H2 (as soon as david adds support)

@phlax
Copy link
Member

phlax commented Apr 13, 2021

@alyssawilk alyssawilk added the no stalebot Disables stalebot from closing an issue label Apr 15, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).

🐱

Caused by: #15926 was synchronize by alyssawilk.

see: more, trace.

@alyssawilk
Copy link
Contributor Author

oops, pushed early, /wait

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk marked this pull request as ready for review April 29, 2021 15:52
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is fantastic! Thanks for writing all this up. One or two questions but mostly some minor nits.

// QUIC support is currently alpha and should be used with caution.
//
// For known outstanding issues before Envoy QUIC support is GA, you can
// track https://github.com/envoyproxy/envoy/labels/quic-mvp for example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that "For example" is the start of a new sentence? If so, perhaps a period and then Capitalize "for". If it's not the start of a new sentence, perhaps it can be rephrased for clarity?

//
// For known outstanding issues before Envoy QUIC support is GA, you can
// track https://github.com/envoyproxy/envoy/labels/quic-mvp for example
// QUIC does not currently support in place filter chain updates, so users
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I feel like 'in-place' is how I am accustomed to seeing this phrase in the context of software. (As opposed to, say, "having everything in place"). Maybe? WDYT?

// requiring dynamic config reload for QUIC should wait until
// https://github.com/envoyproxy/envoy/issues/13115 has been addressed.
//
// For general feature requets beyond production readiness, you can track
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "requets" => "requests"

// present. If HTTP/3 is present, attempts to connect will first be made
// via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2
// based on ALPN) will be used instead.
// present, and will eventually rely on server-side alt-svc advertisement to try HTTP/3.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Alt-Svc, I'd be inclined to say "HTTP Alternative Services" and I might also reference the spec https://tools.ietf.org/html/rfc7838

Is it worth mention the HTTPS DNS RR here as well, since we intend to implement this? (and the spec https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04)

// via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2
// based on ALPN) will be used instead.
// present, and will eventually rely on server-side alt-svc advertisement to try HTTP/3.
// If HTTP/3 is configured via HTTP/3, and if the HTTP/3 connection fails (currently controlled by a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wonder if we can improve on the phrase "If HTTP/3 is configured via HTTP/3". How bout something like:

When HTTP/3 is configured, HTTP/3 connections will be attempted. However, the connection may fail over to TCP (HTTP/1 or HTTP/2 based on ALPN) instead. If the HTTP/3 connection fails explicitly because of a protocol error, a TCP connection will be attempted instead. In addition, if the HTTP/3 connection has not succeeded after a (currently) fixed timeout, a TCP connection will be started in a parallel. The connection the succeeds first will be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, having written all that... I think that's only true when H1, H2 and H3 are all enabled. If only H3 is enabled then there's no fallback and not alt-svc checking, right? Should we mention this? (Wow this gets complex in a hurry!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, for auto, remember that H2 and H1 are on by default, so you can't not configure it.

HTTP3 upstream
===============

HTTP/3 downstream support is still in Alpha, and should be used with caution.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "downstream" => "upstream"

See example :repo:`downstream HTTP/3 configuration </configs/envoyproxy_io_proxy_http3_downstream.template.yaml1>` for example configuration.

Note that the example configuration includes both a TCP and a UDP listener, and the TCP
listener is advertising http/3 support via alt-svc header. Advertising HTTP/3 is not necessary for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "via alt-svc header" => "via an alt-svc header"

Envoy’s HTTP connection manager has native support for HTTP/1.1, WebSockets, and HTTP/2. It does not support
SPDY. Envoy’s HTTP support was designed to first and foremost be an HTTP/2 multiplexing proxy.
Envoy’s HTTP connection manager has native support for HTTP/1.1, WebSockets, HTTP/2 and HTTP/3. It does not support
SPDY. Envoy’s HTTP support was designed to first and foremost be an HTTP/2+ multiplexing proxy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to keep this as "HTTP/2" since when Envoy was designed HTTP/3 was not yet a thing. Your call though.

.. _arch_overview_http3_upstream:

If HTTP/3 is configured in the automatic pool, it will currently attempt an QUIC connection first,
then 300ms later, attempt a TCP connection. Whichever handshake succeeds will be used for the initial
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this doesn't need to be mentioned, but TCP is only attempted after the timeout if the QUIC connection has not already succeeded, right?

stream, but if both TCP and QUIC connections are established, QUIC will eventually be preferred.

Upcoming versions of HTTP/3 support will include only selecting HTTP/3 if the server advertises support
via alt-svc headers, and "QUIC hints" where attempting QUIC but failing over can be hard-coded. This code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention the HTTPS DNS RR?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Contributor Author

I moved most of the details to the docs, where they'd been half in docs and half in APIs.
still torn about having upstream H3 details in the connection pool doc or the H3 doc - thoughts appreciated!

@alyssawilk
Copy link
Contributor Author

cc @moderation if you have any tips on how I can make docs more clear now that you've played around, please add suggestions here!

optimal latency for internet environments, so please be patient and follow along with Envoy release notes
Upcoming versions of HTTP/3 support will include only selecting HTTP/3 if the upstream advertises support
either via `HTTP Alternative Services <https://tools.ietf.org/html/rfc7838>`_,
`HTTPS DNS RR <https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04>`_, "QUIC hints" which
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "or" before QUIC hints, I think

alyssawilk added 3 commits May 5, 2021 10:28
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alyssawilk looks good generally

ive left a few nits inline - quite a few are backticks - some i think are probably more important than others, but feel free to ignore some

api/envoy/config/core/v3/protocol.proto Show resolved Hide resolved
docs/root/configuration/http/http_filters/lua_filter.rst Outdated Show resolved Hide resolved
@@ -140,9 +140,14 @@ upstream.healthy_panic_threshold
Defaults to 50%.

upstream.use_http2
Whether the cluster utilizes the *http2* if configured in `HttpProtocolOptions <envoy_v3_msg_config.upstreams.http.v3.HttpProtocolOptions>`.
Whether the cluster uses *http2* if configured in `HttpProtocolOptions <envoy_v3_msg_config.upstreams.http.v3.HttpProtocolOptions>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one also i think should be double-backtick

docs/root/faq/configuration/timeouts.rst Outdated Show resolved Hide resolved
docs/root/intro/arch_overview/http/http3.rst Show resolved Hide resolved
docs/root/intro/arch_overview/http/http3.rst Outdated Show resolved Hide resolved
docs/root/intro/deployment_types/service_to_service.rst Outdated Show resolved Hide resolved
@@ -1134,6 +1134,7 @@ subtypes
subzone
superclass
superset
svc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probs dont needs this ifwe make use inline literal for alt-svc

alyssawilk added 2 commits May 5, 2021 14:33
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@adisuissa
Copy link
Contributor

/lgtm api

@repokitteh-read-only repokitteh-read-only bot removed the api label May 6, 2021
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Adding an informative log since I ended up confused when trying to sort out envoyproxy#15845 and needed to fix envoyproxy#15926 accordingly.

Risk Level: n/a (adding info log)
Testing: manual
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Gokul Nair <gnair@twitter.com>
@alyssawilk
Copy link
Contributor Author

@RyanTheOptimist / @phlax I miss anything / any other requests, or is this good to go once #16320 lands?

@@ -1,7 +1,10 @@
.. _arch_overview_http3:

HTTP3 overview
==============

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whatever we keep in the api docs - i think a warning is warranted here too

See example :repo:`downstream HTTP/3 configuration </configs/envoyproxy_io_proxy_http3_downstream.template.yaml1>` for example configuration.

Note that the example configuration includes both a TCP and a UDP listener, and the TCP
listener is advertising http/3 support via an ``alt-svc header``. Advertising HTTP/3 is not necessary for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the backticks should just be around alt-svc as that is the literal part - my rule of thumb ~is "should it be translated" if we were translating the docs

alyssawilk added 2 commits May 6, 2021 09:14
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
By default the example configuration uses kernel UDP support, but for production performance on Linux, use of
BPF is strongly advised if Envoy is running with multiple worker threads. Envoy will attepmt to
use BPF by default if multiple worker threads are configured, but may require root, or at least sudo-with-permissions
(e.g. sudo setcap cap_bpf+ep). Envoy will log a warning on start-up if BPF is attempted and fails.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also worth to mention that if BPF is not supported by the platform and multiple worker threads are configured, Envoy will route packets across workers by itself and log a warning of performance degrading, or something similar.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I could have sworn I approved several days ago, but github disagrees. LGTM

Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @alyssawilk


Downstream Envoy HTTP/3 support can be turned up via adding
:ref:`quic_options <envoy_v3_api_field_config.listener.v3.UdpListenerConfig.quic_options>` and
ensuring the downstream transport socket is a QuicDownstreamTransport.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HCM config needs to specify codec type to be Http3 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, at the point you configure a listener, the rest should be auto-validated also shown by the sample config.
happy to add the detail in a follow-up but I think i'm going to land this version today :-)

@adisuissa
Copy link
Contributor

/lgtm api

@repokitteh-read-only repokitteh-read-only bot removed the api label May 7, 2021
@alyssawilk alyssawilk merged commit 73ade9a into envoyproxy:main May 7, 2021
alyssawilk added a commit to alyssawilk/envoy that referenced this pull request May 10, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk added a commit that referenced this pull request May 12, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the h3_docs branch August 18, 2021 13:59
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.

QUIC/HTTP3: docs
8 participants