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

ctmap: right-shift kernel jiffies by BPF_MONO_SCALER #26197

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Jun 13, 2023

Commit 8531c5a ("bpf,datapath: read jiffies from /proc/schedstat") switched package ctmap over to using /proc/schedstat for getting kernel jiffies.

During the conversion, a shift by a magic 'scaler' value was missed, leading to strange output in cilium bpf ct list global -d (.. (remaining: -4391264792 sec(s)) ..) and the CT GC never collecting any flows, since the current jiffies were always perceived to be orders of magnitude larger than the flow expiry time.

This patch wraps probes.Jiffies() in a documented scaledJiffies() helper and declares the BPF_MONO_SCALER macro as a Go constant.

Fixes: #26182
Follow-up to: #25795

Commit 8531c5a ("bpf,datapath: read jiffies from /proc/schedstat")
switched package ctmap over to using /proc/schedstat for getting
kernel jiffies.

During the conversion, a shift by a magic 'scaler' value was missed,
leading to strange output in `cilium bpf ct list global -d`
(.. (remaining: -4391264792 sec(s)) ..) and the CT GC never collecting
any flows, since the current jiffies were always perceived to be orders
of magnitude larger than the flow expiry time.

This patch wraps probes.Jiffies() in a documented scaledJiffies() helper
and declares the BPF_MONO_SCALER macro as a Go constant.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. release-blocker/1.13 This issue will prevent the release of the next version of Cilium. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. labels Jun 13, 2023
@ti-mo ti-mo requested a review from a team as a code owner June 13, 2023 19:49
@ti-mo ti-mo requested a review from gentoo-root June 13, 2023 19:49
@ti-mo
Copy link
Contributor Author

ti-mo commented Jun 13, 2023

/test

@michi-covalent
Copy link
Contributor

ci-external-workloads: looks like a transient infra issue. restarting.

@michi-covalent
Copy link
Contributor

ci-e2e: lots of one.one.one.one failures. not sure if re-trying would help, but let's see.

@borkmann
Copy link
Member

borkmann commented Jun 13, 2023

ci-e2e: lots of one.one.one.one failures. not sure if re-trying would help, but let's see.

Cc @qmonnet we've also seen this in various other PRs.

(#25972)

@michi-covalent
Copy link
Contributor

yeah let's ignore the one.one.one.one failures. it's not related to this pull request, and as you said it's happening in other pull requests too (#26160 (comment) is one example). i'll wait for test-1.26-net-next to pass before merging ✅

@michi-covalent michi-covalent merged commit 3eaca5d into cilium:main Jun 13, 2023
65 of 66 checks passed
@michi-covalent michi-covalent added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Jun 13, 2023
This was referenced Jun 13, 2023
@michi-covalent michi-covalent added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jun 13, 2023
@qmonnet qmonnet added this to Backport pending to v1.13 in 1.13.4 Jun 14, 2023
@michi-covalent michi-covalent added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jun 14, 2023
@ti-mo ti-mo deleted the tb/jiffies-scaler branch June 14, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. kind/bug This is a bug in the Cilium logic. release-blocker/1.13 This issue will prevent the release of the next version of Cilium. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.13.4
Backport pending to v1.13
Development

Successfully merging this pull request may close these issues.

Incorrect remaining time for conntrack entry
3 participants