Reading /proc/cpuinfo is a surprisingly expensive operation. Since
kernel version 4.12 [1], opening /proc/cpuinfo on an x86 system can
block for around 20 milliseconds while the kernel samples the current
CPU frequency. There is a very recent patch [2] which gets rid of the
delay, but has yet to make it into the mainline kenel. Regardless,
kernels for which opening /proc/cpuinfo takes 20ms will continue to be
run in production for years to come. libcontainer only opens
/proc/cpuinfo to read the processor feature flags so all the delays to
get an accurate snapshot of the CPU frequency are just wasted time.
If we wanted to, we could interrogate the CPU features directly from
userspace using the `CPUID` instruction. However, Intel and AMD CPUs
have flags in different positions for their analogous sub-features and
there are CPU quirks [3] which would need to be accounted for. Some
Haswell server CPUs support RDT/CAT but are missing the `CPUID` flags
advertising their support; the kernel checks for support on that
processor family by probing the the hardware using privileged
RDMSR/WRMSR instructions [4]. This sort of probing could not be
implemented in userspace so it would not be possible to check for RDT
feature support in userspace without false negatives on some hardware
configurations.
It looks like libcontainer reads the CPU feature flags as a kind of
optimization so that it can skip checking whether the kernel supports an
RDT sub-feature if the hardware support is missing. As the kernel only
exposes subtrees in the `resctrl` filesystem for RDT sub-features with
hardware and kernel support, checking the CPU feature flags is redundant
from a correctness point of view. Remove the /proc/cpuinfo check as it
is an optimization which actually hurts performance.
[1]: https://unix.stackexchange.com/a/526679
[2]: https://lore.kernel.org/all/20220415161206.875029458@linutronix.de/
[3]: https://github.com/torvalds/linux/blob/7cf6a8a17f5b134b7e783c2d45c53298faef82a7/arch/x86/kernel/cpu/resctrl/core.c#L834-L851
[4]: https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/x86/kernel/cpu/resctrl/core.c#L111-L153
Signed-off-by: Cory Snider <csnider@mirantis.com>
Currently only amd64 and arm64v8 tarball have been checked in testdata,
while busybox bundle is downloaded on fly, and supports multiple architectures.
To enable integration tests for more architectures, the hello world
bundle is replaced by busybox one.
Signed-off-by: Shengjing Zhu <zhsj@debian.org>
This is a forward-port of commit 91fa032da4 ("ci: add basic checks for
CHANGELOG.md"), plus whatever changes were made in release-1.1 branch
(up to v1.1.3).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A new version of staticcheck (included into golangci-lint 1.46.2) gives
this new warning:
> libcontainer/factory_linux.go:230:59: SA9008: e refers to the result of a failed type assertion and is a zero value, not the value that was being type-asserted (staticcheck)
> err = fmt.Errorf("panic from initialization: %v, %s", e, debug.Stack())
> ^
> libcontainer/factory_linux.go:226:7: SA9008(related information): this is the variable being read (staticcheck)
> if e, ok := e.(error); ok {
> ^
Apparently, this is indeed a bug. Fix by using a different name for a
new variable, so we can access the old one under "else".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Due to a bug in commit 9c444070ec, when the user and mount namespaces
are used, and the bind mount is followed by the cgroup mount in the
spec, the cgroup is mounted using the bind mount's mount fd.
This can be reproduced with podman 4.1 (when configured to use runc):
$ podman run --uidmap 0:100:10000 quay.io/libpod/testimage:20210610 mount
Error: /home/kir/git/runc/runc: runc create failed: unable to start container process: error during container init: error mounting "cgroup" to rootfs at "/sys/fs/cgroup": mount /proc/self/fd/11:/sys/fs/cgroup/systemd (via /proc/self/fd/12), flags: 0x20502f: operation not permitted: OCI permission denied
or manually with the spec mounts containing something like this:
{
"destination": "/etc/resolv.conf",
"type": "bind",
"source": "/userdata/resolv.conf",
"options": [
"bind"
]
},
{
"destination": "/sys/fs/cgroup",
"type": "cgroup",
"source": "cgroup",
"options": [
"rprivate",
"nosuid",
"noexec",
"nodev",
"relatime",
"ro"
]
}
The issue was not found earlier since it requires using userns, and even then
mount fd is ignored by mountToRootfs, except for bind mounts, and all the bind
mounts have mountfd set, except for the case of cgroup v1's /sys/fs/cgroup
which is internally transformed into a bunch of bind mounts.
This is a minimal fix for the issue, suitable for backporting.
A test case is added which reproduces the issue without the fix applied.
Fixes: 9c444070ec ("Open bind mount sources from the host userns")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
the struct blockIODevice is used in an exported struct but it is not itself exported rendering that type inaccessible to
outside projects
Signed-off-by: cdoern <cdoern@redhat.com>
systemd emits very loud warnings when the path specified doesn't exist
(which can be the case for some of our default rules). We don't need the
ruleset we give systemd to be completely accurate (we discard some kinds
of wildcard rules anyway) so we can safely skip adding these.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Remove upper bound in integer sanity check
to not restrict the number of socket-activated
sockets passed in.
Closes#3488
Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
This removes the runc dependency on cpuguy83/md2man and
russross/blackfriday, which saves more than 400 KB (more than 300 KB
once stripped) from the binary.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use env directive instead of adding to $GITHUB_ENV.
2. Use bash herefile to feed sha256sum instead of pipe to grep.
3. Fix the hardcoded checksum (it was missing the first character).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add checking of downloaded tarball checksum.
In case it doesn't match the hardcoded value, the error is like this:
libseccomp-2.5.4.tar.gz: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
In case the checksum for a particular version is not specified in the
script, the error will look like this:
./script/seccomp.sh: line 29: SECCOMP_SHA256[${ver}]: unbound variable
In case the the hardcoded value in the file is of wrong format/length,
we'll get:
sha256sum: 'standard input': no properly formatted SHA256 checksum lines found
In any of these cases, the script aborts (due to set -e).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This job is failing with "No space left on device" lately, and this
helps to fix it.
Besides, it seems that caching does not help to shorten execution times
(validate/release job succeeds in under 8 minutes now; ymmv).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Define sizeof(int) as a constant, and also return ENOSYS earlier in the
filter if it doesn't increase the number of instructions we generate
(this is a negligible performance improvement but it does make it easier
to understand the generated filter stub).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Cory Snider