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

Support TRIM in LUKS partitions mounted by Kairos #2693

Closed
Tracked by #2052
kreeuwijk opened this issue Jul 3, 2024 · 13 comments
Closed
Tracked by #2052

Support TRIM in LUKS partitions mounted by Kairos #2693

kreeuwijk opened this issue Jul 3, 2024 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@kreeuwijk
Copy link

Kairos version:
3.0.14

CPU architecture, OS, and Version:
Ubuntu 24.04

Describe the bug
Kairos does not pass the allow-discards config option to systemd-cryptsetup attach, causing the mounted LUKS partitions to not support TRIM. As a result, SSD drive performance falls off a cliff after some time. When a LUKS partition is mounted from a non-rotational disk, the allow-discards option should be automatically added so that weekly fstrim runs can trim the LUKS partitions as well.

DEVICE=nvme0n1

if [ "$(cat /sys/block/$DEVICE/queue/rotational)" == "0" ]
then
  OPTIONS="tpm2-device=auto,allow-discards"
else
  OPTIONS="tpm2-device=auto"
fi

This example shows that fstrim functions correctly when allow-discards is set:

# /usr/lib/systemd/systemd-cryptsetup attach nvme0n1p2 $(findfs PARTLABEL=oem) - tpm2-device=auto,allow-discards

# mount /dev/mapper/nvme0n1p2 /oem

# fstrim -v /oem
/oem: 4.7 GiB (5027479552 bytes) trimmed

To Reproduce
Deploy Kairos with Trusted Boot and attempt to run fstrim -v /oem

Expected behavior
Non-rotational disks should use LUKS partitions that have TRIM support enabled.

@kreeuwijk kreeuwijk added bug Something isn't working triage Add this label to issues that should be triaged and prioretized in the next planning call unconfirmed labels Jul 3, 2024
@mudler
Copy link
Member

mudler commented Jul 3, 2024

that can't be taken as slightly as a bug - systemd-cryptsetup does not set discards on SSDs for a reason probably. We need to spike on this first to see what would be a good approach here, or even let this to be configurable by the end-user.

See for instance also: https://wiki.debian.org/SSDOptimization on the topic.

@mudler mudler added spike and removed bug Something isn't working unconfirmed labels Jul 3, 2024
@mudler
Copy link
Member

mudler commented Jul 3, 2024

for the moment being - maybe we can investigate a workaround that does not require a patch release. E.g. a cloud config that can be shipped in the base image that reattach and remounts the partition with discard enabled on the HW that requires it.

@mudler mudler changed the title LUKS partitions mounted by Kairos don't support TRIM spike: LUKS partitions mounted by Kairos don't support TRIM Jul 3, 2024
@mudler mudler moved this to Todo 🖊 in 🧙Issue tracking board Jul 3, 2024
@mudler mudler removed the prio: high label Jul 3, 2024
@kreeuwijk
Copy link
Author

We do not need remounting of any mountpoints. All that's needed is kcrypt being adjusted to pass the allow-discards option to the systemd-cryptsetup command when attaching the LUKS partition. This will allow weekly fstrim runs to perform TRIM on LUKS partitions, just like it does for regular ext4 partitions.

@Itxaka
Copy link
Member

Itxaka commented Jul 4, 2024

workaround found:

cryptsetup refresh --allow-discards DEVICE will enable discards for the DEVICE while its mounted, it wont need to dettach/attach. This is only valid for attached devices. Once you detach and attach them the value will be lost and you will need to run it again

cryptsetup refresh --persistent --allow-discards DEVICE will do the same, but save the option to the luks header so on each dettach/attach operation the device will use that option forever. To remove the option just run the same but without the flag cryptsetup refresh --persistent DEVICE

@Itxaka
Copy link
Member

Itxaka commented Jul 4, 2024

@kreeuwijk confirmed this as working

@mudler
Copy link
Member

mudler commented Jul 4, 2024

for reference, this is the workaround from @kreeuwijk :

stages:
  boot.after:
    - name: Ensure encrypted partitions can be trimmed
      commands:
        - |
          DEVICES=$(lsblk -p -n -l -o NAME)
          if cat /proc/cmdline | grep rd.immucore.uki; then TRUSTED_BOOT="true"; fi
          for part in $DEVICES
          do
            if cryptsetup isLuks $part; then
              echo "Detected encrypted partition $part, ensuring TRIM is enabled..."
              if ! cryptsetup status ${part#/dev/} | grep discards; then
                echo "TRIM is not enabled on $part, enabling TRIM..."
                if [ "$TRUSTED_BOOT" = "true" ]; then
                  cryptsetup refresh --allow-discards --persistent ${part#/dev/}
                else
                  passphrase=$(echo '{ "data": "{ \"label\": \"LABEL\" }"}' | /system/discovery/kcrypt-discovery-challenger "discovery.password" | jq -r '.data')
                  echo $passphrase | cryptsetup refresh --allow-discards ${part#/dev/}
                fi
                if [ "$?" = "0" ]; then
                  echo "TRIM is now enabled on $part"
                else
                  echo "TRIM coud not be enabled on $part!"
                fi
              else
                echo "TRIM is already enabled on $part, nothing to do."
              fi
            fi
          done

@mauromorales mauromorales removed the triage Add this label to issues that should be triaged and prioretized in the next planning call label Jul 9, 2024
@jimmykarily
Copy link
Contributor

Planning decision: Let's try to implement this in immucore and control it with a config flag (in Kairos config).

@jimmykarily jimmykarily moved this from Todo 🖊 to In Progress 🏃 in 🧙Issue tracking board Jul 22, 2024
@jimmykarily jimmykarily moved this from In Progress 🏃 to Todo 🖊 in 🧙Issue tracking board Jul 22, 2024
@jimmykarily jimmykarily added enhancement New feature or request and removed spike labels Jul 22, 2024
@jimmykarily jimmykarily changed the title spike: LUKS partitions mounted by Kairos don't support TRIM LUKS partitions mounted by Kairos don't support TRIM Jul 22, 2024
@jimmykarily jimmykarily changed the title LUKS partitions mounted by Kairos don't support TRIM Support TRIM in LUKS partitions mounted by Kairos Jul 22, 2024
@Itxaka Itxaka self-assigned this Jul 23, 2024
@Itxaka Itxaka moved this from Todo 🖊 to In Progress 🏃 in 🧙Issue tracking board Jul 23, 2024
@Itxaka
Copy link
Member

Itxaka commented Jul 24, 2024

After a small discussion

  • --allow-insecure flag will always be used as it refers to the future possibility of mounting unencrypted parts. It doesnt automatically enable the discard option, it just allows to do it
  • agent will gain new config option for mouting encrypted partition with discard flag. It wont affect anything on install
  • On boot, immucore will mount oem, read the config, get the value of the discard and remount /oem with that flag if needed
  • On boot, immucore will mount persistent directly with the option as it can read it from the config in oem directly
  • defaults will be the same, no discard option on anything

@kreeuwijk
Copy link
Author

kreeuwijk commented Jul 24, 2024

@Itxaka we can't you pass the --allow-discards option to cryptsetup by default? We don't want to set the discard flag on the filesystem mount, we only want fstrim to be able to perform TRIM on crypted filesystems during its weekly run...

@Itxaka
Copy link
Member

Itxaka commented Jul 24, 2024

@Itxaka we can't you pass the --allow-discards option to cryptsetup by default? We don't want to set the discard flag on the filesystem mount, we only want fstrim to be able to perform TRIM on crypted filesystems during its weekly run...

We do? It's mentioned above, we will pass that to the crypt setup Luks creation by default?

@Itxaka
Copy link
Member

Itxaka commented Jul 29, 2024

kcrypt and agent will now create the luks encrypted partitions with the allow-discard option always enabled.

As the OS now have an auto-trim service running, I dont think we need to go any further. Trim will be auto-run and it should trim the system automatically.

@Itxaka
Copy link
Member

Itxaka commented Jul 29, 2024

@kreeuwijk will this cover the current use-case? Encrypted partitions have the allow-discard option on by default, underlying mounts are NOT mounted witht he discard option, trim service still works on the timer.

Or is there some scenario missing here? Notice that nobody recommends using the discard option on encrypted mounts by default as it weakens security, so we would prefer to NOT allow mounting the underlying partitions with the discard option.

Feel free to to reopen if we are missing something here

@Itxaka Itxaka closed this as completed Jul 29, 2024
@github-project-automation github-project-automation bot moved this from In Progress 🏃 to Done ✅ in 🧙Issue tracking board Jul 29, 2024
@kreeuwijk
Copy link
Author

@Itxaka this is great, thank you!

mauromorales added a commit that referenced this issue Aug 29, 2024
Focuses on these fixes:

- Kairos user ids change on upgrade, breaking ssh login #2797
- Long duration hang during boot #2802
- Support TRIM in LUKS partitions mounted by Kairos #2693
mauromorales added a commit that referenced this issue Aug 29, 2024
Focuses on these fixes:

- Kairos user ids change on upgrade, breaking ssh login #2797
- Long duration hang during boot #2802
- Support TRIM in LUKS partitions mounted by Kairos #2693
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

5 participants