Skip to content

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

Merged
mattklein123 merged 54 commits intoenvoyproxy:masterfrom
danzh2010:writer
Jul 11, 2019
Merged

quiche: add EnvoyQuicPacketWriter with sendmsg to set source address#7416
mattklein123 merged 54 commits intoenvoyproxy:masterfrom
danzh2010:writer

Conversation

@danzh2010
Copy link
Copy Markdown
Contributor

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

danzh1989 added 25 commits June 6, 2019 16:16
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
@danzh2010
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

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

see: more, trace.

mattklein123
mattklein123 previously approved these changes Jul 9, 2019
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 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.

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'.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@mattklein123
Copy link
Copy Markdown
Member

@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
Copy link
Copy Markdown
Member

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

/wait

Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
@danzh2010
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🤷‍♀️ nothing to rebuild.

🐱

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

see: more, trace.

@danzh2010
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🤷‍♀️ nothing to rebuild.

🐱

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

see: more, trace.

@danzh2010
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🤷‍♀️ nothing to rebuild.

🐱

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

see: more, trace.

@htuch
Copy link
Copy Markdown
Member

htuch commented Jul 11, 2019

/acp rerun

@htuch
Copy link
Copy Markdown
Member

htuch commented Jul 11, 2019

/azp rerun

@azure-pipelines
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Member

htuch commented Jul 11, 2019

/azp run envoy-macos

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks, will merge once 1.11 is cut.

@mattklein123 mattklein123 merged commit 37c5ce2 into envoyproxy:master Jul 11, 2019
// if it's not specified in cmsghdr.
send_from_addr.reset(new Address::Ipv6Instance(
#ifdef IP_FREEBIND
"::9",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@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.

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.

5 participants