Skip to content

docs: Extend BPF-based masquerading section#12145

Merged
borkmann merged 6 commits intomasterfrom
pr/brb/docs-bpf-masq
Jun 19, 2020
Merged

docs: Extend BPF-based masquerading section#12145
borkmann merged 6 commits intomasterfrom
pr/brb/docs-bpf-masq

Conversation

@brb
Copy link
Copy Markdown
Member

@brb brb commented Jun 17, 2020

This PR:

  • Extend the section.
  • Extends cilium status to show masq devices.
  • Improve usage of global.devices in the kubeproxy-free gsg.

Reviewable per commit.

@brb brb added pending-review area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/daemon Impacts operation of the Cilium daemon. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Jun 17, 2020
@brb brb requested review from a team as code owners June 17, 2020 16:10
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 17, 2020

test-me-please

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 17, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.02%) to 37.088% when pulling 70ed8e6 on pr/brb/docs-bpf-masq into 9d78e1a on master.

joestringer
joestringer requested changes Jun 17, 2020
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.

Awesome thanks! A few minor nits below.

Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/spelling_wordlist.txt Outdated
@brb brb force-pushed the pr/brb/docs-bpf-masq branch 2 times, most recently from 7b60a4b to a377c8a Compare June 18, 2020 08:44
@brb brb requested review from a team June 18, 2020 09:43
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 18, 2020

@joestringer Thanks for the review. I've addressed your comments + extended the cilium status to include the SNAT exclusion CIDR (three last commits). PTAL.

@brb brb requested a review from joestringer June 18, 2020 09:44
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 18, 2020

test-me-please

aanm
aanm approved these changes Jun 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

I didn't read the docs only code.

@brb brb force-pushed the pr/brb/docs-bpf-masq branch from 9d67d7c to 77a988d Compare June 18, 2020 13:26
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 18, 2020

test-me-please

brb added 2 commits June 18, 2020 15:26
@brb
cli: Report BPF masquerading devices
41d6e04 
This commit extends "cilium status" to show which devices can run
the BPF masquerading program. E.g.:

    $ cilium status | grep Masquerading
    Masquerading:           BPF   [eth0, eth1]

Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
docs: Improve usage of global.devices in kubeproxy-free gsg
1c18ea6 
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 18, 2020

CI net-next provisioning failed:

[2020-06-18T15:17:43.550Z]     k8s3-1.11: Created symlink /etc/systemd/system/multi-user.target.wants/kubelet.service -> /lib/systemd/system/kubelet.service.
[2020-06-18T15:17:43.869Z]     k8s3-1.11: Setting up kubectl (1.11.10-00) ...
[2020-06-18T15:17:43.869Z]     k8s3-1.11: Setting up kubeadm (1.11.10-00) ...
[2020-06-18T15:23:58.204Z] Cannot contact jenkins-fixed-27: java.lang.InterruptedException

@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 18, 2020

run-net-next

joestringer
joestringer requested changes Jun 19, 2020
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.

Minor nits, otherwise LGTM.

Comment thread Documentation/concepts/networking/masquerading.rst Outdated
Comment thread Documentation/concepts/networking/masquerading.rst
I think it's important for users to understand how we sync a config. The example below
shows how to configure the agent via `ConfigMap` and to verify it:

::
Copy link
Copy Markdown
Member

@joestringer joestringer Jun 19, 2020

Choose a reason for hiding this comment

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

Any reason to avoid using .. code-block:: shell-session and obvious shell bits like starting lines you execute with $? Seems a bit weird to not differentiate the commands and the output here.

Copy link
Copy Markdown
Member Author

@brb brb Jun 19, 2020

Choose a reason for hiding this comment

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

Agree, but I wanted to make it consistent with other guides. IIRC we had a discussion some time ago whether we should keep $'s in our guides. The consensus was to get rid of them.

Comment thread pkg/datapath/config.go Outdated
Comment thread pkg/datapath/config.go
Comment thread pkg/datapath/linux/config/config.go Outdated
@joestringer
Copy link
Copy Markdown
Member

joestringer commented Jun 19, 2020

Previous test lost connectivity to the VM:

08:17:43      k8s3-1.11: Setting up kubeadm (1.11.10-00) ...
08:23:58  Cannot contact jenkins-fixed-27: java.lang.InterruptedException

https://jenkins.cilium.io/job/Cilium-PR-K8s-oldest-net-next/883/

@joestringer
Copy link
Copy Markdown
Member

joestringer commented Jun 19, 2020

retest-net-next

@brb brb force-pushed the pr/brb/docs-bpf-masq branch from 77a988d to 70ed8e6 Compare June 19, 2020 06:48
@brb
Copy link
Copy Markdown
Member Author

brb commented Jun 19, 2020

test-me-please

@brb brb requested a review from joestringer June 19, 2020 06:49
brb added 4 commits June 19, 2020 08:49
@brb
docs: Extend BPF masquerading section
a3baa2d 
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
datapath: Add GetRemoteSNATDstAddrExclusionCIDR helper
9745550 
The helper is used to determine the dst CIDR for SNAT exclusion.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
api,daemon,cli: Add SNAT exclusion dst to cilium status
147185d 
This commits extends "cilium status" to show dst cidr of SNAT exclusion.
E.g.:

    $ cilium status | grep Masquerading
    Masquerading:           BPF   [eth0, eth1]  10.0.0.0/16

Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
docs: Add exclusion CIDR output to BPF masq section
70ed8e6 
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@borkmann
Copy link
Copy Markdown
Member

borkmann commented Jun 19, 2020

test-me-please

@borkmann borkmann merged commit 2f04afd into master Jun 19, 2020
@borkmann borkmann deleted the pr/brb/docs-bpf-masq branch June 19, 2020 09:46
@borkmann borkmann mentioned this pull request Jun 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aanm aanm aanm approved these changes

@joestringer joestringer Awaiting requested review from joestringer

Assignees

No one assigned

Labels

area/daemon Impacts operation of the Cilium daemon. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@brb @coveralls @joestringer @borkmann @aanm