Skip to content

bpf: Support proxy upstream connections with original source address/port#8870

Merged
ianvernon merged 1 commit intomasterfrom
pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars
Aug 15, 2019
Merged

bpf: Support proxy upstream connections with original source address/port#8870
ianvernon merged 1 commit intomasterfrom
pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars

Conversation

@jrajahalme
Copy link
Copy Markdown
Member

@jrajahalme jrajahalme commented Aug 12, 2019 •
edited
Loading

Allow Envoy egress proxies to use the original source address and port in the upstream connections. This allows destination node to map the source IP to the policy ID. Egress Kafka is not supported yet, as Kafka only works for ingress at the moment. DNS proxy (always egress) support for original source address is left for a follow-up PR.

This change is Reviewable

@jrajahalme jrajahalme added the wip label Aug 12, 2019
@jrajahalme jrajahalme requested a review from a team August 12, 2019 08:18
@jrajahalme jrajahalme requested a review from a team as a code owner August 12, 2019 08:18
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch from 9122630 to fd1092e Compare August 12, 2019 08:24
@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 12, 2019

test-me-please

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch from fd1092e to d27771b Compare August 13, 2019 18:44
@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 13, 2019

test-me-please

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch from d27771b to 9843f41 Compare August 13, 2019 23:41
@jrajahalme jrajahalme requested a review from a team August 13, 2019 23:41
@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 13, 2019

test-me-please

joestringer
joestringer requested changes Aug 14, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

I took a quick look over the changes, and I'm a bit surprised by some of them, questions below.

Comment thread pkg/datapath/iptables/iptables.go Outdated
Copy link
Copy Markdown
Member

@joestringer joestringer Aug 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

(Note, not a question): These rules overlap with the same ones being added in #8864.

Copy link
Copy Markdown
Member Author

@jrajahalme jrajahalme Aug 14, 2019

Choose a reason for hiding this comment

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

Right, will rebase on top of #8864 when it merges.

Comment thread bpf/bpf_lxc.c Outdated
Copy link
Copy Markdown
Member

@joestringer joestringer Aug 14, 2019

Choose a reason for hiding this comment

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

The comment says "ingress proxy" but the code below is setting the mark for egress proxy too..?[0]

Copy link
Copy Markdown
Member Author

@jrajahalme jrajahalme Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

reply direction traffic from an LXC, if going to a local proxy, is for the ingress proxy. Original direction traffic from an LXC, if going to a local proxy, if for the egress proxy :-)

Comment thread bpf/bpf_lxc.c Outdated
Comment thread bpf/bpf_lxc.c Outdated
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch 2 times, most recently from 8dbb373 to c9fd511 Compare August 14, 2019 19:04
@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 14, 2019

test-me-please

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 14, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.05%) to 44.16% when pulling ba81061 on pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars into 3439d88 on master.

@joestringer
Copy link
Copy Markdown
Member

joestringer commented Aug 14, 2019 •
edited
Loading

Works like a charm in my cluster with egress + ingress HTTP policy.

houndci-bot
houndci-bot reviewed Aug 15, 2019
View reviewed changes
Comment thread pkg/fqdn/dnsproxy/proxy.go Outdated
Copy link
Copy Markdown

@houndci-bot houndci-bot Aug 15, 2019

Choose a reason for hiding this comment

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

var sourceIp should be sourceIP

@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 15, 2019

test-me-please

@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 15, 2019

Using the original source address for the dns proxy may be a stretch. Basic problem that two consecutive requests from the same source address:port will cause two client sockets to be opened, now with the same address:port, and the second one failed with bind: address already in use. I'm testing if SO_REUSEADDR makes any difference, but still miekg/dns expects the response to arrive on the same socket as the request was sent on, so even if two sockets could bind to the same address, it likely does not work.

A proper solution would use one socket for each source address:port, and then multiplex multiple queries on top of it.

@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 15, 2019

It may be better to split the DNS proxy changes to a separate follow-on PR, as it seems to be needing more work.

@jrajahalme
bpf: Support proxy using original source address and port.
ba81061 
Add a 'proxy_redirect' bit to the conntrack entry so that the reply
direction packets on proxy upstream connections using the original
source address and port in addition to the original destination
address and port can be redirected back to the local stack for local
delivery.

iptables rules are added to mark packets matching a transparent socket
as going to the host proxy.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch from cc5459c to ba81061 Compare August 15, 2019 18:41
@jrajahalme jrajahalme changed the title WIP: Support proxy upstream connections with original source address/port bpf: Support proxy upstream connections with original source address/port Aug 15, 2019
@jrajahalme jrajahalme added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. needs-backport/1.6 and removed wip labels Aug 15, 2019
@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 15, 2019

Split out the DNS proxy changes to a follow-on PR so that the egress Envoy L7 support can be shipped.

@jrajahalme
Copy link
Copy Markdown
Member Author

jrajahalme commented Aug 15, 2019

test-me-please

@jrajahalme jrajahalme mentioned this pull request Aug 15, 2019
Merged
@ianvernon ianvernon merged commit 830adba into master Aug 15, 2019
@ianvernon ianvernon deleted the pr/jrajahalme/envoy-avoid-bpf-warnings-on-sidecars branch August 15, 2019 22:58
@joestringer joestringer mentioned this pull request Aug 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joestringer joestringer joestringer approved these changes

+1 more reviewer

@houndci-bot houndci-bot houndci-bot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jrajahalme @coveralls @joestringer @houndci-bot @ianvernon