quiche: add EnvoyQuicPacketWriter with sendmsg to set source address #7416

danzh2010 · 2019-06-27T19:05:10Z

EnvoyQuicPacketWriter calls UdpListener::send() to write packet.
Modify IoSocketHandleImpl::sendmsg() to take source ip address and used it in ancillary data.

Risk Level: low, udp listener is not in use.
Testing: Added unit tests for EnvoyQuicPacketWriter and integration test for UdpListener to send self address.
Part of #2557

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2019-07-09T14:49:06Z

/retest

repokitteh-read-only · 2019-07-09T14:49:11Z

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7416 (comment) was created by @danzh2010.

see: more, trace.

mattklein123

LGTM, thanks.

mattklein123 · 2019-07-09T15:51:03Z

test/common/network/udp_listener_impl_test.cc

-  Buffer::InstancePtr result_buffer(new Buffer::OwnedImpl());
-  const uint64_t bytes_to_read = 11;
+  const uint64_t bytes_to_read = payload.length();
+  // Make receive buffer 1 byte larger for trailling '\0'.


typo "trailling" (feel free to fix in your next PR)

mattklein123 · 2019-07-09T17:35:03Z

@danzh1989 I suspect the coverage error is a legit timing error. It looks like something might be hanging on one of the UDP tests.

/wait

Signed-off-by: Dan Zhang <danzh@google.com>

mattklein123 · 2019-07-10T15:38:25Z

@danzh2010 sorry this is going to need a master merge due to #7457

/wait

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2019-07-10T22:17:50Z

/retest

repokitteh-read-only · 2019-07-10T22:17:52Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #7416 (comment) was created by @danzh2010.

see: more, trace.

danzh2010 · 2019-07-10T22:43:10Z

/retest

repokitteh-read-only · 2019-07-10T22:43:12Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #7416 (comment) was created by @danzh2010.

see: more, trace.

danzh2010 · 2019-07-11T14:42:13Z

/retest

repokitteh-read-only · 2019-07-11T14:42:17Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #7416 (comment) was created by @danzh2010.

see: more, trace.

htuch · 2019-07-11T14:44:41Z

/acp rerun

htuch · 2019-07-11T14:44:47Z

/azp rerun

azure-pipelines · 2019-07-11T14:44:52Z

Command 'rerun' is not supported by Azure Pipelines. Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or a specific pipeline for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify a pipeline to run. Example: "run" or "run pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

htuch · 2019-07-11T14:45:08Z

/azp run envoy-macos

azure-pipelines · 2019-07-11T14:45:17Z

Azure Pipelines successfully started running 1 pipeline(s).

mattklein123

Thanks, will merge once 1.11 is cut.

htuch · 2019-07-18T19:20:22Z

test/common/network/udp_listener_impl_test.cc

+    // if it's not specified in cmsghdr.
+    send_from_addr.reset(new Address::Ipv6Instance(
+#ifdef IP_FREEBIND
+        "::9",


@danzh2010 this one cause problems during Google Envoy import. It seems we don't support the ability to bind to ::9 like this on our internal test farm. One of the issues that seems to exist here is that we assume that a compile time support for IP_FREEBIND translates to runtime ability. Do you think you could express this differently? CC @dnoe @yanavlasov

@danzh2010 this one cause problems during Google Envoy import. It seems we don't support the ability > to bind to ::9 like this on our internal test farm. One of the issues that seems to exist here is that we
assume that a compile time support for IP_FREEBIND translates to runtime ability.

This might has walk-around, I'll check our test infrastructure about it.

Do you think you could express this differently? CC @dnoe @yanavlasov

It's hard to test it in a different way as we need an v6 address that we can use to send which is different from what kernel would have picked for us. An compromise is we always use ::1 for v6, but this will leave v6 behavior untested.

danzh1989 added 25 commits June 6, 2019 16:16

add quic_core_crypto_crypto_handshake_lib

c0e6ddc

Signed-off-by: Dan Zhang <danzh@google.com>

update quic_ip_address

3290e50

Signed-off-by: Dan Zhang <danzh@google.com>

make crypto_handshake_lib compile

b6fa1d1

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'master' into cryptoconfig

1ca0d1d

Signed-off-by: Dan Zhang <danzh@google.com>

compile quic_session_lib

9dcd6b8

Signed-off-by: Dan Zhang <danzh@google.com>

rename alarm interface

b4c5c56

Signed-off-by: Dan Zhang <danzh@google.com>

add bbr2 platform impl

0800797

Signed-off-by: Dan Zhang <danzh@google.com>

build dispatcher

70f1206

Signed-off-by: Dan Zhang <danzh@google.com>

remove unnecessary sed command

427f498

Signed-off-by: Dan Zhang <danzh@google.com>

wait for removal of base/logging.h

f5a3684

Signed-off-by: Dan Zhang <danzh@google.com>

roll up tar ball

4357d0a

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'master' into buildquche

d6f6cee

Signed-off-by: Dan Zhang <danzh@google.com>

add operator<< for vector

d69b2e6

Signed-off-by: Dan Zhang <danzh@google.com>

deprecate quic_socket_address_impl.h

8295ec9

Signed-off-by: Dan Zhang <danzh@google.com>

sendmsg include self ip

df8c151

Signed-off-by: Dan Zhang <danzh@google.com>

add envoy_quic_writer

97c21ed

Signed-off-by: Dan Zhang <danzh@google.com>

add writer test

75e836f

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'master' into writer

1cd6f9b

Signed-off-by: Dan Zhang <danzh@google.com>

Remove onDestroy mock

3a24e6f

Signed-off-by: Dan Zhang <danzh@google.com>

Fix compile error

54a5db7

Signed-off-by: Dan Zhang <danzh@google.com>

Add more error code

b1363dd

Signed-off-by: Dan Zhang <danzh@google.com>

Test send with address

8de7470

Signed-off-by: Dan Zhang <danzh@google.com>

add integration test

07a692a

Signed-off-by: Dan Zhang <danzh@google.com>

IP_FREEBIND for v6

a418940

Signed-off-by: Dan Zhang <danzh@google.com>

refine socket option macros

f663e0d

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 requested review from alyssawilk and mattklein123 as code owners June 27, 2019 19:05

danzh1989 added 3 commits June 27, 2019 15:12

fix spell

40a6e27

Signed-off-by: Dan Zhang <danzh@google.com>

fix spell again

96770be

Signed-off-by: Dan Zhang <danzh@google.com>

fix IP_SENDSRCADDR

4b458bb

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot removed the waiting label Jul 8, 2019

change to initialize list

a34c354

Signed-off-by: Dan Zhang <danzh@google.com>

mattklein123 previously approved these changes Jul 9, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Jul 9, 2019

use log delegate

6df8797

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 dismissed mattklein123’s stale review via 6df8797 July 10, 2019 15:13

repokitteh-read-only bot removed the waiting label Jul 10, 2019

repokitteh-read-only bot added the waiting label Jul 10, 2019

danzh1989 added 3 commits July 10, 2019 14:55

fix typo

d529ad1

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'master' into writer

97cc55c

Signed-off-by: Dan Zhang <danzh@google.com>

format

3c55b50

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot removed the waiting label Jul 10, 2019

mattklein123 approved these changes Jul 11, 2019

View reviewed changes

mattklein123 merged commit 37c5ce2 into envoyproxy:master Jul 11, 2019

htuch reviewed Jul 18, 2019

View reviewed changes

quiche: add EnvoyQuicPacketWriter with sendmsg to set source address #7416

quiche: add EnvoyQuicPacketWriter with sendmsg to set source address #7416

Uh oh!

Conversation

danzh2010 commented Jun 27, 2019

Uh oh!

danzh2010 commented Jul 9, 2019

Uh oh!

repokitteh-read-only bot commented Jul 9, 2019

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 Jul 9, 2019

Choose a reason for hiding this comment

Uh oh!

danzh2010 Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Jul 9, 2019

Uh oh!

mattklein123 commented Jul 10, 2019

Uh oh!

danzh2010 commented Jul 10, 2019

Uh oh!

repokitteh-read-only bot commented Jul 10, 2019

Uh oh!

danzh2010 commented Jul 10, 2019

Uh oh!

repokitteh-read-only bot commented Jul 10, 2019

Uh oh!

danzh2010 commented Jul 11, 2019

Uh oh!

repokitteh-read-only bot commented Jul 11, 2019

Uh oh!

htuch commented Jul 11, 2019

Uh oh!

htuch commented Jul 11, 2019

Uh oh!

azure-pipelines bot commented Jul 11, 2019

Uh oh!

htuch commented Jul 11, 2019

Uh oh!

azure-pipelines bot commented Jul 11, 2019

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

htuch Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

danzh2010 Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!