-
Notifications
You must be signed in to change notification settings - Fork 53
Network stack boot time optimization: decouple from DATA partition #638
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
base: develop
Are you sure you want to change the base?
Conversation
4f90c47 to
c601e5b
Compare
c601e5b to
b948d56
Compare
|
Pushed new commit to address a race condition we noticed during our test: Networkd service started too early and it sometimes failed to get the status info of the network interface from D-Bus because D-Bus was not yet ready. Added change to initialize D-Bus earlier and enforced dependency on dbus-broker.service in network-pre.target. |
5c4c9b9 to
a9b4687
Compare
|
Synced with @bcressey , we are adopting different implementation here - instead of using overlayfs for the /.bottlerocket/netdog to grant netdog access to the files on private partition, we will just grant netdog Pushed code changes for that. |
a9b4687 to
33a8fe6
Compare
|
Pushed a minor change to add |
85fd745 to
94a18a4
Compare
|
Pushed change so that we use |
| RequiresMountsFor=/.bottlerocket | ||
| After=systemd-networkd-wait-online.service systemd-resolved.service run-netdog.mount prepare-sysctl.service | ||
| Requires=systemd-networkd-wait-online.service systemd-resolved.service run-netdog.mount prepare-sysctl.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid the change to run-netdog.mount:
| RequiresMountsFor=/.bottlerocket | |
| After=systemd-networkd-wait-online.service systemd-resolved.service run-netdog.mount prepare-sysctl.service | |
| Requires=systemd-networkd-wait-online.service systemd-resolved.service run-netdog.mount prepare-sysctl.service | |
| RequiresMountsFor=/.bottlerocket /run/netdog | |
| After=systemd-networkd-wait-online.service systemd-resolved.service prepare-sysctl.service | |
| Requires=systemd-networkd-wait-online.service systemd-resolved.service prepare-sysctl.service |
I would probably also just add the mkdir -p /etc/sysctl.d as another ExecStart command, vs. adding another unit that systemd has to do the bookkeeping for.
The run-netdog.mount change is a little odd because that unit is already WantedBy network-pre.target, which is reached before any of these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think systemd ordering phase, /run/netdog gets mounted pretty early. This is just to explicitly call out the dependency here since we now use /run/netdog for the status files. IMO it does not hurt. Thoughts?
| z /var/lib/systemd/random-seed 600 root root - | ||
| R /var/lib/systemd/linger | ||
| D /var/lib/systemd/linger 0700 root root - | ||
| d /etc/sysctl.d 0700 root root - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm inclined to keep this for now, and just have any units that need this directory early arrange to create it for themselves.
Right now corndog also needs this directory, and doesn't create it, and it's conceptually weird for corndog to depend on netdog (or related units) for this.
It's outside the scope of this change, but I'd like to explore a better mechanism, like etc.target, where we can collect all the units needed to populate /etc, and we order almost everything after that, to stop the madness of every early unit needing to depend on selinux-policy-files.service or run the risk subtle breakage.
prepare-sysctl.service seems like a similar dependency where more units may end up needing it over time.
| Before=local-fs.target umount.target | ||
| After=dev-disk-by\x2dpartlabel-BOTTLEROCKET\x2dPRIVATE.device selinux-policy-files.service | ||
| Requires=dev-disk-by\x2dpartlabel-BOTTLEROCKET\x2dPRIVATE.device | ||
| Wants=selinux-policy-files.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not making this Required as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used Wants here to be consistent with elsewhere https://github.com/search?q=repo%3Abottlerocket-os%2Fbottlerocket-core-kit+%3Dselinux-policy-files.service&type=code
| Type=ext4 | ||
| Options=defaults,nosuid,nodev,noexec,noatime,private,context=system_u:object_r:private_t:s0 | ||
| Type=none | ||
| Options=rbind,rshared |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have to make this rshared? Why not just rprivate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking @bcressey 's comments - #638 (comment)
Most of these options aren't valid for bind mounts because they share the same filesystem superblock.
Generally it's useful to set up the recursive options so mounts are propagated in both locations
Create a direct mount point (/.bottlerocket) for the PRIVATE partition that bypasses the dependency on the DATA partition. Previously, /var/lib/bottlerocket was backed by the PRIVATE partition but required DATA partition availability through the /var -> /local/var bind mount chain. This change maintains backward compatibility by adding a bind mount from /.bottlerocket to /var/lib/bottlerocket, allowing network services to initialize in parallel with local-fs operations. Signed-off-by: Yutong Sun <[email protected]>
- Update path constants to use /run/netdog instead of /var/lib/netdog. - Remove OVERRIDE_NET_CONFIG_FILE as it's no longer needed. Signed-off-by: Yutong Sun <[email protected]>
- Drop dependency on tmpfiles. - Drop default dependencies and update generate-network-config.service with /.bottlerocket dependencies. - Create /etc/sysctl.d with ExecStart entry for write-network-status.service. - Drop default dependencies for write-network-status.service. - Explicitly add Before=network-online.target for write-network-status.service. Signed-off-by: Yutong Sun <[email protected]>
Fix race condition where network services attempt to use D-Bus functionality before the D-Bus broker is fully initialized: 1. Add DefaultDependencies=no to dbus.socket to allow D-Bus to start earlier in the boot process 2. Make network-pre.target require dbus-broker.service to ensure D-Bus is ready before network services start 3. Disable PrivateTmp for dbus-broker.service to remove dependency on systemd-tmpfiles and local-fs.target Signed-off-by: Yutong Sun <[email protected]>
Migrator depends on /var/lib/bottlerocket/, previously it was the mount point to PRIVATE partition, now that /.bottlerocket is the mount point and /var/lib/bottlerocket became the bind mount, the explicit dependency would be necessary. Signed-off-by: Yutong Sun <[email protected]>
94a18a4 to
8c34907
Compare
| RefuseManualStart=true | ||
| RefuseManualStop=true | ||
| Before=early-boot-config.service | ||
| Before=early-boot-config.service network-online.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add this explicit Before annotation, otherwise I get weird systemd ordering that write-network-status happens after network-online.target:
network-online.target @1.979s
└─systemd-networkd-wait-online.service @1.254s +722ms
└─systemd-networkd.service @1.189s +61ms
└─network-pre.target @1.186s
...
With this change, the write-network-status.service happens before the network-online.target.
network-online.target @1.788s
└─write-network-status.service @1.753s +33ms <-- listed as dependency
└─systemd-resolved.service @2.669s +217ms
└─systemd-sysctl.service @753ms +22ms
...
|
Pushed change to address comments from @bcressey and @arnaldo2792 |
|
Nice! |
| DefaultDependencies=false | ||
| After=dbus.socket | ||
| # Ensure the dbus user is created before starting dbus-broker | ||
| After=dbus.socket systemd-sysusers.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will have to make the same change to whippet.service now that I merged #661 😅
| @@ -1,9 +1,12 @@ | |||
| [Unit] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the commit message only explains what was done but not why. There is another commit in the series where you used a list and in each item, you explained why you did what you did. You should follow the same convention (or even better, favor a narrative rather than a bullet list)
| @@ -0,0 +1,10 @@ | |||
| [Unit] | |||
| # Ensure D-Bus is fully initialized before network services start | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing . at the end of this sentence
Issue number:
Closes #637
Description of changes:
This PR optimizes boot time by decoupling the network stack initialization from the DATA partition, allowing network services to start in parallel with local filesystem operations.
Boot sequence change overview
Direct PRIVATE Partition Mount
/.bottlerocketfor the PRIVATE partition that bypasses dependency on DATA partition/.bottlerocketto/var/lib/bottlerocketfor backward compatibility/.bottlerocket/netdogdirectly instead of/var/lib/netdogNetdog Updates
/.bottlerocket/netdoginstead of/var/lib/netdoggenerate-network-config.servicewith /.bottlerocket dependencies./etc/sysctl.dwithExecStartentry forwrite-network-status.service.write-network-status.serviceand explicitly addBefore=network-online.targetforwrite-network-status.service.systemd-tmpfilesfor directory creationD-Bus and Network Service Improvements
DefaultDependencies=notodbus.socketto allow early D-Bus initializationnetwork-pre.targetrequiredbus-broker.serviceto ensure D-Bus is ready before network services startPrivateTmpfordbus-broker.serviceto remove dependency onsystemd-tmpfilesandlocal-fs.targetTesting done [WIP]:
Click to expand testing details
Functional Testing
metal-devinstance to confirm thatnet.tomlinjected to PRIVATE partition can still be read and thatwrite-network-status.servicecan write to the/.bottlerocket/netdogpath with no issuepartition
Boot-time Testing [WIP]
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.