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

Add preliminary support for a Linux 6.1 guest kernel, plus kvm_ptp #4188

Merged
merged 11 commits into from
Oct 23, 2023

Conversation

pb8o
Copy link
Contributor

@pb8o pb8o commented Oct 19, 2023

Changes

  • Add preliminary support for Linux 6.1 guest kernel. Only for c7g and Linux 6.1 host
  • Add kvm_ptp support in aarch64 and Linux 6.1

Reason

...

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet contribution quality standards.

  • This functionality cannot be added in rust-vmm.

@pb8o pb8o self-assigned this Oct 19, 2023
@pb8o pb8o added Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Awaiting author Indicates that an issue or pull request requires author action labels Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b0e03a) 82.94% compared to head (94a84e8) 82.94%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4188   +/-   ##
=======================================
  Coverage   82.94%   82.94%           
=======================================
  Files         221      221           
  Lines       28419    28419           
=======================================
  Hits        23572    23572           
  Misses       4847     4847           
Flag Coverage Δ
4.14-c7g.metal 78.46% <ø> (ø)
4.14-m5d.metal 80.26% <ø> (-0.02%) ⬇️
4.14-m6a.metal 79.40% <ø> (ø)
4.14-m6g.metal 78.46% <ø> (ø)
4.14-m6i.metal 80.25% <ø> (ø)
5.10-c7g.metal 81.39% <ø> (ø)
5.10-m5d.metal 82.96% <ø> (ø)
5.10-m6a.metal 82.19% <ø> (ø)
5.10-m6g.metal 81.39% <ø> (ø)
5.10-m6i.metal 82.95% <ø> (ø)
6.1-c7g.metal 81.39% <ø> (ø)
6.1-m5d.metal 82.95% <ø> (-0.02%) ⬇️
6.1-m6a.metal 82.19% <ø> (ø)
6.1-m6g.metal 81.39% <ø> (ø)
6.1-m6i.metal 82.95% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pb8o pb8o force-pushed the kvm-ptp branch 8 times, most recently from 0ff9fdb to a581d19 Compare October 23, 2023 09:24
@pb8o pb8o marked this pull request as ready for review October 23, 2023 09:35
pb8o added 11 commits October 23, 2023 13:47
Disabled some options that we don't seem to need for our integration
tests. There's still options we can disable, but this already brings
down the kernel size from

- 5.10: 43.5 -> 26.0 (-35%)
- 4.14: 20.5 -> 19.0 (- 8%)

Signed-off-by: Pablo Barbáchano <[email protected]>
Those configs are not used anymore. The -ci- versions are the ones used.

Signed-off-by: Pablo Barbáchano <[email protected]>
Preliminary support for a Linux 6.1 guest kernel.

Started from the respective 5.10 kernel configs.

Signed-off-by: Pablo Barbáchano <[email protected]>
Use the CPU model name rather than having instance types hardcoded.

Signed-off-by: Pablo Barbáchano <[email protected]>
Convert the test to use sets of CPU features. This has two main
benefits: 1) it is easy to see the differences of each set, and 2) if
the test fails, it will print the difference of the sets, which helps
troubleshooting.

Signed-off-by: Pablo Barbáchano <[email protected]>
Add preliminary support for 6.1 guest kernel, but only limited to c7g
instances and host kernel Linux 6.1.

Also do not enable performance test checks for 6.1 guest kernels.

Signed-off-by: Pablo Barbáchano <[email protected]>
We don't have an immediate use case for it, but this may help in
troubleshooting use cases.

Signed-off-by: Pablo Barbáchano <[email protected]>
Add kvm_ptp in aarch64 to match the x86_64 config.

kvm_ptp was introduced in
https://lore.kernel.org/all/[email protected]/

Signed-off-by: Pablo Barbáchano <[email protected]>
Corrected to use the new chrony URL.

Signed-off-by: Pablo Barbáchano <[email protected]>
These timeouts are not needed, as we already have a 5 minute per test
timeout, and the test itself takes around 3 minutes.

Signed-off-by: Pablo Barbáchano <[email protected]>
Move the timeout per test, since we don't know a priori how many tests
are going to run, depending on the supported host and guest kernels.

Signed-off-by: Pablo Barbáchano <[email protected]>
Copy link
Contributor

@bchalios bchalios left a comment

Choose a reason for hiding this comment

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

I've only have some questions regarding the trimmed options from the guest kernels' .config. I don't think it's really important because we can always re-introduce them, but I'd like us to discuss whether we really want to trim down all these options.

Also, it is not clear to me whether this trimming, as well as some other changes (I've commented where I found them) are related with this PR directly. For example, maybe they helped with test stability(?).

If these changes are ground-work for the actual changes the PR does, then fine (even though we might consider doing this in a separate PR). If it's just piggy-backing unrelated work, then that's ok but at the very least let's mention this clearly in the PR description. It would help with the review.

resources/chroot.sh Outdated Show resolved Hide resolved
tests/framework/microvm.py Show resolved Hide resolved
.buildkite/pipeline_perf.py Show resolved Hide resolved
.buildkite/pipeline_ab.py Show resolved Hide resolved
@wearyzen wearyzen merged commit 6141c2c into firecracker-microvm:main Oct 23, 2023
5 checks passed
@pb8o pb8o deleted the kvm-ptp branch October 23, 2023 14:45
@pb8o pb8o mentioned this pull request Oct 29, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Awaiting author Indicates that an issue or pull request requires author action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants