Buildroot package for nfs root#2191
Conversation
|
Quick note for testing. I did find a reliable way to tag/identify individual cameras without solely referencing the MAC address. bootp_vci (option 60) can be set in the uboot env which will get sent during the initial DHCP request. This can be a useful key when deciding which kernel/rootfs to serve to the device. |
…nel ip autoconf is dhcp. Use static mac_pton helper for older linux kernel support.
…' into br-package-nfs-root
|
Sorry for the regression failure! That copy rule slipped through. Fixes
All gated CI regressions should pass now.
Any feedback would be appreciated once it passes CI tests. |
widgetii
left a comment
There was a problem hiding this comment.
This is the package-based redesign that came out of the #2168 discussion — replacing the per-vendor MAC-driver patches with an opt-in package — and it executes that direction well: one ipconfig.c patch at the early-network layer, fully opt-in via BR2_PACKAGE_OPENIPC_NFS_ROOT, attached through Buildroot's linux-kernel-extension infra. The added NFS RO/RW overlay init, DNS/NTP/hostname, SSH-key persistence to u-boot env, timezone derivation, and the make_nfsroot helper round it out nicely.
CI: green as of 1266d36 — 103 checks pass, CI Gate success. Worth noting the earlier 28-board build failure was real (not the wireguard/squashfs tarball drift from #2168): the pre-fix LINUX_PRE_BUILD_HOOKS += OPENIPC_NFS_ROOT_SYNC_ETHADDR_SOURCE ran the openipc_ethaddr.c cp on every board → cp: ... drivers/net/ethernet/openipc_ethaddr.c: No such file or directory on boards that don't enable the package / old kernels. Moving it into the Kconfig-gated extension PREPARE_KERNEL hook fixed it, and the green matrix empirically confirms the no-op-when-disabled property. 👍 Also verified DT-MAC-wins holds by construction (eth_platform_get_mac_address() checks DT/ACPI before the arch_get_platform_mac_address() fallback).
A few things before merge:
1. repack gate only sees the cmdline var, not the fragment. The top Makefile does include $(CONFIG) (board defconfig) and never sources the merged .config, so ifeq ($(BR2_PACKAGE_OPENIPC_NFS_ROOT),y) in repack is only true when the var is passed on the make line (which make_nfsroot does). But the description also advertises enabling via general/openipc.fragment — that path leaves the gate false, so a plain make BOARD=… runs the normal NOR repack and CHECK_SIZE fails on the missing rootfs.squashfs. The kernel-side gating is fine (it reads .config); only the repack gate has this gap. Either source the var from .config in the Makefile, or document make_nfsroot as the only supported enable path.
2. S40network never brings up lo on the NFS-root path. The is_nfs_root branch returns after "leaving eth0/lo as configured by kernel", but kernel ip-autoconfig brings up eth0, not loopback. Base S40network always runs ifup lo. Anything binding 127.0.0.1 will break — please ifup lo explicitly on the NFS path.
3. overlay/init duplicates general/overlay/init. The flash-overlay branch is copied verbatim with an elif … nfs branch appended; late-overlay rsync replaces the base /init, so it works but the two will drift. Consider making the NFS branch additive or factoring the shared logic. (Also a couple of trailing-whitespace lines in the new init.)
4. ether_addr_copy(dev->dev_addr, mac) and dev_addr const-ness. net_device.dev_addr became const in Linux 5.17 (writes must go via eth_hw_addr_set()). CI passing means current targets are < 5.17, so this is fine today — just a flag for whoever bumps the neo kernel later.
Minor: the default_mac sentinel correctly matches the 00:00:23:34:45:66 overlay fallback and the ether_addr_equal form addresses the earlier #2168 nit 👍; the s|…| sed password substitution escapes / & \ but not the | delimiter (safe since crypt/base64 hashes can't contain |, but worth a comment); and consider noting the baked-in default 12345 root password as not-for-production in the Kconfig help.
Lastly — could you drop the two boot logs into the PR description: one with ethaddr= on the cmdline showing the resulting ip link MAC, and one showing the DT MAC winning with no ethaddr=? That lets reviewers confirm both paths without rebuilding the matrix.
|
1. repack not seeing fragment definitions. 2. S40network never brings up lo on the NFS-root path. 3. overlay/init duplicates general/overlay/init. 4. ether_addr_copy(dev->dev_addr, mac) and dev_addr const-ness. 5. Password hash substitution and defaults. Log files |
Changes
Edit - Added requested logs to top comment above. |
|
@widgetii - Let me know if you need any other changes. |
widgetii
left a comment
There was a problem hiding this comment.
Thanks for the thorough turnaround — all four points from the last review are addressed and CI is green on 4867a46 (103 checks, CI Gate ✅). Confirmed each:
- repack/fragment — the
repack-finaltarget +-include $(BR_CONF)makes repack seeopenipc.fragment, so fragment-enable andmake_nfsrootare now equivalent. 👍 /initduplication — fully resolved: merged intogeneral/overlay/init(now$is_nfs_root-aware with@TMPFS_SIZE@) and the packageoverlay/dir is gone. No drift surface left — nice.dev_addrconst-ness —eth_hw_addr_set()with theLINUX_VERSION_CODE < 5.17inline shim is exactly right; future-proofed for the neo bump.- sed comment + insecure-default note — both added; the
12345-preserves-webui-first-login rationale makes sense.
The networking rework (no-op ifup/ifdown profiles so no process can tear down eth0 and strand the rootfs) is a clever fix for #2 — I like it better than the original S40network override. Two things on it:
(please fix before merge) Path typo in S39netprofiles backup guard. The guard tests the singular profile:
if [ ! -f /etc/network/profile/eth0 ]; then
cp /etc/network/interfaces.d/eth0 /etc/network/profiles/eth0 # plural
fi/etc/network/profile/... never exists, so the backup fires on every boot. On the 2nd boot interfaces.d/eth0 is already the no-op copy, so it overwrites the profiles/eth0 backup with the no-op — the original is lost. Self-heals on RO (fresh tmpfs overlay each boot) but is destructive on RW NFS root. Should be /etc/network/profiles/eth0 / /etc/network/profiles/interfaces in both the guard and the copy (and matching for interfaces).
(non-blocking, optional) Runtime file-swap vs. build-time install. S39netprofiles copies profiles around at every boot rather than the package just installing the no-op eth0/interfaces to their final paths at build time. The runtime approach is what keeps the package install-only and the base overlay untouched, so I understand the tradeoff — just flagging that a build-time install (where feasible) would remove the boot-time mutation and the backup dance entirely. Your call; not a blocker.
Minor: the header comments in files/eth0 and files/interfaces say "Symlink no-op profile" but it's a cp, not a symlink — cosmetic.
Fix the profile→profiles typo and I'm happy to merge. Thanks again for the persistence — this is in good shape.
|
@tinylabs — I drafted a wiki page documenting the NFS-root work so users can pick it up in their own workflows: OpenIPC/wiki#476 ( Could you give the package section a once-over for accuracy against the implementation? I also sent you a read invite on the wiki repo so I can add you as a formal reviewer once you accept (https://github.com/OpenIPC/wiki/invitations) — otherwise feel free to just comment on the PR. |
Kernel NFS root package
Changes
Testing
Compile
Notes
bootargs_override.log
boot_dtb_wins.log
Added logs
ethaddr_override.log
dtb_deadbeed1122_no_ethaddr.log