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

#17168 and #17230 #17273

Closed
wants to merge 5 commits into from
Closed

#17168 and #17230 #17273

wants to merge 5 commits into from

Conversation

bmcustodio
Copy link
Contributor

@bmcustodio bmcustodio commented Aug 31, 2021

This PR is based off of v1.10 and contains #17230, #17168 and #17418 on top, for testing purposes.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 31, 2021
@bmcustodio bmcustodio changed the base branch from master to v1.10 August 31, 2021 13:52
brb and others added 5 commits August 31, 2021 14:54
The new option is used to specify a device which globally scoped IP addr
should be used for BPF-based masquerading.

This is a workaround for an environment which uses ECMP for outgoing
traffic and it has a dedicated device which IP addr should be used for
the masquerading. The workaround is relevant until
#17158 has been resolved (thus,
we hide the flag).

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Previously, the BPF-based masquerading (--enable-bpf-masquerade=true)
was wrongly masquerading replies from a pod to an outside when the
outside had initiated a connection. This was possible when e.g., the
outside had a route to the pod cidr.

To fix this, we introduce a lightweight CT lookup function
ct_is_reply4() which checks whether a given flow is a reply. The lookup
function is called in snat_v4_needed().

As a side note, I've tried to move the port extraction to a separate
function, but unfortunately it hits complexity issues on the 4.19
kernel in the "K8sDatapathConfig AutoDirectNodeRoutes Check direct
connectivity with per endpoint routes" suite:

    BPF program is too large. Processed 131073 insn
    libbpf: failed to load program 'handle_to_container'
    libbpf: failed to load object '624_next/bpf_lxc.o'

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
Previously, they were failing due to our datapath masquerading replies
from pod to outside. As it got fixed in the previous commit, we can
enable BPF-based masquerading.

This will also gives us some coverage for the fix.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
In initExcludedIPs() we build a list of IPs that Cilium needs to exclude
to operate. One check to determine if an IP should be excluded is based
on the state of the net device: if the device is not up, then its IPs
are excluded.

Unfortunately, this check is not enough, as it's possible to have a
device reporting an unknown state (because its driver is missing the
operstate handling, e.g. a dummy device) while still being operational.

This commit changes the logic in initExcludedIPs() to not exclude IPs of
devices reporting an unknown state.

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge/needs-release-note-label The author needs to describe the release impact of these changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants