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

v1.14 Backports 2023-12-20 #29996

Merged
merged 9 commits into from
Jan 12, 2024
Merged

Conversation

YutaroHayakawa
Copy link
Member

@YutaroHayakawa YutaroHayakawa commented Dec 20, 2023

@YutaroHayakawa YutaroHayakawa added kind/backports This PR provides functionality previously merged into master. backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. labels Dec 20, 2023
@YutaroHayakawa
Copy link
Member Author

/test-backport-1.14

@YutaroHayakawa
Copy link
Member Author

/test-backport-1.14

@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review December 20, 2023 09:30
@YutaroHayakawa YutaroHayakawa requested a review from a team as a code owner December 20, 2023 09:30
@YutaroHayakawa
Copy link
Member Author

Conformance AKS: #30002

@tklauser tklauser removed their request for review December 20, 2023 13:40
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

Changes LGTM, but I'm not sure I understand the backporter note that you left. It says:

[ backporter's note: backporter's note: Conflict due to the
  do_netdev_encrypt_encap doesn't exist on v1.12 branch. Removed it. ]

But the change on line 980 in bpf/bpf_host.c is inside the function do_netdev_encrypt_encap. Maybe I'm misunderstanding something?

joamaki and others added 9 commits January 10, 2024 19:10
[ upstream commit 50e9666 ]

[ backporter's note: Needed to import cache package to fix compilation
  error. ]

The fake K8s client checks if the events channel becomes full and panics
if it does. Fix this by using a hand-rolled ListerWatcher to have full
control over the events channel.

Fixes: #28575
Fixes: a7f1eef ("resource: Fix double upserts on subscribe and retrying of delete events")
Signed-off-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit 23ef3c0 ]

Cilium-agent is overriding K8S_NODE_NAME env variable based on node
name. Cilium-preflight was not overriding it so in case that hostname
did not match node name, preflight was stuck waiting for node
information.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit 8c2dfda ]

This commit changes the log fields for the "stale identity observed"
message to include the full context (i.e. trace observation point,
source and destination addresses etc) for identifying which flow caused
the message to be emitted. In addition, the "old" and "new" identity
field names are replaced with the better fitting "userspace" and
"datapath" terminology.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit caef2d3 ]

This was reported by Paul in [1]. After investigating the sysdump, it is
clear that Hubble should not complain about such packets.

[1] #15283 (comment)

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit 4196935 ]

[ backporter's note: time package doesn't exist on v1.14 branch. Use
  standard time package instead. ]

This limits the amount of "stale identities observed" messages to one
every 30 seconds. This is particularly important if monitor aggregation
is disabled, as otherwise any unknown discrepancy fills the log buffers
and thus makes it hard to e.g. debug CI flakes.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit 0d35af0 ]

To make it easy for callers, ipv4_handle_fragmentation() should always
return a DROP_* reason on error. But for errors from l4_load_ports() we're
currently just propagating those raw errors back.

Return a drop reason instead. This also makes us consistent with the
non-fragment path in ipv4_load_l4_ports().

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit a495abd ]

[ backporter's note: backporter's note: Upstream calls
  do_netdev_encrypt_encap instead of wrapping it with do_netdev_encrypt.
  Use do_netdev_encrypt. ]

This commit modifies calls to `send_trace_notify` in the datapath to
drop trace events related to encrypted packets when monitor aggregation
is enabled. More specifically, this commit ensures that whenever
`send_trace_notify` is called with a `trace_reason` of `TRACE_REASON_ENCRYPTED`,
the `monitor` argument is set to zero. A Coccinelle script is provided
in this commit to add a build-time check for this requirement moving forward.

This change helps to reduce the overall CPU usage of Cilium Agents when IPSec
encryption is enabled, by reducing the number of trace events emitted by
the datapath. Normally monitor aggregation can be used in order to
reduce the number of trace events, however IPSec-related trace events
are not able to be aggregated since they lack the necessary connection
tracking information. See the function `emit_trace_notify` in `bpf/lib/trace.h`
for more information.

This same change was applied in `bpf/bpf_lxc.c` in commit 3e52822.

Thank you to Lorenz who added a workaround for passing the verifier with
Clang 10.

See also: #27168

Co-authored-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit e95f2e0 ]

[ backporter's note:
  - Makefile target structure is different between v1.14 and main. Took
    the v1.14's structure.
  - TRACE_NOTIFY is already defined in the bpf_align_checker.c. Avoid
    redefining it.
]

Add an initial bpf_network configuration. The defines are taken from a
sysdump for a failing CI run that seems to exhibit verifier problems.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
[ upstream commit 3d3f69c ]

[ backporter's note: Fix a minor conflict comes from the variable name
  change (key -> ms).]

Datapath equalness of the old and new entry must be evaluated before
entries are merged, afterwards the entries will be the same and the key
is not added in ChangeState.Adds, which may lead to a policy map entry
not being updated during incremental updates.

Fixes: #26331

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa
Copy link
Member Author

@learnitall Same for #30004 (comment). Sorry for the confusion. Fixed the message in the latest push.

@YutaroHayakawa
Copy link
Member Author

/test-backport-1.14

Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

Thank you!

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 11, 2024
@julianwiedmann julianwiedmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 11, 2024
@julianwiedmann julianwiedmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 11, 2024
@dylandreimerink dylandreimerink merged commit 547ed3b into v1.14 Jan 12, 2024
217 checks passed
@dylandreimerink dylandreimerink deleted the pr/v1.14-backport-2023-12-20 branch January 12, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

10 participants