Merge pull request #16518 from yuwata/network-fix-failure-in-updating-mac-address

network: fix failures in updating MAC address

docs/DESKTOP_ENVIRONMENTS.md

@@ -91,16 +91,22 @@ global default for all (graphical) applications.
 
 ## XDG autostart integration
 
-To allow XDG autostart integration, systemd will ship a cross-desktop generator
-to create appropriate units for the autostart directory.
-Desktop Environments will be able to make use of this simply by starting the
-appropriate XDG related targets (representing e.g. content of the
-`$XDG_CURRENT_DESKTOP` environment variable to handle `OnlyShowIn/NotShowIn`).
-The names and ordering rules for these targets are to be defined.
+To allow XDG autostart integration, systemd ships a cross-desktop generator
+to create appropriate units for the autostart directory
+(`systemd-xdg-autostart-generator`).
+Desktop Environments can opt-in to using this by starting
+`xdg-desktop-autostart.target`. The systemd generator correctly handles
+`OnlyShowIn=` and `NotShowin=`. It also handles the KDE and GNOME specific
+`X-KDE-autostart-condition=` and `AutostartCondition=` by using desktop
+environment provided binaries in an `ExecCondition=` line.
 
-This generator will likely never support certain desktop specific extensions.
-One such example is the GNOME specific feature to bind a service to a settings
-variable.
+However, this generator is somewhat limited in what it supports. For example,
+all generated units will have `After=graphical-session.target` set on them,
+it may therefore not be useful to start session services.
+
+Desktop files can be marked to be explicitly excluded from the generator using the line
+`X-systemd-skip=true`. This should be set if an application provides its own
+systemd service file for startup.
 
 ## Startup and shutdown best practices
 

src/libsystemd-network/sd-radv.c

@@ -77,6 +77,12 @@ _public_ sd_event *sd_radv_get_event(sd_radv *ra) {
         return ra->event;
 }
 
+_public_ int sd_radv_is_running(sd_radv *ra) {
+        assert_return(ra, false);
+
+        return ra->state != SD_RADV_STATE_IDLE;
+}
+
 static void radv_reset(sd_radv *ra) {
         assert(ra);
 

src/network/networkd-link.c

@@ -3932,78 +3932,114 @@ int link_update(Link *link, sd_netlink_message *m) {
         /* The kernel may broadcast NEWLINK messages without the MAC address
            set, simply ignore them. */
         r = sd_netlink_message_read_ether_addr(m, IFLA_ADDRESS, &mac);
-        if (r >= 0) {
-                if (memcmp(link->mac.ether_addr_octet, mac.ether_addr_octet,
-                           ETH_ALEN)) {
+        if (r >= 0 && memcmp(link->mac.ether_addr_octet, mac.ether_addr_octet, ETH_ALEN) != 0) {
 
-                        memcpy(link->mac.ether_addr_octet, mac.ether_addr_octet,
-                               ETH_ALEN);
+                memcpy(link->mac.ether_addr_octet, mac.ether_addr_octet, ETH_ALEN);
 
-                        log_link_debug(link, "MAC address: "
-                                       "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
-                                       mac.ether_addr_octet[0],
-                                       mac.ether_addr_octet[1],
-                                       mac.ether_addr_octet[2],
-                                       mac.ether_addr_octet[3],
-                                       mac.ether_addr_octet[4],
-                                       mac.ether_addr_octet[5]);
+                log_link_debug(link, "Gained new MAC address: "
+                               "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+                               mac.ether_addr_octet[0],
+                               mac.ether_addr_octet[1],
+                               mac.ether_addr_octet[2],
+                               mac.ether_addr_octet[3],
+                               mac.ether_addr_octet[4],
+                               mac.ether_addr_octet[5]);
 
-                        if (link->ipv4ll) {
-                                r = sd_ipv4ll_set_mac(link->ipv4ll, &link->mac);
+                if (link->ipv4ll) {
+                        bool restart = sd_ipv4ll_is_running(link->ipv4ll) > 0;
+
+                        if (restart) {
+                                r = sd_ipv4ll_stop(link->ipv4ll);
                                 if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update MAC address in IPv4LL client: %m");
+                                        return log_link_warning_errno(link, r, "Could not stop IPv4LL client: %m");
                         }
 
-                        if (link->dhcp_client) {
-                                r = sd_dhcp_client_set_mac(link->dhcp_client,
-                                                           (const uint8_t *) &link->mac,
-                                                           sizeof (link->mac),
-                                                           ARPHRD_ETHER);
-                                if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update MAC address in DHCP client: %m");
+                        r = sd_ipv4ll_set_mac(link->ipv4ll, &link->mac);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update MAC address in IPv4LL client: %m");
 
-                                r = dhcp4_set_client_identifier(link);
+                        if (restart) {
+                                r = sd_ipv4ll_start(link->ipv4ll);
                                 if (r < 0)
-                                        return r;
+                                        return log_link_warning_errno(link, r, "Could not restart IPv4LL client: %m");
+                        }
+                }
+
+                if (link->dhcp_client) {
+                        r = sd_dhcp_client_set_mac(link->dhcp_client,
+                                                   (const uint8_t *) &link->mac,
+                                                   sizeof (link->mac),
+                                                   ARPHRD_ETHER);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update MAC address in DHCP client: %m");
+
+                        r = dhcp4_set_client_identifier(link);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not set DHCP client identifier: %m");
+                }
+
+                if (link->dhcp6_client) {
+                        const DUID* duid = link_get_duid(link);
+                        bool restart = sd_dhcp6_client_is_running(link->dhcp6_client) > 0;
+
+                        if (restart) {
+                                r = sd_dhcp6_client_stop(link->dhcp6_client);
+                                if (r < 0)
+                                        return log_link_warning_errno(link, r, "Could not stop DHCPv6 client: %m");
                         }
 
-                        if (link->dhcp6_client) {
-                                const DUID* duid = link_get_duid(link);
+                        r = sd_dhcp6_client_set_mac(link->dhcp6_client,
+                                                    (const uint8_t *) &link->mac,
+                                                    sizeof (link->mac),
+                                                    ARPHRD_ETHER);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update MAC address in DHCPv6 client: %m");
 
-                                r = sd_dhcp6_client_set_mac(link->dhcp6_client,
-                                                            (const uint8_t *) &link->mac,
-                                                            sizeof (link->mac),
-                                                            ARPHRD_ETHER);
+                        if (link->network->iaid_set) {
+                                r = sd_dhcp6_client_set_iaid(link->dhcp6_client, link->network->iaid);
                                 if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update MAC address in DHCPv6 client: %m");
-
-                                if (link->network->iaid_set) {
-                                        r = sd_dhcp6_client_set_iaid(link->dhcp6_client,
-                                                                     link->network->iaid);
-                                        if (r < 0)
-                                                return log_link_warning_errno(link, r, "Could not update DHCPv6 IAID: %m");
-                                }
-
-                                r = sd_dhcp6_client_set_duid(link->dhcp6_client,
-                                                             duid->type,
-                                                             duid->raw_data_len > 0 ? duid->raw_data : NULL,
-                                                             duid->raw_data_len);
-                                if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update DHCPv6 DUID: %m");
+                                        return log_link_warning_errno(link, r, "Could not update DHCPv6 IAID: %m");
                         }
 
-                        if (link->radv) {
-                                r = sd_radv_set_mac(link->radv, &link->mac);
+                        r = sd_dhcp6_client_set_duid(link->dhcp6_client,
+                                                     duid->type,
+                                                     duid->raw_data_len > 0 ? duid->raw_data : NULL,
+                                                     duid->raw_data_len);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update DHCPv6 DUID: %m");
+
+                        if (restart) {
+                                r = sd_dhcp6_client_start(link->dhcp6_client);
                                 if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update MAC for Router Advertisement: %m");
+                                        return log_link_warning_errno(link, r, "Could not restart DHCPv6 client: %m");
+                        }
+                }
+
+                if (link->radv) {
+                        bool restart = sd_radv_is_running(link->radv);
+
+                        if (restart) {
+                                r = sd_radv_stop(link->radv);
+                                if (r < 0)
+                                        return log_link_warning_errno(link, r, "Could not stop Router Advertisement: %m");
                         }
 
-                        if (link->ndisc) {
-                                r = sd_ndisc_set_mac(link->ndisc, &link->mac);
+                        r = sd_radv_set_mac(link->radv, &link->mac);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update MAC for Router Advertisement: %m");
+
+                        if (restart) {
+                                r = sd_radv_start(link->radv);
                                 if (r < 0)
-                                        return log_link_warning_errno(link, r, "Could not update MAC for ndisc: %m");
+                                        return log_link_warning_errno(link, r, "Could not restart Router Advertisement: %m");
                         }
                 }
+
+                if (link->ndisc) {
+                        r = sd_ndisc_set_mac(link->ndisc, &link->mac);
+                        if (r < 0)
+                                return log_link_warning_errno(link, r, "Could not update MAC for NDisc: %m");
+                }
         }
 
         old_master = link->master_ifindex;

src/systemd/sd-radv.h

@@ -50,6 +50,7 @@ sd_event *sd_radv_get_event(sd_radv *ra);
 
 int sd_radv_start(sd_radv *ra);
 int sd_radv_stop(sd_radv *ra);
+int sd_radv_is_running(sd_radv *ra);
 
 int sd_radv_set_ifindex(sd_radv *ra, int interface_index);
 int sd_radv_set_mac(sd_radv *ra, const struct ether_addr *mac_addr);

test/TEST-50-DISSECT/test.sh

@@ -10,6 +10,7 @@ TEST_NO_NSPAWN=1
 
 command -v mksquashfs >/dev/null 2>&1 || exit 0
 command -v veritysetup >/dev/null 2>&1 || exit 0
+command -v sfdisk >/dev/null 2>&1 || exit 0
 
 # Need loop devices for systemd-dissect
 test_create_image() {
@@ -25,9 +26,26 @@ test_create_image() {
         instmods loop =block
         instmods squashfs =squashfs
         instmods dm_verity =md
+        install_dmevent
         generate_module_dependencies
-        inst_binary mksquashfs
-        inst_binary veritysetup
+        inst_binary sfdisk
+        inst_binary losetup
+
+        BASICTOOLS=(
+            bash
+            cat
+        )
+        oldinitdir=$initdir
+        export initdir=$TESTDIR/minimal
+        mkdir -p $initdir
+        setup_basic_dirs
+        install_basic_tools
+        inst /usr/lib/os-release
+        ln -s ../usr/lib/os-release $initdir/etc/os-release
+        echo MARKER=1 >> $initdir/usr/lib/os-release
+        mksquashfs $initdir $oldinitdir/usr/share/minimal.raw
+        veritysetup format $oldinitdir/usr/share/minimal.raw $oldinitdir/usr/share/minimal.verity | grep '^Root hash:' | cut -f2 | tr -d '\n' > $oldinitdir/usr/share/minimal.roothash
+        export initdir=$oldinitdir
     )
 }
 

test/units/testsuite-50.sh

@@ -4,27 +4,109 @@
 set -ex
 set -o pipefail
 
+cleanup()
+{
+    if [ -z "${image_dir}" ]; then
+        return
+    fi
+    rm -rf "${image_dir}"
+}
+
 cd /tmp
 
-image=$(mktemp -d -t -p /tmp tmp.XXXXXX)
-if [ -z "${image}" ] || [ ! -d "${image}" ]; then
-	echo "Could not create temporary directory with mktemp under /tmp"
-	exit 1
+image_dir="$(mktemp -d -t -p /tmp tmp.XXXXXX)"
+if [ -z "${image_dir}" ] || [ ! -d "${image_dir}" ]; then
+    echo "mktemp under /tmp failed"
+    exit 1
 fi
 
-mkdir -p ${image}/usr/lib ${image}/etc
-cp /usr/lib/os-release ${image}/usr/lib/
-cp /etc/machine-id /etc/os-release ${image}/etc/
-mksquashfs ${image} ${image}.raw
-veritysetup format ${image}.raw ${image}.verity | grep '^Root hash:' | cut -f2 | tr -d '\n' > ${image}.roothash
+trap cleanup EXIT
+
+cp /usr/share/minimal.* "${image_dir}/"
+image="${image_dir}/minimal"
+roothash="$(cat ${image}.roothash)"
 
 /usr/lib/systemd/systemd-dissect ${image}.raw | grep -q -F "Found read-only 'root' partition of type squashfs with verity"
+/usr/lib/systemd/systemd-dissect ${image}.raw | grep -q -F "MARKER=1"
 /usr/lib/systemd/systemd-dissect ${image}.raw | grep -q -F -f /usr/lib/os-release
 
 mv ${image}.verity ${image}.fooverity
 mv ${image}.roothash ${image}.foohash
-/usr/lib/systemd/systemd-dissect ${image}.raw --root-hash=`cat ${image}.foohash` --verity-data=${image}.fooverity | grep -q -F "Found read-only 'root' partition of type squashfs with verity"
-/usr/lib/systemd/systemd-dissect ${image}.raw --root-hash=`cat ${image}.foohash` --verity-data=${image}.fooverity | grep -q -F -f /usr/lib/os-release
+/usr/lib/systemd/systemd-dissect ${image}.raw --root-hash=${roothash} --verity-data=${image}.fooverity | grep -q -F "Found read-only 'root' partition of type squashfs with verity"
+/usr/lib/systemd/systemd-dissect ${image}.raw --root-hash=${roothash} --verity-data=${image}.fooverity | grep -q -F "MARKER=1"
+/usr/lib/systemd/systemd-dissect ${image}.raw --root-hash=${roothash} --verity-data=${image}.fooverity | grep -q -F -f /usr/lib/os-release
+mv ${image}.fooverity ${image}.verity
+mv ${image}.foohash ${image}.roothash
+
+mkdir -p ${image_dir}/mount
+/usr/lib/systemd/systemd-dissect --mount ${image}.raw ${image_dir}/mount
+cat ${image_dir}/mount/usr/lib/os-release | grep -q -F -f /usr/lib/os-release
+cat ${image_dir}/mount/etc/os-release | grep -q -F -f /usr/lib/os-release
+cat ${image_dir}/mount/usr/lib/os-release | grep -q -F "MARKER=1"
+umount ${image_dir}/mount
+
+systemd-run -t --property RootImage=${image}.raw /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1"
+mv ${image}.verity ${image}.fooverity
+mv ${image}.roothash ${image}.foohash
+systemd-run -t --property RootImage=${image}.raw --property RootHash=${image}.foohash --property RootVerity=${image}.fooverity /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1"
+systemd-run -t --property RootImage=${image}.raw --property RootHash=${roothash} --property RootVerity=${image}.fooverity /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1"
+mv ${image}.fooverity ${image}.verity
+mv ${image}.foohash ${image}.roothash
+
+# Make a GPT disk on the fly, with the squashfs as partition 1 and the verity hash tree as partition 2
+machine="$(uname -m)"
+if [ "${machine}" = "x86_64" ]; then
+    root_guid=4f68bce3-e8cd-4db1-96e7-fbcaf984b709
+    verity_guid=2c7357ed-ebd2-46d9-aec1-23d437ec2bf5
+elif [ "${machine}" = "i386" ] || [ "${machine}" = "i686" ] || [ "${machine}" = "x86" ]; then
+    root_guid=44479540-f297-41b2-9af7-d131d5f0458a
+    verity_guid=d13c5d3b-b5d1-422a-b29f-9454fdc89d76
+elif [ "${machine}" = "aarch64" ] || [ "${machine}" = "aarch64_be" ] || [ "${machine}" = "armv8b" ] || [ "${machine}" = "armv8l" ]; then
+    root_guid=b921b045-1df0-41c3-af44-4c6f280d3fae
+    verity_guid=df3300ce-d69f-4c92-978c-9bfb0f38d820
+elif [ "${machine}" = "arm" ]; then
+    root_guid=69dad710-2ce4-4e3c-b16c-21a1d49abed3
+    verity_guid=7386cdf2-203c-47a9-a498-f2ecce45a2d6
+elif [ "${machine}" = "ia64" ]; then
+    root_guid=993d8d3d-f80e-4225-855a-9daf8ed7ea97
+    verity_guid=86ed10d5-b607-45bb-8957-d350f23d0571
+fi
+# du rounds up to block size, which is more helpful for partitioning
+root_size="$(du -k ${image}.raw | cut -f1)"
+verity_size="$(du -k ${image}.verity | cut -f1)"
+# 4MB seems to be the minimum size blkid will accept, below that probing fails
+dd if=/dev/zero of=${image}.gpt bs=512 count=$((8192+${root_size}*2+${verity_size}*2))
+# sfdisk seems unhappy if the size overflows into the next unit, eg: 1580KiB will be interpreted as 1MiB
+# so do some basic rounding up if the minimal image is more than 1 MB
+if [ ${root_size} -ge 1024 ]; then
+    root_size="$((${root_size}/1024 + 1))MiB"
+else
+    root_size="${root_size}KiB"
+fi
+verity_size="${verity_size}KiB"
+uuid="$(head -c 32 ${image}.roothash | cut -c -8)-$(head -c 32 ${image}.roothash | cut -c 9-12)-$(head -c 32 ${image}.roothash | cut -c 13-16)-$(head -c 32 ${image}.roothash | cut -c 17-20)-$(head -c 32 ${image}.roothash | cut -c 21-)"
+echo -e "label: gpt\nsize=${root_size}, type=${root_guid}, uuid=${uuid}" | sfdisk ${image}.gpt
+uuid="$(tail -c 32 ${image}.roothash | cut -c -8)-$(tail -c 32 ${image}.roothash | cut -c 9-12)-$(tail -c 32 ${image}.roothash | cut -c 13-16)-$(tail -c 32 ${image}.roothash | cut -c 17-20)-$(tail -c 32 ${image}.roothash | cut -c 21-)"
+echo -e "size=${verity_size}, type=${verity_guid}, uuid=${uuid}" | sfdisk ${image}.gpt --append
+sfdisk --part-label ${image}.gpt 1 "Root Partition"
+sfdisk --part-label ${image}.gpt 2 "Verity Partition"
+loop="$(losetup --show -P -f ${image}.gpt)"
+dd if=${image}.raw of=${loop}p1
+dd if=${image}.verity of=${loop}p2
+losetup -d ${loop}
+
+/usr/lib/systemd/systemd-dissect --root-hash ${roothash} ${image}.gpt | grep -q "Found read-only 'root' partition (UUID $(head -c 32 ${image}.roothash)) of type squashfs for .* with verity on partition #1"
+/usr/lib/systemd/systemd-dissect --root-hash ${roothash} ${image}.gpt | grep -q "Found read-only 'root-verity' partition (UUID $(tail -c 32 ${image}.roothash)) of type DM_verity_hash for .* on partition #2"
+/usr/lib/systemd/systemd-dissect --root-hash ${roothash} ${image}.gpt | grep -q -F "MARKER=1"
+/usr/lib/systemd/systemd-dissect --root-hash ${roothash} ${image}.gpt | grep -q -F -f /usr/lib/os-release
+
+/usr/lib/systemd/systemd-dissect --root-hash ${roothash} --mount ${image}.gpt ${image_dir}/mount
+cat ${image_dir}/mount/usr/lib/os-release | grep -q -F -f /usr/lib/os-release
+cat ${image_dir}/mount/etc/os-release | grep -q -F -f /usr/lib/os-release
+cat ${image_dir}/mount/usr/lib/os-release | grep -q -F "MARKER=1"
+umount ${image_dir}/mount
+
+systemd-run -t --property RootImage=${image}.gpt --property RootHash=${roothash} /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1"
 
 echo OK > /testok
 

travis-ci/managers/debian.sh

@@ -59,8 +59,9 @@ for phase in "${PHASES[@]}"; do
         RUN|RUN_GCC|RUN_CLANG)
             if [[ "$phase" = "RUN_CLANG" ]]; then
                 ENV_VARS="-e CC=clang -e CXX=clang++"
+                MESON_ARGS="--optimization=1"
             fi
-            docker exec $ENV_VARS -it $CONT_NAME meson --werror -Dtests=unsafe -Dslow-tests=true -Dsplit-usr=true -Dman=true build
+            docker exec $ENV_VARS -it $CONT_NAME meson --werror -Dtests=unsafe -Dslow-tests=true -Dsplit-usr=true -Dman=true $MESON_ARGS build
             $DOCKER_EXEC ninja -v -C build
             docker exec -e "TRAVIS=$TRAVIS" -it $CONT_NAME ninja -C build test
             ;;