From 8d5c045435eeda5109a3ca73fb51ded0a89a77c4 Mon Sep 17 00:00:00 2001 From: Tony Fang Date: Mon, 7 Nov 2022 17:02:24 +0000 Subject: [PATCH 1/2] Use uname machine field to get CPU variant if fails at /proc/cpuinfo When images/containers in ARM arch were built/executed on x86 host, getCPUVariant will fail as it tries to look for /proc/cpuinfo, whose content is from the host. Adding a new method as fallback to check uname machine when it happens. Signed-off-by: Tony Fang --- platforms/cpuinfo.go | 111 +----------- platforms/cpuinfo_linux.go | 161 ++++++++++++++++++ ...{cpuinfo_test.go => cpuinfo_linux_test.go} | 17 +- platforms/cpuinfo_other.go | 57 +++++++ platforms/database.go | 7 - 5 files changed, 238 insertions(+), 115 deletions(-) create mode 100644 platforms/cpuinfo_linux.go rename platforms/{cpuinfo_test.go => cpuinfo_linux_test.go} (78%) create mode 100644 platforms/cpuinfo_other.go diff --git a/platforms/cpuinfo.go b/platforms/cpuinfo.go index 23b6f180b002..e205f1b383ff 100644 --- a/platforms/cpuinfo.go +++ b/platforms/cpuinfo.go @@ -17,14 +17,9 @@ package platforms import ( - "bufio" - "fmt" - "os" "runtime" - "strings" "sync" - "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" ) @@ -37,108 +32,12 @@ var cpuVariantOnce sync.Once func cpuVariant() string { cpuVariantOnce.Do(func() { if isArmArch(runtime.GOARCH) { - cpuVariantValue = getCPUVariant() + var err error + cpuVariantValue, err = getCPUVariant() + if err != nil { + log.L.Errorf("Error getCPUVariant for OS %s : %v", runtime.GOOS, err) + } } }) return cpuVariantValue } - -// For Linux, the kernel has already detected the ABI, ISA and Features. -// So we don't need to access the ARM registers to detect platform information -// by ourselves. We can just parse these information from /proc/cpuinfo -func getCPUInfo(pattern string) (info string, err error) { - if !isLinuxOS(runtime.GOOS) { - return "", fmt.Errorf("getCPUInfo for OS %s: %w", runtime.GOOS, errdefs.ErrNotImplemented) - } - - cpuinfo, err := os.Open("/proc/cpuinfo") - if err != nil { - return "", err - } - defer cpuinfo.Close() - - // Start to Parse the Cpuinfo line by line. For SMP SoC, we parse - // the first core is enough. - scanner := bufio.NewScanner(cpuinfo) - for scanner.Scan() { - newline := scanner.Text() - list := strings.Split(newline, ":") - - if len(list) > 1 && strings.EqualFold(strings.TrimSpace(list[0]), pattern) { - return strings.TrimSpace(list[1]), nil - } - } - - // Check whether the scanner encountered errors - err = scanner.Err() - if err != nil { - return "", err - } - - return "", fmt.Errorf("getCPUInfo for pattern: %s: %w", pattern, errdefs.ErrNotFound) -} - -func getCPUVariant() string { - if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { - // Windows/Darwin only supports v7 for ARM32 and v8 for ARM64 and so we can use - // runtime.GOARCH to determine the variants - var variant string - switch runtime.GOARCH { - case "arm64": - variant = "v8" - case "arm": - variant = "v7" - default: - variant = "unknown" - } - - return variant - } - if runtime.GOOS == "freebsd" { - // FreeBSD supports ARMv6 and ARMv7 as well as ARMv4 and ARMv5 (though deprecated) - // detecting those variants is currently unimplemented - var variant string - switch runtime.GOARCH { - case "arm64": - variant = "v8" - default: - variant = "unknown" - } - - return variant - } - - variant, err := getCPUInfo("Cpu architecture") - if err != nil { - log.L.WithError(err).Error("failure getting variant") - return "" - } - - // handle edge case for Raspberry Pi ARMv6 devices (which due to a kernel quirk, report "CPU architecture: 7") - // https://www.raspberrypi.org/forums/viewtopic.php?t=12614 - if runtime.GOARCH == "arm" && variant == "7" { - model, err := getCPUInfo("model name") - if err == nil && strings.HasPrefix(strings.ToLower(model), "armv6-compatible") { - variant = "6" - } - } - - switch strings.ToLower(variant) { - case "8", "aarch64": - variant = "v8" - case "7", "7m", "?(12)", "?(13)", "?(14)", "?(15)", "?(16)", "?(17)": - variant = "v7" - case "6", "6tej": - variant = "v6" - case "5", "5t", "5te", "5tej": - variant = "v5" - case "4", "4t": - variant = "v4" - case "3": - variant = "v3" - default: - variant = "unknown" - } - - return variant -} diff --git a/platforms/cpuinfo_linux.go b/platforms/cpuinfo_linux.go new file mode 100644 index 000000000000..fdddce2f2f67 --- /dev/null +++ b/platforms/cpuinfo_linux.go @@ -0,0 +1,161 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package platforms + +import ( + "bufio" + "bytes" + "fmt" + "os" + "runtime" + "strings" + + "github.com/containerd/containerd/errdefs" + "golang.org/x/sys/unix" +) + +// getMachineArch retrieves the machine architecture through system call +func getMachineArch() (string, error) { + var uname unix.Utsname + err := unix.Uname(&uname) + if err != nil { + return "", err + } + + arch := string(uname.Machine[:bytes.IndexByte(uname.Machine[:], 0)]) + + return arch, nil +} + +// For Linux, the kernel has already detected the ABI, ISA and Features. +// So we don't need to access the ARM registers to detect platform information +// by ourselves. We can just parse these information from /proc/cpuinfo +func getCPUInfo(pattern string) (info string, err error) { + + cpuinfo, err := os.Open("/proc/cpuinfo") + if err != nil { + return "", err + } + defer cpuinfo.Close() + + // Start to Parse the Cpuinfo line by line. For SMP SoC, we parse + // the first core is enough. + scanner := bufio.NewScanner(cpuinfo) + for scanner.Scan() { + newline := scanner.Text() + list := strings.Split(newline, ":") + + if len(list) > 1 && strings.EqualFold(strings.TrimSpace(list[0]), pattern) { + return strings.TrimSpace(list[1]), nil + } + } + + // Check whether the scanner encountered errors + err = scanner.Err() + if err != nil { + return "", err + } + + return "", fmt.Errorf("getCPUInfo for pattern: %s: %w", pattern, errdefs.ErrNotFound) +} + +// getCPUVariantFromArch get CPU variant from arch through a system call +func getCPUVariantFromArch(arch string) (string, error) { + + var variant string + + arch = strings.ToLower(arch) + + if arch == "aarch64" { + variant = "8" + } else if len(arch) >= 5 { + //Valid arch format is in form of armvXx + switch arch[3:5] { + case "v8": + variant = "8" + case "v7": + variant = "7" + case "v6": + variant = "6" + case "v5": + variant = "5" + case "v4": + variant = "4" + case "v3": + variant = "3" + default: + variant = "unknown" + } + } else { + return "", fmt.Errorf("getCPUVariantFromArch invalid arch : %s, %v", arch, errdefs.ErrInvalidArgument) + } + return variant, nil +} + +// getCPUVariant returns cpu variant for ARM +// We first try reading "Cpu architecture" field from /proc/cpuinfo +// If we can't find it, then fall back using a system call +// This is to cover running ARM in emulated environment on x86 host as this field in /proc/cpuinfo +// was not present. +func getCPUVariant() (string, error) { + + variant, err := getCPUInfo("Cpu architecture") + if err != nil { + if errdefs.IsNotFound(err) { + //Let's try getting CPU variant from machine architecture + arch, err := getMachineArch() + if err != nil { + return "", fmt.Errorf("failure getting machine architecture : %v", err) + } + + variant, err = getCPUVariantFromArch(arch) + if err != nil { + return "", fmt.Errorf("failure getting CPU variant from machine architecture : %v", err) + } + } else { + return "", fmt.Errorf("failure getting CPU variant : %v", err) + } + } + + // handle edge case for Raspberry Pi ARMv6 devices (which due to a kernel quirk, report "CPU architecture: 7") + // https://www.raspberrypi.org/forums/viewtopic.php?t=12614 + if runtime.GOARCH == "arm" && variant == "7" { + model, err := getCPUInfo("model name") + if err == nil && strings.HasPrefix(strings.ToLower(model), "armv6-compatible") { + variant = "6" + } + } + + switch strings.ToLower(variant) { + case "8", "aarch64": + variant = "v8" + case "7", "7m", "?(12)", "?(13)", "?(14)", "?(15)", "?(16)", "?(17)": + variant = "v7" + case "6", "6tej": + variant = "v6" + case "5", "5t", "5te", "5tej": + variant = "v5" + case "4", "4t": + variant = "v4" + case "3": + variant = "v3" + default: + variant = "unknown" + } + + return variant, nil +} diff --git a/platforms/cpuinfo_test.go b/platforms/cpuinfo_linux_test.go similarity index 78% rename from platforms/cpuinfo_test.go rename to platforms/cpuinfo_linux_test.go index fca6b69ab12f..ba0e775b988d 100644 --- a/platforms/cpuinfo_test.go +++ b/platforms/cpuinfo_linux_test.go @@ -22,13 +22,18 @@ import ( ) func TestCPUVariant(t *testing.T) { - if !isArmArch(runtime.GOARCH) || !isLinuxOS(runtime.GOOS) { + if !isArmArch(runtime.GOARCH) { t.Skip("only relevant on linux/arm") } variants := []string{"v8", "v7", "v6", "v5", "v4", "v3"} - p := getCPUVariant() + p, err := getCPUVariant() + if err != nil { + t.Fatalf("Error getting CPU variant: %v\n", err) + return + } + for _, variant := range variants { if p == variant { t.Logf("got valid variant as expected: %#v = %#v\n", p, variant) @@ -38,3 +43,11 @@ func TestCPUVariant(t *testing.T) { t.Fatalf("could not get valid variant as expected: %v\n", variants) } + +func TestGetCPUVariantFromArch(t *testing.T) { + + if !isArmArch(runtime.GOARCH) { + t.Skip("only relevant on linux/arm") + } + +} diff --git a/platforms/cpuinfo_other.go b/platforms/cpuinfo_other.go new file mode 100644 index 000000000000..9c6102846358 --- /dev/null +++ b/platforms/cpuinfo_other.go @@ -0,0 +1,57 @@ +//go:build !linux +// +build !linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package platforms + +import ( + "runtime" +) + +func getCPUVariant() (string, error) { + + var variant string + + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + // Windows/Darwin only supports v7 for ARM32 and v8 for ARM64 and so we can use + // runtime.GOARCH to determine the variants + switch runtime.GOARCH { + case "arm64": + variant = "v8" + case "arm": + variant = "v7" + default: + variant = "unknown" + } + } else if runtime.GOOS == "freebsd" { + // FreeBSD supports ARMv6 and ARMv7 as well as ARMv4 and ARMv5 (though deprecated) + // detecting those variants is currently unimplemented + switch runtime.GOARCH { + case "arm64": + variant = "v8" + default: + variant = "unknown" + } + + } else { + return "", fmt.Errorf("getCPUVariant for OS %s: %v", runtime.GOOS, errdefs.ErrNotImplemented) + + } + + return variant, nil +} diff --git a/platforms/database.go b/platforms/database.go index dbe9957ca9db..2e26fd3b4fae 100644 --- a/platforms/database.go +++ b/platforms/database.go @@ -21,13 +21,6 @@ import ( "strings" ) -// isLinuxOS returns true if the operating system is Linux. -// -// The OS value should be normalized before calling this function. -func isLinuxOS(os string) bool { - return os == "linux" -} - // These function are generated from https://golang.org/src/go/build/syslist.go. // // We use switch statements because they are slightly faster than map lookups From 6e55234c38efe8604599146f01e0ca8d70a2a892 Mon Sep 17 00:00:00 2001 From: Tony Fang Date: Mon, 19 Dec 2022 22:07:43 +0000 Subject: [PATCH 2/2] Add unit test to function GetCPUVariantFromArch Add unit test to function GetCPUVariantFromArch Fix import issue on non-linux platforms Fix some style issue Signed-off-by: Tony Fang --- platforms/cpuinfo.go | 2 +- platforms/cpuinfo_linux.go | 12 ++-- platforms/cpuinfo_linux_test.go | 100 ++++++++++++++++++++++++++++++-- platforms/cpuinfo_other.go | 3 + 4 files changed, 104 insertions(+), 13 deletions(-) diff --git a/platforms/cpuinfo.go b/platforms/cpuinfo.go index e205f1b383ff..8c600fc96b1c 100644 --- a/platforms/cpuinfo.go +++ b/platforms/cpuinfo.go @@ -35,7 +35,7 @@ func cpuVariant() string { var err error cpuVariantValue, err = getCPUVariant() if err != nil { - log.L.Errorf("Error getCPUVariant for OS %s : %v", runtime.GOOS, err) + log.L.Errorf("Error getCPUVariant for OS %s: %v", runtime.GOOS, err) } } }) diff --git a/platforms/cpuinfo_linux.go b/platforms/cpuinfo_linux.go index fdddce2f2f67..722d86c3578c 100644 --- a/platforms/cpuinfo_linux.go +++ b/platforms/cpuinfo_linux.go @@ -70,7 +70,7 @@ func getCPUInfo(pattern string) (info string, err error) { return "", err } - return "", fmt.Errorf("getCPUInfo for pattern: %s: %w", pattern, errdefs.ErrNotFound) + return "", fmt.Errorf("getCPUInfo for pattern %s: %w", pattern, errdefs.ErrNotFound) } // getCPUVariantFromArch get CPU variant from arch through a system call @@ -82,7 +82,7 @@ func getCPUVariantFromArch(arch string) (string, error) { if arch == "aarch64" { variant = "8" - } else if len(arch) >= 5 { + } else if arch[0:4] == "armv" && len(arch) >= 5 { //Valid arch format is in form of armvXx switch arch[3:5] { case "v8": @@ -101,7 +101,7 @@ func getCPUVariantFromArch(arch string) (string, error) { variant = "unknown" } } else { - return "", fmt.Errorf("getCPUVariantFromArch invalid arch : %s, %v", arch, errdefs.ErrInvalidArgument) + return "", fmt.Errorf("getCPUVariantFromArch invalid arch: %s, %w", arch, errdefs.ErrInvalidArgument) } return variant, nil } @@ -119,15 +119,15 @@ func getCPUVariant() (string, error) { //Let's try getting CPU variant from machine architecture arch, err := getMachineArch() if err != nil { - return "", fmt.Errorf("failure getting machine architecture : %v", err) + return "", fmt.Errorf("failure getting machine architecture: %v", err) } variant, err = getCPUVariantFromArch(arch) if err != nil { - return "", fmt.Errorf("failure getting CPU variant from machine architecture : %v", err) + return "", fmt.Errorf("failure getting CPU variant from machine architecture: %v", err) } } else { - return "", fmt.Errorf("failure getting CPU variant : %v", err) + return "", fmt.Errorf("failure getting CPU variant: %v", err) } } diff --git a/platforms/cpuinfo_linux_test.go b/platforms/cpuinfo_linux_test.go index ba0e775b988d..c0b8b0f6fae5 100644 --- a/platforms/cpuinfo_linux_test.go +++ b/platforms/cpuinfo_linux_test.go @@ -17,8 +17,11 @@ package platforms import ( + "errors" "runtime" "testing" + + "github.com/containerd/containerd/errdefs" ) func TestCPUVariant(t *testing.T) { @@ -30,24 +33,109 @@ func TestCPUVariant(t *testing.T) { p, err := getCPUVariant() if err != nil { - t.Fatalf("Error getting CPU variant: %v\n", err) + t.Fatalf("Error getting CPU variant: %v", err) return } for _, variant := range variants { if p == variant { - t.Logf("got valid variant as expected: %#v = %#v\n", p, variant) + t.Logf("got valid variant as expected: %#v = %#v", p, variant) return } } - t.Fatalf("could not get valid variant as expected: %v\n", variants) + t.Fatalf("could not get valid variant as expected: %v", variants) } func TestGetCPUVariantFromArch(t *testing.T) { - if !isArmArch(runtime.GOARCH) { - t.Skip("only relevant on linux/arm") - } + for _, testcase := range []struct { + name string + input string + output string + expectedErr error + }{ + { + name: "Test aarch64", + input: "aarch64", + output: "8", + expectedErr: nil, + }, + { + name: "Test Armv8 with capital", + input: "Armv8", + output: "8", + expectedErr: nil, + }, + { + name: "Test armv7", + input: "armv7", + output: "7", + expectedErr: nil, + }, + { + name: "Test armv6", + input: "armv6", + output: "6", + expectedErr: nil, + }, + { + name: "Test armv5", + input: "armv5", + output: "5", + expectedErr: nil, + }, + { + name: "Test armv4", + input: "armv4", + output: "4", + expectedErr: nil, + }, + { + name: "Test armv3", + input: "armv3", + output: "3", + expectedErr: nil, + }, + { + name: "Test unknown input", + input: "armv9", + output: "unknown", + expectedErr: nil, + }, + { + name: "Test invalid input which doesn't start with armv", + input: "armxxxx", + output: "", + expectedErr: errdefs.ErrInvalidArgument, + }, + { + name: "Test invalid input whose length is less than 5", + input: "armv", + output: "", + expectedErr: errdefs.ErrInvalidArgument, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + t.Logf("input: %v", testcase.input) + variant, err := getCPUVariantFromArch(testcase.input) + + if err == nil { + if testcase.expectedErr != nil { + t.Fatalf("Expect to get error: %v, however no error got", testcase.expectedErr) + } else { + if variant != testcase.output { + t.Fatalf("Expect to get variant: %v, however %v returned", testcase.output, variant) + } + } + + } else { + if !errors.Is(err, testcase.expectedErr) { + t.Fatalf("Expect to get error: %v, however error %v returned", testcase.expectedErr, err) + } + } + }) + + } } diff --git a/platforms/cpuinfo_other.go b/platforms/cpuinfo_other.go index 9c6102846358..51fb62ea7195 100644 --- a/platforms/cpuinfo_other.go +++ b/platforms/cpuinfo_other.go @@ -20,7 +20,10 @@ package platforms import ( + "fmt" "runtime" + + "github.com/containerd/containerd/errdefs" ) func getCPUVariant() (string, error) {