mobile: support QUIC connection migration #42104

danzh2010 · 2025-11-18T20:25:53Z

Commit Message: this PR adds support of QUIC connection migration. To achieve this, this PR made several change:

Added an extension point in createPersistentQuicInfoForCluster() for a custom QUIC client packet writer. The config API is QuicProtocolOptions.client_packet_writer. And provided an extension implementation QuicPlatformPacketWriterFactory for mobile which knows how to bind sockets to given network handles provided by Android.
Changed QuicNetworkConnectivityObserverImpl to actually propagate network change events (connected, disconnected, became defualt) of a specific network to QuicSession's migration manager interface which makes decision whether and how to migrate the connection. Added 2 new interfaces getDefaultNetwork() and getAlternativeNetwork() to EnvoyQuicNetworkObserverRegistry and implemented them in the subclass EnvoyMobileQuicNetworkObserverRegistry using the network states cached in ConnectivityManagerImpl. These interfaces are used by createQuicNetworkConnection() and EnvoyQuicMigrationHelper to pass network information to the packet writer factory and QUICHE respectively.
Also added a config API QuicProtocolOptions.connection_migration for upstream cluster to configure several migration options, i.e. whether to migrate an idle QUIC connection or not, etc. (Currently hidden from doc) And add mobile APIs setEnableConnectionMigration() to Android engine builders to populate this config knob in the bootstrap config. Note that if this knob is configured, QuicProtocolOptions.client_packet_writer must be configured with a packet writer extension that supports binding socket to a given network handle of the platform's own definition. And Android engine builders will automatically plug in QuicPlatformPacketWriterFactory extension in such case.

Currently we handles 4 network change events with connection migration if there is alternative network to use in Android:

the current network gets disconnected;
a different network gets picked as the default by the platform;
a packet write error occurs;
QuicConnection detects path degrading.

Migration between different ports on the same network and to a different server address is already supported.

To enable connection migration in Cronvoy engine, these APIs need to be called:
setEnableConnectionMigration(true)
setUseV2NetworkMonitor(true)
addRuntimeGuard("drain_pools_on_network_change", true) // default false, needed to disable request rehash during conn pool picking

The feature also depends on these default-true runtime guards:
envoy.reloadable_features.mobile_use_network_observer_registry
envoy.reloadable_features.decouple_explicit_drain_pools_and_dns_refresh // needed to disable request rehash during conn pool picking
If any of them are turned off, migration needs to be turned off as well.

Additional Description: use QuicPlatformPacketWriterFactory extension by default in examples/java/hello_world:hello_envoy Java app to ensure correct interaction with Android APIs.

Risk Level: low, the feature is default off
Testing: new integration tests added
Docs Changes: Y
Release Notes: N
Platform Specific Features: QUIC connection migration on Android

Signed-off-by: Dan Zhang <[email protected]>

repokitteh-read-only · 2025-11-18T20:26:06Z

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

🐱

Caused by: #42104 was opened by danzh2010.

see: more, trace.

Signed-off-by: Dan Zhang <[email protected]>

RyanTheOptimist · 2025-11-20T04:40:08Z

@abeyad can you take a first pass?

Signed-off-by: Dan Zhang <[email protected]>

abeyad

This is great, thanks @danzh2010 !

api/envoy/config/core/v3/protocol.proto

abeyad · 2025-11-20T16:33:04Z

mobile/library/cc/engine_builder.cc

+      }
+    }
+
+    if (use_quic_platform_packet_writer_ || enable_connection_migration_) {


why would use_quic_platform_packet_writer_ be set if connection migration is not enabled?

use_quic_platform_packet_writer_ can be used on Android regardless of enabling migration or not. As long as the platform support network handles.

abeyad · 2025-11-20T16:35:09Z

mobile/library/common/extensions/quic_packet_writer/platform/platform_packet_writer_factory.h

+
+private:
+  Network::ConnectionSocketPtr
+  createConnectionSocketOnGivenNetwork(Network::Address::InstanceConstSharedPtr peer_addr,


where is the definition of this method?

This is no longer needed.

abeyad · 2025-11-20T16:46:36Z

mobile/library/common/system/default_system_helper_apple.cc

 }

+void DefaultSystemHelper::bindSocketToNetwork(Network::ConnectionSocket&, int64_t) {
+  PANIC("unreachable");


why panicking here for Apple? wouldn't this get called by platform_packet_writer_factory.cc on any platform?

2 choice here: 1) Apple will continue using the default packet writer. It doesn't propagate the network handle to QUICHE anyway. 2) using the extension packet writer as well, but because of lack of network handle, this function won't be called.

Sounds like this deserves a comment in the code. Since this class explicitly has "Apple"in the name, it is assumed to work on Apple platforms, I think. So if we think choice 1 is a valid option, then perhaps we should nuke the class? But option 2 seems plausible and worth a comment?

"nuke the class" means leave this method body empty?
Anyway, I added comment about why this is unreachable.

mobile/test/java/org/chromium/net/CronetHttp3Test.java

abeyad · 2025-11-20T16:49:54Z

source/common/quic/envoy_quic_client_packet_writer_factory.h

+
+  struct CreationResult {
+    // Not null.
+    std::unique_ptr<EnvoyQuicPacketWriter> writer_;


I'm not a big fan of the using declarations with EnvoyQuicPacketWriterPtr, but up to you if you want to do that here for consistency

me either, especially this class name isn't too long. I'll leave as is.

Signed-off-by: Dan Zhang <[email protected]>

abeyad

LGTM, will let @RyanTheOptimist review as an Envoy maintainer

abeyad · 2025-11-26T15:38:24Z

/assign @botengyao

for API review

abeyad · 2025-11-26T16:02:30Z

/assign @adisuissa

for API review

adisuissa

Thanks. Left a few comments about the API.

adisuissa · 2025-11-26T16:11:02Z

api/envoy/config/core/v3/protocol.proto

+// picked by the platform. A connection is this special state is only allowed to
+// serve new requests for a certain period of time before being drained, and
+// meanwhile, QUIC will try to migrate to the default network if possible.
+message ConnectionMigrationSettings {


If this is QUIC only settings (and will not be used in other places), I think it should be inside the QuicProtocolOptions).

adisuissa · 2025-11-26T16:12:07Z

api/envoy/config/core/v3/protocol.proto

+// network change events from the platform, i.e. the current network gets
+// disconnected, or upon the QUIC detecting a bad connection. After migration, the
+// connection may be on a different network other than the default network
+// picked by the platform. A connection is this special state is only allowed to


Suggested change

// picked by the platform. A connection is this special state is only allowed to

// picked by the platform. A connection in this special state is only allowed to

adisuissa · 2025-11-26T16:16:08Z

api/envoy/config/core/v3/protocol.proto

+  // if it hasn't been idle for longer than this idle period. Otherwise, the
+  // connection will be closed instead.
+  // Default to 30s.
+  google.protobuf.Duration max_idle_time_before_migration = 2


IIUC this field is highly coupled with migrate_idle_connections being set to true.
If that's the case, will it make sense to create a new type (e.g., IdleConnectionsMigration) that has the max_idle_time... field there?
There will be an idle_connections_migration field that if not set, the idle connections will be closed upon a migration signal.

adisuissa · 2025-11-26T16:18:08Z

api/envoy/config/core/v3/protocol.proto

+  bool migrate_idle_connections = 1;
+
+  // If idle connections are allowed to be migrated, only migrate the connection
+  // if it hasn't been idle for longer than this idle period. Otherwise, the


Just top make sure I understand correctly, this implies that once a migration signal was received, the implementation will look "backwards" at all the connections and see if any of them has been idle (according to the set timeout), and if they are not, they will be migrated. Is that correct?
(I just want to make sure that this doesn't imply that once a migration signal arrives, a timer starts for all the connections to see which is idle and which is not).

Just top make sure I understand correctly, this implies that once a migration signal was received, the implementation will look "backwards" at all the connections and see if any of them has been idle (according to the set timeout), and if they are not, they will be migrated. Is that correct?

Yes

adisuissa · 2025-11-26T16:23:16Z

api/envoy/config/core/v3/protocol.proto

+  // After migrating to a non-default network interface, the connection will
+  // only be allowed to stay on that network for up to this period of time before
+  // being drained unless it migrates to the default network or that network
+  // gets picked as the default by the device by then.


Sounds like many edge cases and races may be introduced by this :)
I think one thing that is missing is the definition of "default network". I guess it is a QUIC internal definition.
Can you please add a link in the first mention of default network in this proto, that explains what's its definition.

default network is not a QUIC terminology, but one for mobile platforms. Both Android and iOS has the concept of the default network to interact with the internet, usually prefer unmetered network (WIFI) over metered ones (cellular). I inlined some explanation here. I couldn't find an external link to explain this concept.

RyanTheOptimist

Just a few small comments.

(It might have been simpler to do a stand-alone PR which added the new writer and the ability to use it, but c'est la vie :>)

mobile/library/cc/engine_builder.h

RyanTheOptimist · 2025-11-26T18:34:36Z

mobile/library/common/system/default_system_helper_apple.cc

 }

+void DefaultSystemHelper::bindSocketToNetwork(Network::ConnectionSocket&, int64_t) {
+  PANIC("unreachable");


Sounds like this deserves a comment in the code. Since this class explicitly has "Apple"in the name, it is assumed to work on Apple platforms, I think. So if we think choice 1 is a valid option, then perhaps we should nuke the class? But option 2 seems plausible and worth a comment?

RyanTheOptimist · 2025-11-26T18:35:53Z

mobile/library/common/system/system_helper.h

  virtual std::vector<std::pair<int64_t, ConnectionType>> getAllConnectedNetworks() PURE;
+
+  /**
+   * Binds the given socket to the network interface associated with the handle.


Maybe a comment about what happens if the underlying socket doesn't support networking binding. (Should it be a no-op, or is it the callers responsibility to not call it in this case. If the latter, should we expose "supportsNetworkBinding()" or somesuch)

The caller (platform writer) will check against invalid network handle using that as a signal of whether the socket supports network binding or not. The network handle can be invalid even on platforms that support network binding, i.e. the default Android network monitor doesn't propagate the network handle to native code. So whether the network binding is meaningful or not doesn't purely depends on the SystemHelper interface.

RyanTheOptimist · 2025-11-26T18:36:51Z

mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java

+   * Set whether to use a platform specific APIs to create UDP socket and the associated QUIC packet
+   * writer. Note that `setUseV2NetworkMonitor()` also needs to be called to take effect. This is a
+   * temporary API which will be deprecated once the platform specific extension is verified to work
+   * and will be used as the default.


What's the relationship between this method and setUseV2NetworkMonitor? I would have thought that using this method would enable the new writer regardless of any migration / network monitoring things.

That's right, this API can be called regardless of whether the other migration API is called or not. But if the latter is called the engine builder will behave as if the former is also called.

RyanTheOptimist · 2025-11-26T18:37:51Z

mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java

+  }
+
+  /**
+   * Set whether to migrate idle connections to a different network upon network events.


The other similar methods use the phrase "QUIC connections". We should probably be consistent.

RyanTheOptimist · 2025-11-26T18:38:15Z

mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java

+   * Note that `setUseV2NetworkMonitor()` also needs to be called to take effect.
+   * If enabled, the engine will automatically be configured to use platform packet writer. *
+   */
+  public NativeCronvoyEngineBuilderImpl setEnableConnectionMigration(boolean enable) {


Since this and the following methods are QUIC-specific, should they have QUIC in the name?

Renamed them to mention Quic

Signed-off-by: Dan Zhang <[email protected]>

danzh2010 · 2025-11-27T03:40:49Z

/retest

Signed-off-by: Dan Zhang <[email protected]>

danzh2010 · 2025-11-27T04:36:23Z

/retest

Signed-off-by: Dan Zhang <[email protected]>

abeyad · 2025-12-01T15:20:42Z

/retest

abeyad · 2025-12-01T15:21:07Z

/wait

@danzh2010 is OOO

abeyad · 2025-12-01T16:44:05Z

Looks like the only remaining CI issue is test coverage:

✗ source/common/quic: 93.2% (threshold: 93.3%)

adisuissa

/lgtm api

danzh1989 added 8 commits November 14, 2025 19:51

client integration test work

9e08913

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

6a52ec1

Signed-off-by: Dan Zhang <[email protected]>

more test

d5c58f8

Signed-off-by: Dan Zhang <[email protected]>

core quic tests pass

203e2c3

Signed-off-by: Dan Zhang <[email protected]>

add cronvoy test

2645b6f

Signed-off-by: Dan Zhang <[email protected]>

proto format

0794e9f

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

9b4b3b4

Signed-off-by: Dan Zhang <[email protected]>

fix kotlin engine

2a0b356

Signed-off-by: Dan Zhang <[email protected]>

danzh2010 requested review from RyanTheOptimist and abeyad as code owners November 18, 2025 20:25

repokitteh-read-only bot added the api label Nov 18, 2025

repokitteh-read-only bot assigned mattklein123 Nov 18, 2025

danzh1989 added 2 commits November 19, 2025 05:22

fix test build

e93e7d0

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

5ef446c

Signed-off-by: Dan Zhang <[email protected]>

mattklein123 assigned RyanTheOptimist and unassigned mattklein123 Nov 20, 2025

RyanTheOptimist assigned abeyad and unassigned RyanTheOptimist Nov 20, 2025

fix build

79ac22b

Signed-off-by: Dan Zhang <[email protected]>

abeyad reviewed Nov 20, 2025

View reviewed changes

danzh1989 added 8 commits November 21, 2025 04:20

fix build

908dcf0

Signed-off-by: Dan Zhang <[email protected]>

fix build

3324553

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

31af080

Signed-off-by: Dan Zhang <[email protected]>

config api change

0f2da09

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

c5bb241

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

dd92f4a

Signed-off-by: Dan Zhang <[email protected]>

remove unused function

4a1a503

Signed-off-by: Dan Zhang <[email protected]>

coverage

aced2cb

Signed-off-by: Dan Zhang <[email protected]>

abeyad previously approved these changes Nov 26, 2025

View reviewed changes

repokitteh-read-only bot assigned botengyao Nov 26, 2025

abeyad unassigned botengyao Nov 26, 2025

repokitteh-read-only bot assigned adisuissa Nov 26, 2025

adisuissa reviewed Nov 26, 2025

View reviewed changes

RyanTheOptimist previously approved these changes Nov 26, 2025

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 26, 2025

danzh1989 added 2 commits November 26, 2025 20:48

Merge branch 'main' into writerfactoryextension

5a1c99c

Signed-off-by: Dan Zhang <[email protected]>

assuming java and kotlin code work

0eba0f0

Signed-off-by: Dan Zhang <[email protected]>

danzh2010 dismissed stale reviews from RyanTheOptimist and abeyad via 0eba0f0 November 26, 2025 21:06

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Nov 26, 2025

danzh1989 added 3 commits November 26, 2025 21:43

config api change

52d688d

Signed-off-by: Dan Zhang <[email protected]>

fix build

615d06d

Signed-off-by: Dan Zhang <[email protected]>

fix kt

ebee1ff

Signed-off-by: Dan Zhang <[email protected]>

fix test

67b72e0

Signed-off-by: Dan Zhang <[email protected]>

danzh1989 added 3 commits November 28, 2025 04:24

change output

e9c6b86

Signed-off-by: Dan Zhang <[email protected]>

fix java test

ee240a3

Signed-off-by: Dan Zhang <[email protected]>

Merge branch 'main' into writerfactoryextension

f161005

Signed-off-by: Dan Zhang <[email protected]>

repokitteh-read-only bot added the waiting label Dec 1, 2025

abeyad added the no stalebot Disables stalebot from closing an issue label Dec 1, 2025

adisuissa reviewed Dec 1, 2025

View reviewed changes

repokitteh-read-only bot removed the api label Dec 1, 2025

	// picked by the platform. A connection is this special state is only allowed to
	// picked by the platform. A connection in this special state is only allowed to

mobile: support QUIC connection migration #42104

Are you sure you want to change the base?

mobile: support QUIC connection migration #42104

Conversation

danzh2010 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Nov 18, 2025

Uh oh!

RyanTheOptimist commented Nov 20, 2025

Uh oh!

abeyad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abeyad left a comment

Choose a reason for hiding this comment

Uh oh!

abeyad commented Nov 26, 2025

Uh oh!

abeyad commented Nov 26, 2025

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danzh2010 commented Nov 18, 2025 •

edited

Loading