treewide: stop using LZ4 for initrd compression

There are two issues at play here: One is a bug in pierrec/lz4 when using the legacy framing format [1]. This bit us when we hit a broken size region with CL:2130, taking hours to debug. The other is the fact that the Linux LZ4 frame format has significant design issues [2], especially with concatenanted initrds. The first issue could be fixed by switching to a different LZ4 implementation (we do even have the reference impl in the monorepo) but there is no API to generate the legacy frame format and things like [3], a patch carried by Ubuntu to fix more edge cases just do not inspire confidence in such a solution. Thus, this CL switches over to using zstd for compressing initrds. Zstd is slower than LZ4 for decompressing, but it still decompresses at multiple GB/s per core while having a much better compression ratio. It also doesn't have any Linux-specific bits and Linux uses the reference implementation for decoding, which should make it much more robust. So overall I think this is a good tradeoff. [1] pierrec/lz4#156 [2] lz4/lz4#956 (comment) [3] https://launchpadlibrarian.net/507407918/0001-unlz4-Handle-0-size-chunks-discard-trailing-padding-.patch Change-Id: I69cf69f2f361de325f4b39f2d3644ee729643716 Reviewed-on: https://review.monogon.dev/c/monogon/+/2313 Tested-by: Jenkins CI Reviewed-by: Serge Bazanski <[email protected]>
monogon-dev · Nov 14, 2023 · 62f1d36 · 62f1d36
1 parent 60461b2
commit 62f1d36
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 28 deletions.
diff --git a/cloud/agent/e2e/BUILD.bazel b/cloud/agent/e2e/BUILD.bazel
@@ -16,7 +16,7 @@ go_test(
         "//metropolis/pkg/pki",
         "//metropolis/proto/api",
         "@com_github_cavaliergopher_cpio//:cpio",
-        "@com_github_pierrec_lz4_v4//:lz4",
+        "@com_github_klauspost_compress//zstd",
         "@org_golang_google_grpc//:go_default_library",
         "@org_golang_google_grpc//credentials",
         "@org_golang_google_protobuf//proto",

diff --git a/cloud/agent/e2e/main_test.go b/cloud/agent/e2e/main_test.go
@@ -21,7 +21,7 @@ import (
 	"time"
 
 	"github.com/cavaliergopher/cpio"
-	"github.com/pierrec/lz4/v4"
+	"github.com/klauspost/compress/zstd"
 	"golang.org/x/sys/unix"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
@@ -183,7 +183,7 @@ func TestMetropolisInstallE2E(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	initramfsOrigPath, err := datafile.ResolveRunfile("cloud/agent/initramfs.cpio.lz4")
+	initramfsOrigPath, err := datafile.ResolveRunfile("cloud/agent/initramfs.cpio.zst")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -207,17 +207,19 @@ func TestMetropolisInstallE2E(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	compressedOut := lz4.NewWriter(initramfsFile)
-	compressedOut.Apply(lz4.LegacyOption(true))
-	cpioW := cpio.NewWriter(compressedOut)
+	compressedW, err := zstd.NewWriter(initramfsFile, zstd.WithEncoderLevel(1))
+	if err != nil {
+		t.Fatal(err)
+	}
+	cpioW := cpio.NewWriter(compressedW)
 	cpioW.WriteHeader(&cpio.Header{
 		Name: "/init.pb",
 		Size: int64(len(agentInitRaw)),
 		Mode: cpio.TypeReg | 0o644,
 	})
 	cpioW.Write(agentInitRaw)
 	cpioW.Close()
-	compressedOut.Close()
+	compressedW.Close()
 
 	grpcGuestFwd := fmt.Sprintf("guestfwd=tcp:%s-tcp:127.0.0.1:%d", grpcAddr.String(), grpcListenAddr.Port)
 	blobGuestFwd := fmt.Sprintf("guestfwd=tcp:%s-tcp:127.0.0.1:%d", blobAddr.String(), blobListenAddr.Port)

diff --git a/cloud/takeover/BUILD.bazel b/cloud/takeover/BUILD.bazel
@@ -19,7 +19,7 @@ go_library(
         "//net/dump",
         "//net/proto",
         "@com_github_cavaliergopher_cpio//:cpio",
-        "@com_github_pierrec_lz4_v4//:lz4",
+        "@com_github_klauspost_compress//zstd",
         "@org_golang_google_protobuf//proto",
         "@org_golang_x_sys//unix",
     ],

diff --git a/cloud/takeover/takeover.go b/cloud/takeover/takeover.go
@@ -27,7 +27,7 @@ import (
 	"time"
 
 	"github.com/cavaliergopher/cpio"
-	"github.com/pierrec/lz4/v4"
+	"github.com/klauspost/compress/zstd"
 	"golang.org/x/sys/unix"
 	"google.golang.org/protobuf/proto"
 
@@ -44,7 +44,7 @@ var kernel []byte
 //go:embed ucode.cpio
 var ucode []byte
 
-//go:embed cloud/agent/initramfs.cpio.lz4
+//go:embed cloud/agent/initramfs.cpio.zst
 var initramfs []byte
 
 // newMemfile creates a new file which is not located on a specific filesystem,
@@ -124,17 +124,19 @@ func setupTakeover() (*api.TakeoverSuccess, error) {
 	}
 
 	// Append AgentInit spec to initramfs
-	compressedOut := lz4.NewWriter(initramfsFile)
-	compressedOut.Apply(lz4.LegacyOption(true))
-	cpioW := cpio.NewWriter(compressedOut)
+	compressedW, err := zstd.NewWriter(initramfsFile, zstd.WithEncoderLevel(1))
+	if err != nil {
+		return nil, fmt.Errorf("while creating zstd writer: %w", err)
+	}
+	cpioW := cpio.NewWriter(compressedW)
 	cpioW.WriteHeader(&cpio.Header{
 		Name: "/init.pb",
 		Size: int64(len(agentInitRaw)),
 		Mode: cpio.TypeReg | 0o644,
 	})
 	cpioW.Write(agentInitRaw)
 	cpioW.Close()
-	compressedOut.Close()
+	compressedW.Close()
 
 	agentParams := bootparam.Params{
 		bootparam.Param{Param: "quiet"},

diff --git a/go.mod b/go.mod
@@ -108,7 +108,6 @@ require (
 	github.com/mitchellh/go-wordwrap v1.0.0
 	github.com/opencontainers/runc v1.1.3
 	github.com/packethost/packngo v0.29.0
-	github.com/pierrec/lz4/v4 v4.1.14
 	github.com/pkg/errors v0.9.1
 	github.com/pkg/sftp v1.10.1
 	github.com/prometheus/node_exporter v1.3.1
@@ -310,7 +309,7 @@ require (
 	github.com/josharian/native v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/karrick/godirwalk v1.16.1 // indirect
-	github.com/klauspost/compress v1.14.2 // indirect
+	github.com/klauspost/compress v1.17.2
 	github.com/kr/fs v0.1.0 // indirect
 	github.com/kr/pty v1.1.8 // indirect
 	github.com/libopenstorage/openstorage v1.0.0 // indirect

diff --git a/go.sum b/go.sum
@@ -1432,8 +1432,9 @@ github.com/klauspost/compress v1.12.2/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8
 github.com/klauspost/compress v1.13.1/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg=
 github.com/klauspost/compress v1.13.4/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg=
 github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk=
-github.com/klauspost/compress v1.14.2 h1:S0OHlFk/Gbon/yauFJ4FfJJF5V0fc5HbBTJazi28pRw=
 github.com/klauspost/compress v1.14.2/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk=
+github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4=
+github.com/klauspost/compress v1.17.2/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE=
 github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
 github.com/koneu/natend v0.0.0-20150829182554-ec0926ea948d/go.mod h1:QHb4k4cr1fQikUahfcRVPcEXiUgFsdIstGqlurL0XL4=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
@@ -1824,7 +1825,6 @@ github.com/pierrec/lz4 v2.5.2+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi
 github.com/pierrec/lz4 v2.6.1+incompatible h1:9UY3+iC23yxF0UfGaYrGplQ+79Rg+h/q9FV9ix19jjM=
 github.com/pierrec/lz4 v2.6.1+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
 github.com/pierrec/lz4/v4 v4.1.8/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
-github.com/pierrec/lz4/v4 v4.1.14 h1:+fL8AQEZtz/ijeNnpduH0bROTu0O3NZAlPjQxGn8LwE=
 github.com/pierrec/lz4/v4 v4.1.14/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
 github.com/pingcap/errors v0.11.0/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8=
 github.com/pingcap/errors v0.11.5-0.20210425183316-da1aaba5fb63 h1:+FZIDR/D97YOPik4N4lPDaUcLDF/EQPogxtlHB2ZZRM=

diff --git a/metropolis/node/build/def.bzl b/metropolis/node/build/def.bzl
@@ -133,7 +133,7 @@ def _fsspec_core_impl(ctx, tool, output_file):
     return
 
 def _node_initramfs_impl(ctx):
-    initramfs_name = ctx.label.name + ".cpio.lz4"
+    initramfs_name = ctx.label.name + ".cpio.zst"
     initramfs = ctx.actions.declare_file(initramfs_name)
 
     _fsspec_core_impl(ctx, ctx.executable._mkcpio, initramfs)

diff --git a/metropolis/node/build/mkcpio/BUILD.bazel b/metropolis/node/build/mkcpio/BUILD.bazel
@@ -8,7 +8,7 @@ go_library(
     deps = [
         "//metropolis/node/build/fsspec",
         "@com_github_cavaliergopher_cpio//:cpio",
-        "@com_github_pierrec_lz4_v4//:lz4",
+        "@com_github_klauspost_compress//zstd",
         "@org_golang_x_sys//unix",
     ],
 )

diff --git a/metropolis/node/build/mkcpio/main.go b/metropolis/node/build/mkcpio/main.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 
 	"github.com/cavaliergopher/cpio"
-	"github.com/pierrec/lz4/v4"
+	"github.com/klauspost/compress/zstd"
 	"golang.org/x/sys/unix"
 
 	"source.monogon.dev/metropolis/node/build/fsspec"
@@ -47,16 +47,18 @@ type place struct {
 	Inode interface{}
 }
 
-// Usage: -out <out-path.cpio.lz4> fsspec-path...
+// Usage: -out <out-path.cpio.zst> fsspec-path...
 func main() {
 	flag.Parse()
 	outFile, err := os.Create(*outPath)
 	if err != nil {
 		log.Fatalf("Failed to open CPIO output file: %v", err)
 	}
 	defer outFile.Close()
-	compressedOut := lz4.NewWriter(outFile)
-	compressedOut.Apply(lz4.LegacyOption(true))
+	compressedOut, err := zstd.NewWriter(outFile)
+	if err != nil {
+		log.Fatalf("While initializing zstd writer: %v", err)
+	}
 	defer compressedOut.Close()
 	cpioWriter := cpio.NewWriter(compressedOut)
 	defer cpioWriter.Close()
@@ -168,7 +170,7 @@ func main() {
 			}); err != nil {
 				log.Fatalf("Failed to write cpio header for file %q: %v", i.Path, err)
 			}
-			if _, err := io.Copy(cpioWriter, inF); err != nil {
+			if n, err := io.Copy(cpioWriter, inF); err != nil || n != inFStat.Size() {
 				log.Fatalf("Failed to copy file %q into cpio: %v", i.SourcePath, err)
 			}
 			inF.Close()

diff --git a/metropolis/test/launch/cluster/cluster.go b/metropolis/test/launch/cluster/cluster.go
@@ -790,7 +790,7 @@ func LaunchCluster(ctx context.Context, opts ClusterOptions) (*Cluster, error) {
 		if err := launch.RunMicroVM(ctxT, &launch.MicroVMOptions{
 			Name:                   "nanoswitch",
 			KernelPath:             "metropolis/test/ktest/vmlinux",
-			InitramfsPath:          "metropolis/test/nanoswitch/initramfs.cpio.lz4",
+			InitramfsPath:          "metropolis/test/nanoswitch/initramfs.cpio.zst",
 			ExtraNetworkInterfaces: switchPorts,
 			PortMap:                portMap,
 			GuestServiceMap:        guestSvcMap,

diff --git a/third_party/go/repositories.bzl b/third_party/go/repositories.bzl
@@ -3180,8 +3180,8 @@ def go_repositories():
     go_repository(
         name = "com_github_klauspost_compress",
         importpath = "github.com/klauspost/compress",
-        sum = "h1:S0OHlFk/Gbon/yauFJ4FfJJF5V0fc5HbBTJazi28pRw=",
-        version = "v1.14.2",
+        sum = "h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4=",
+        version = "v1.17.2",
     )
     go_repository(
         name = "com_github_klauspost_cpuid",