Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipmasq: Add support for ip-masq-agent with IPv6 #23219

Merged
merged 7 commits into from Jun 15, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jan 20, 2023

Follow-up to #23165

  • bpf: nat: Rename ENABLE_IP_MASQ_AGENT to [...]_IPV4
  • ipmasq: Rename ipmasq-related variables to [...]IPv4
  • bpf: nat: Plumb IPv6 ip-masq-agent in datapath
  • ipmasq: Implement ip-masq-agent support for IPv6
  • ipmasq: Enable IPv6 support for ip-masq-agent
  • ipmasq: Add integration tests for IPv6 with ip-masq-agent
  • test: Add datapath configuration tests for the ip-masq-agent with IPv6

@qmonnet qmonnet added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. dont-merge/blocked Another PR must be merged before this one. feature/ipv6 Relates to IPv6 protocol support labels Jan 20, 2023
@qmonnet
Copy link
Member Author

qmonnet commented Jan 20, 2023

/test

@qmonnet qmonnet force-pushed the pr/ipv6-masq-agent branch 2 times, most recently from a7faea8 to 6c0b6b5 Compare January 24, 2023 17:08
@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 1, 2023
@qmonnet qmonnet removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 1, 2023
@qmonnet qmonnet force-pushed the pr/ipv6-masq-agent branch 2 times, most recently from 5bc0285 to dc50547 Compare June 7, 2023 12:59
@qmonnet
Copy link
Member Author

qmonnet commented Jun 7, 2023

Marking as a release blocker so we can discuss it during today's meeting, as it would be great to have it in 1.14 alongside #23165.

Current status: blocked on #23165. Has not been into review yet. This PR currently contains the commits from #23165; only the last 7 commits are new here (most of which are small changes).

@qmonnet qmonnet added the release-blocker/1.14 This issue will prevent the release of the next version of Cilium. label Jun 7, 2023
@qmonnet qmonnet force-pushed the pr/ipv6-masq-agent branch 3 times, most recently from c6ee164 to ab5ec36 Compare June 14, 2023 02:02
@qmonnet
Copy link
Member Author

qmonnet commented Jun 14, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathConfig Check BPF masquerading with ip-masq-agent VXLAN

Failure Output

FAIL: Kubernetes DNS did not become ready in time

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/776/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

@qmonnet qmonnet removed the dont-merge/blocked Another PR must be merged before this one. label Jun 14, 2023
@qmonnet qmonnet marked this pull request as ready for review June 14, 2023 02:03
@qmonnet qmonnet requested review from a team as code owners June 14, 2023 02:03
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

Some nits, otherwise LGTM. Approving to unblock with the understanding that changes are required prior to merge.

@@ -65,7 +65,7 @@ to determine which devices the program is running on:
From the output above, the program is running on the ``eth0`` and ``eth1`` devices.


The eBPF-based masquerading can masquerade packets of the following IPv4 L4 protocols:
The eBPF-based masquerading can masquerade packets of the following L4 protocols:
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope for this PR, but someday I'd love to see a clearer definition of masquerading in the page intro. We just sort of roll right into using it to mean a particular behavior without directly defining what it means when we use it. The current state isn't bad; I only think it could be better.

Documentation/network/concepts/masquerading.rst Outdated Show resolved Hide resolved
Documentation/network/concepts/masquerading.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@gentoo-root gentoo-root left a comment

Choose a reason for hiding this comment

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

Looks good to me, one question regarding a part that I don't fully understand.

bpf/lib/nat.h Outdated Show resolved Hide resolved
@qmonnet
Copy link
Member Author

qmonnet commented Jun 14, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathConfig Check BPF masquerading with ip-masq-agent VXLAN

Failure Output

FAIL: Timed out after 240.001s.

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/811/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Loader related changes lgtm.

@joestringer joestringer added the kind/feature This introduces new functionality. label Jun 14, 2023
@qmonnet
Copy link
Member Author

qmonnet commented Jun 14, 2023

/test

@qmonnet
Copy link
Member Author

qmonnet commented Jun 14, 2023

/test

The macro ENABLE_IP_MASQ_AGENT is used to guard the ip-masq-agent code
for IPv4 only, rename it accordingly.

This is in prevision for IPv6 support for the ip-masq-agent.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Currently, the ip-masq-agent only works with IPv4. In preparation for
IPv6 support, rename some internal variables and functions by adding an
"IPv4" suffix.

The following are affected:

- ipmasq.IPMasqAgent.masqLinkLocal
- ipmasq.MapName
- ipmasq.MaxEntries
- ipmasq.config.MasqLinkLocal
- ipmasq.ipMasqMapMock.cidrs
- ipmasq.key
- ipmasq.keyToIPNet
- ipmasq.linkLocalCIDR
- ipmasq.linkLocalCIDRStr
- ipmasq.once

No functional change.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Add support for ip-masq-agent with IPv6 to the BPF datapath. The
required edits are small, and replicate what the datapath does for IPv4.
This code, guarded by ENABLE_IP_MASQ_AGENT_IPV6, is not enabled by the
Agent yet.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Add the relevant map definition and the corresponding handling in
package ipmasq for supporting IPv6 with the ip-masq-agent.

We get a new BPF map for storing entries that we do not want to
masquerade. The maximum number of entries is the same as for the
IPv4-related map, and will be made customisable in a future commit.
Handling of the map is nearly identical to the one for IPv4.

From the outside of the ipmasq package, the IPMasqBPFMap abstraction
hides the fact that the maps are distinct, its methods take care of
identifying the IP version in use for the CIDRs passed in arguments and
to update the relevant map.

Option "nonMasqueradeCIDRs" is supported transparently as well. Support
for option "masqLinkLocalIPv6" is added. By default, the ip-masq-agent
prevents masquerading to CIDR fe80::/10, as described in [0], based on
RFC 4291.

However, at this point, the Agent still refuses to accept configurations
using IPv6 and ip-masq-agent, so this code is not used just yet.

[0] https://github.com/kubernetes-sigs/ip-masq-agent#configuring-the-agent

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Enable ip-masq-agent support for IPv6 in the Cilium Agent:

- Make the Agent accept IPv6 _and_ ip-masq-agent in its configuration.
- Similarly, make the daemon accept IPv6 with the ip-masq-agent.
- Create the IPv6-related BPF map on Agent startup.
- When IPv6 and the ip-masq-agent are selected, pass the relevant macro
  to the datapath to process packets accordingly.

Related updates:

- Display the maximum number of entries for the IPv6-related map in the
  status command.
- Update the documentation.
- Add the (commented) Helm value.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Extend the existing tests for ip-masq-agent to test the IPv6
infrastructure. In addition of update and restore operations for IPv4,
we add the equivalent for IPv6-only, and for when both IPv4 and IPv6 are
enabled.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
The ip-masq-agent for BPF masquerasing is now supported with IPv6.
Extend the tests to make sure that it behaves as expected. We simply
reuse the setup for the IPv4 test, and attempt to connect to the echo
server using IPv6.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member Author

qmonnet commented Jun 14, 2023

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 14, 2023
@tklauser tklauser merged commit 06fb6b4 into cilium:main Jun 15, 2023
61 checks passed
@qmonnet qmonnet deleted the pr/ipv6-masq-agent branch June 15, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ipv6 Relates to IPv6 protocol support kind/feature This introduces new functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants