http3: changing how http3 is built #15540
Conversation
Signed-off-by: Alyssa Wilk <[email protected]>
repokitteh-read-only
bot
added
api
deps
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
|
ah, looks like macos failure is real, |
Signed-off-by: Alyssa Wilk <[email protected]>
| @@ -127,7 +127,7 @@ namespace quic { | |||
| using QuicLogLevel = spdlog::level::level_enum; | |||
|
|
|||
| static const QuicLogLevel TRACE = spdlog::level::trace; | |||
| static const QuicLogLevel DEBUG = spdlog::level::debug; | |||
| static const QuicLogLevel QDEBUG = spdlog::level::debug; | |||
There was a problem hiding this comment.
cc @danzh2010 David said you had a workaround for this - lmk if this conflicts or you have a better plan? Honestly I was surprised this compiled, having assumed we needed to match the google log defines...
There was a problem hiding this comment.
There was a problem hiding this comment.
This works. I'll have a more complete fix which will bring back DEBUG (this time without breaking macOS) ASAP. Feel free to merge this.
There was a problem hiding this comment.
The other fix I mentioned is at #15616
| "envoy.listener.quic": "//source/extensions/quic_listeners/quiche:quic_factory_lib", | ||
| "envoy.transport_sockets.quic": "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib", |
There was a problem hiding this comment.
Is your plan to follow up here and completely remove these as extensions? TODO? Also this will allow us to use the GSO writer for udp_proxy also and remove all of that config as well. TODO around that?
There was a problem hiding this comment.
hm, I didn't think having quic as a "required" extension made any less sense than tls as a required extension, but if you want to remove it entirely we can. Want me to do that here, or a follow up?
I wasn't planning on following up on the GSO bits, I was largely hoping to just make the QUIC hackery I'm dealing with less hacky. Would you or @danzh2010 be up for tackling GSO as a followup?
There was a problem hiding this comment.
hm, I didn't think having quic as a "required" extension made any less sense than tls as a required extension, but if you want to remove it entirely we can. Want me to do that here, or a follow up?
TLS as an extension is "required" because of the openssl stuff. I think if we completely remove QUIC as an extension it will clean up a lot of the places I have commented on where we are referencing extension code in core code and vice versa. I'm happy to do some of this work as a side project if you don't have time but let's add the TODO either way?
I wasn't planning on following up on the GSO bits, I was largely hoping to just make the QUIC hackery I'm dealing with less hacky. Would you or @danzh2010 be up for tackling GSO as a followup?
Yes I can work on this as part of ^ if you don't want to do it now.
Signed-off-by: Alyssa Wilk <[email protected]>
|
/retest |
|
Retrying Azure Pipelines: |
bazel/README.md
Outdated
| @@ -634,7 +634,7 @@ The following optional features can be disabled on the Bazel build command-line: | |||
| * tcmalloc with `--define tcmalloc=disabled`. Also you can choose Gperftools' implementation of | |||
| tcmalloc with `--define tcmalloc=gperftools` which is the default for builds other than x86_64 and aarch64. | |||
| * deprecated features with `--define deprecated_features=disabled` | |||
|
|
|||
| * http3/quic with --define `http3=disabled` | |||
There was a problem hiding this comment.
This need to be updated to --//bazel:http3=False
Signed-off-by: Alyssa Wilk <[email protected]>
|
Can I get an API stamp too? |
|
oh, dependency change? oy |
|
/lgtm deps |
repokitteh-read-only
bot
removed
the
deps
Will undraft and tag Matt/Dan for review once I make sure various CIs are Ok.
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
Much of #12829