When a [SR-IOV] section has no setting, e.g.
```ini
[SR-IOV]
VirtualFunction=0
```
then the kernel previously replied -EINVAL, as we send a rtnl message
with an empty IFLA_VF_INFO container.
See See do_setvfinfo() in net/core/rtnetlink.c of the kernel.
When a [SR-IOV] section that has an unsupported settings by the
interface driver, then previously the kernel partially applied
settings and returned -EOPNOTSUPP. E.f.
```ini
[SR-IOV]
VirtualFunction=0
LinkState=auto
Trust=true
MACAddress=02:01:00:3e:61:34
```
and the interface does not support configuring the link state, then
the MAC address is assigned, but the trust is not applied:
```
enp3s0f0: Failed to configure SR-IOV virtual function 0, ignoring: Operation not supported
vf 0 link/ether 02:01:00:3e:61:34 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state auto, trust off
```
To fix such issues, this makes networkd/udevd send each attribute
for VF one-by-one.
The configuration can easily fail when the target virtual function
does not exist, and there is nothing networkd can do in such case.
Also, it is overkill to make the physical interface entered to the
failed state in such case. Let's warn but ignore the failure.
The header requires struct sockaddr declared. So, otherwise, we need to
include sys/socket.h earlier than linux/vm_sockets.h.
Let's make the header includable at any place.
udev: Enable delegation without delegating any controllers
Delegation is enabled for udev so that it can mess around with the
cgroup hierarchy to avoid killing control processes when it calls
cg_kill in on_post() when it goes idle. We don't actually care about
any specific cgroup controllers in udev, so set Delegate= to enable
delegation without delegating any controllers
Follow up for https://github.com/systemd/systemd/pull/22752
various: do not use assert_se as a workaround in non-test code
This partially reverts 5332be60d3897c7b86d28cf7b9d61c5dc6847fd6. I expect that
there is no practical difference, but it seems philosophically wrong to use
assert_se(), i.e. for the generation of the code in non-debug builds, just to
suppress a warning. We have _unused_ for that, use it.
I verified that we don't get warnings with clang and -DNDEBUG=1 with this patch.
@DaanDeMeyer Obviously this doesn't fix nearly everything, so gradually
moving things over is probably a smart thing? It seems clang-tidy does
support drop in configs for example:
Its a bit strange that `WarningsAsErrors` isn't propagated, but dropping
this file in src/report/.clang-tiday invokes:
```
[1314/1543][1.5s] /usr/bin/clang-tidy --use-color -extra-arg=-fno-caret-diagnostics -p=/home/jelle/projects/systemd/build -quiet /home/jelle/projec
ts/systemd/src/repart/repart.c
../src/repart/repart.c:4715:41: error: argument name 'pubkey' in comment does not match parameter name 'public' [bugprone-argument-comment,-warning
s-as-errors]
4715 | /* pubkey= */ NULL, /* Turn this one off for the 2nd shard */
| ^
../src/shared/tpm2-util.h:281:108: note: 'public' declared here
281 | int tpm2_calculate_sealing_policy(const Tpm2PCRValue *pcr_values, size_t n_pcr_values, const TPM2B_PUBLIC *public, bool use_pin, const Tpm2
PCRLockPolicy *policy, TPM2B_DIGEST *digest);
| ^
```
So that seems to behave as intended :)
And in some cases I am not sure if switching to the correct argument is
an improvement ie.:
```
../src/bootctl/bootctl-reboot-to-firmware.c:66:51: [38;2;190;132;255m0;1;31merror: argument name 'dispatch_table' in comment does not match paramet
er name 'table' [bugprone-argument-comment,-warnings-as-errors]
66 | r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL);
| [38;2;190;132;255m0;1;32m ^
../src/systemd/sd-varlink.h:187:98: [38;2;190;132;255m0;1;36mnote: 'table' declared here
187 | int sd_varlink_dispatch(sd_varlink *v, sd_json_variant *parameters, const sd_json_dispatch_field table[], void *userdata);
| [38;2;190;132;255m0;1;32m ^
```
or
```
../src/validatefs/validatefs.c:274:83: [38;2;190;132;255m0;1;31merror: argument name 'ret_len' in comment does not match parameter name 'len' [bugprone-argument-comment,-warnings-as-errors]
274 | (void) blkid_probe_lookup_value(b, "PART_ENTRY_TYPE", &v, /* ret_len= */ NULL);
| [38;2;190;132;255m0;1;32m ^
/usr/include/blkid/blkid.h:455:52: [38;2;190;132;255m0;1;36mnote: 'len' declared here
455 | const char **data, size_t *len)
| [38;2;190;132;255m0;1;32m ^
```
But that's also half a style thing with `len` winning over `ret_len`.
Notable changes are
- SIGTERM is the highest among others, to make not udevd queue too
many events, as we need to serialize them anyway.
- device monitor has the second highest priority, to make 'remove'
uevents received earlier than IN_IGNORED inotify events. Otherwise,
after IN_IGNORED is received, if there is no queued event,
/run/udev/queue file will be removed by the post-event source of the
inotify event, and 'udevadm settle' or friends may wrongly finish,
even we will soon queue 'remove' uevents for the device.
This change should fix the recent instability of TEST-64-UDEV-STORAGE.
udev: do not remove /run/udev/queue file when we are synthesizing events
Note, it should be safe even if we synthesize no event, e.g. when the
device has been already removed. In such case, the post event after
SIGCHLD will remove the file.
udev: try again to create /run/udev/queue when queueing the next event
This is mostly a paranoia, but if we failed to create /run/udev/queue
for some reasons on queueing an event, previously we would never create
the file until once the queue became empty. This makes in such case we
try to create the file again when queueing the next event.
networkd: reduce the default IPv4 DAD (ACD) timeout and make it configurable (#37138)
RFC 5227 specifies randomized intervals to avoid that a large number of
hosts powered up at the same time send their message simultaneously.
Performing the conflict detection takes a variable time between 4 and 7
seconds from the beginning to the first announcement, as shown by the
following diagram where P indicates a probe and A an announcement:
```
time(s) 0 1 2 3 4 5 6 7 8 9
+---+---+---+---+---+---+---+---+---+
SHORTEST P P P A A
LONGEST P P P A A
```
The host can't use the address until the first announcement is sent. 7
seconds is a very long time on modern computers especially considering
the fact that the round-trip time on current LAN technologies is at most
few milliseconds. Section 2.2 of the RFC addresses this matter and hints
that a future standard will adjust those timeouts; however that standard
doesn't exist yet.
Make the timeout configurable via a new
"IPv4DuplicateAddressDetectionTimeoutSec=" option. The intervals defined
in the RFC are then scaled proportionally so that the duration of the
conflict detection takes at most the given value. Interval happening
after the first announcement are not scaled, as recommended by the RFC.
Also reduce the default value from 7s to 200ms, which is a more suitable
value for today's technology.
The original timeout of 7 seconds is very long for today's networks. Reduce it
to 200ms. Note that this change also affects IPv4 link-local addressing.
RFC 5227 specifies randomized intervals to avoid that a large number of hosts
powered up at the same time send their message simultaneously. Performing the
conflict detection takes a variable time between 4 and 7 seconds from the
beginning to the first announcement, as shown by the following diagram where P
indicates a probe and A an announcement:
time(s) 0 1 2 3 4 5 6 7 8 9
+---+---+---+---+---+---+---+---+---+
SHORTEST P P P A A
LONGEST P P P A A
The host can't use the address until the first announcement is sent. 7 seconds
is a very long time on modern computers especially considering the fact that
the round-trip time on current LAN technologies is at most few milliseconds.
Section 2.2 of the RFC addresses this matter and hints that a future standard
will adjust those timeouts; however that standard doesn't exist yet.
Make the timeout configurable via a new IPv4DuplicateAddressDetectionTimeoutSec=
option. The intervals defined in the RFC are then scaled proportionally so that
the duration of the conflict detection takes at most the given value. Interval
happening after the first announcement are not scaled, as recommended by the
RFC.
man/systemd-nspawn: call dnf with --use-host-config
This is needed for dnf5. But dnf-4 doesn't know about it. So also add a hint to
skip the option with dnf-4. We can drop this later when dnf5 is the default
everywhere.
Also, s/vim-minimal/nano/. Nano is the default editor in Fedora since
https://fedoraproject.org/wiki/Changes/UseNanoByDefault.
selinux: Disable selinux logging in mac_init() as well
We currently only disable selinux logging in mac_selinux_setup(),
but not in mac_init(). We don't want libraries we use to log unless
we tell them to, so disable selinux's logging in mac_init() as well.
test: Add custom signal handlers to integration test wrapper script
meson will send SIGTERM if the test gets stuck and hits the timeout,
in which case we still want to do log saving and analysis, so let's
add some signal handlers which allow us to do that.
This won't be very useful until https://github.com/mesonbuild/meson/pull/14513
lands, since we only get half a second from meson to handle SIGTERM
before it sends SIGKILL, but let's land this already so we immediately
start taking advantage of the meson fix once it lands.
meson's target has a few issues:
- Runs on all source files regardless if they're included in the
build or not
- Doesn't have any dependencies on generated sources which means we
have to do a full build first before we can run clang-tidy
- Doesn't allow us to pass any extra arguments
To work around these, let's define our own clang-tidy target instead
using llvm's run-clang-tidy script. Alongside the clang-tidy target,
let's start keeping track of all generated sources which we make the
clang-tidy target depend on. We also add a new target which will only
generate source files which is useful for setting up the source tree
for running code analysis against it.
mkosi: Run clangd within the tools tree instead of the build container
Running within the build sandbox has a number of disadvantages:
- We have a separate clangd cache for each distribution/release combo
- It requires to build the full image before clangd can be used
- It breaks every time the image becomes out of date and requires a
rebuild
- We can't look at system headers as we don't have the knowledge to map
them from inside the build sandbox to the corresponding path on the host
Instead, let's have mkosi.clangd run clangd within the tools tree. We
already require building systemd for both the host and the target anyway,
and all the dependencies to build systemd are installed in the tools tree
already for that, as well as clangd since it's installed together with the
other clang tooling we install in the tools tree. Unlike the previous approach,
this approach only requires the mkosi tools tree to be built upfront, which has
a much higher chance of not invalidating its cache. We can also trivially map
system header lookups from within the sandbox to the path within mkosi.tools
on the host so that starts working as well.
Let's add a basic clang-tidy check to the linter workflow. This
gives us the following:
- A check so that we don't introduce any new cyclic header dependencies
- A check to make sure all of our header files are standalone, as clang-tidy
will fail to parse header files that don't include all their dependencies.
Add .clangd configuration file that disables the unused include check
The clangd include checker is rather broken and is littered with false
positives in our codebase, so let's disable the feature until it improves
a bit.
We already have LOG_CONTEXT_PUSH_EXEC() which with two additions
does exactly the same as the custom logging macros, so let's get rid
of the custom logging macros and use LOG_CONTEXT_PUSH_EXEC() instead.
timedate: Drop custom logging macros in favor of log context
Additionally, using the log context makes sure the extra fields are
applied to all log messages generated while the context is in place,
rather than only log messages logged with log_unit_xxx() in timedated
itself.
It also means the unit name is prefixed to all log messages logged
within that context. While it's not clear whether we always want the
unit name to be attached to library log messages, we also don't want
the unit name to not be attached to any library log messages, so we opt
to add more rather than less information by adding the unit name everywhere
for now.
In the future we can look into some log.h helpers to enable/disable
adding the prefix to the following log messages or not.
unit: Make sure individual unit maximum log level always takes priority
Currently LogLevelMax= can only be used to decrease the maximum log level
for a unit but not to increase it. Let's make sure the latter works as
well, so LogLevelMax=debug can be used to enable debug logging for specific
units without enabling debug logging globally.
basic: Move various macros from assert-util.h to assert-fundamental.h
ASSERT_PTR() and friends in assert-fundamental.h make use of assert()
and assert_se() which when not building for sd-boot are defined in
assert-util.h. Because assert() from glibc is only overridden in
assert-util.h, the macros in assert-fundamental.h still end up using
the glibc assert.
Let's fix this by moving the required macros and related logic to
assert-fundamental.h.
tree-wide: Use assert_se() instead of assert() in various places
assert() is compiled away if NDEBUG is set which causes an unused
variable warning in various places when the next commit is applied
so let's use assert_se() to avoid these warnings.
journal: Always compile journal authentication related files
Tooling such as clang-tidy is bad at dealing with condition
compilation on the build system level instead of at the source file
level. What happens? It still tries to analyze the file and fails
horribly if the required headers aren't available. Let's work around
the issue and make things more consistent at the same time by doing
the necessary HAVE_GCRYPT checks inside of the source files instead
of doing them at the build system level.
We also add some typedefs to allow getting rid of various HAVE_GCRYPT
checks.
libsystemd: Skip _sd-common.h include check when __clang_analyzer__ is defined
This check doesn't work properly when tools such as clang-tidy are
analyzing headers. Since we don't care about the check in that case,
skip it if __clang_analyzer__ is defined similar to how we skip it as
well if __COVERITY__ is defined.
The static inline declaration in iovec-fundamental.h causes static
analyze tooling warnings (clang-tidy). Let's get around it by making
free() non-inline. LTO will make sure it's still inlined properly.
basic: Use _Static_assert() in missing_audit.h instead of assert_cc()
We want to make the header standalone so it includes all the stuff it
needs. However, including macro.h for assert_cc() doesn't work because
of generate-audit_type-list.sh which would have to become more complex
to handle the extra include directories.
Instead, let's just use _Static_assert() directly which is a builtin and
doesn't need any extra includes.
resolve: Move implementations of some functions to resolved-dns-packet.c
These depend on a full definition of DnsResourceRecord which we want
to forward declare in the next commit, so let's move the implementation
of these functions to the implementation file and make them lowercase.
network: Remove circular header dependencies in network/tc
Include the headers in the implementation file (and the gperf file)
instead of in the qdisc.h header. I opted to keep them separate from
the other headers since they're slightly different than regular headers.