Skip to content

Commit 70a47e2

Browse files
saiarcot895tirupatihemanth
saiarcot895
and
tirupatihemanth
committed
Update systemd-sonic-generator to make it work on Trixie
The version of systemd on Trixie no longer allows service generators to write to directories outside of what has been explicitly passed in. This affects DPU and multi-ASIC use cases. Therefore, rework systemd-sonic-generator to meet these requirements. Also, compile systemd-sonic-generator with C++17. The gtest headers no longer support C++11, so it needs to be bumped up to C++14 at minimum. Also, move logs for systemd-sonic-generator into /dev/kmsg (sonic-net#34) Co-authored-by: Hemanth Kumar Tirupati <tirupatihemanthkumar@gmail.com> Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
1 parent bae32ce commit 70a47e2

11 files changed

Lines changed: 332 additions & 417 deletions

File tree

‎files/build_templates/per_namespace/dash-ha.service.j2‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ StartLimitBurst=3
1010

1111
[Service]
1212
User={{ sonicadmin_user }}
13+
ExecCondition=/bin/bash /usr/local/bin/is-npu-or-dpu.sh
1314
ExecStartPre=/usr/local/bin/{{docker_container_name}}.sh start{% if multi_instance == 'true' %} %i{% endif %}
1415
ExecStart=/usr/local/bin/{{docker_container_name}}.sh wait{% if multi_instance == 'true' %} %i{% endif %}
1516
ExecStop=/usr/local/bin/{{docker_container_name}}.sh stop{% if multi_instance == 'true' %} %i{% endif %}

‎files/build_templates/sonic_debian_extension.j2‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -674,10 +674,16 @@ sudo cp $IMAGE_CONFIGS/config-chassisdb/config-chassisdb $FILESYSTEM_ROOT/usr/bi
674674
echo "config-chassisdb.service" | sudo tee -a $GENERATED_SERVICE_FILE
675675
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable config-chassisdb.service
676676

677+
# Copy basic script to handle NPU/DPU-specific things that cannot otherwise be handled
678+
# Note: Intentionally not setting executable bit because users generally won't
679+
# need to manually run these scripts.
680+
sudo cp $IMAGE_CONFIGS/midplane-network/is-npu-or-dpu.sh $FILESYSTEM_ROOT/usr/local/bin/
681+
sudo cp $IMAGE_CONFIGS/midplane-network/define-npu-specific-netdevs.sh $FILESYSTEM_ROOT/usr/local/bin/
682+
677683
# Copy midplane network service file for smart switch
678-
sudo cp $IMAGE_CONFIGS/midplane-network/bridge-midplane.netdev $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/bridge-midplane.netdev
684+
sudo cp $IMAGE_CONFIGS/midplane-network/bridge-midplane.netdev $FILESYSTEM_ROOT/usr/share/sonic/templates/bridge-midplane.netdev
679685
sudo cp $IMAGE_CONFIGS/midplane-network/bridge-midplane.network $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/bridge-midplane.network
680-
sudo cp $IMAGE_CONFIGS/midplane-network/dummy-midplane.netdev $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/dummy-midplane.netdev
686+
sudo cp $IMAGE_CONFIGS/midplane-network/dummy-midplane.netdev $FILESYSTEM_ROOT/usr/share/sonic/templates/dummy-midplane.netdev
681687
sudo cp $IMAGE_CONFIGS/midplane-network/dummy-midplane.network $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/dummy-midplane.network
682688
sudo cp $IMAGE_CONFIGS/midplane-network/midplane-network-npu.network $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/midplane-network-npu.network
683689
sudo cp $IMAGE_CONFIGS/midplane-network/midplane-network-dpu.network $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_NETWORK/midplane-network-dpu.network
@@ -686,29 +692,21 @@ sudo cp $IMAGE_CONFIGS/midplane-network/midplane-network-dpu.service $FILESYSTEM
686692

687693
# Disable smart switch unit by default, these units will be controlled by systemd-sonic-generator
688694
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/bridge-midplane.netdev
689-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/bridge-midplane.network
690695
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/dummy-midplane.netdev
691-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/dummy-midplane.network
692-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/midplane-network-npu.network
693-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/midplane-network-dpu.network
694-
echo "midplane-network-npu.service" | sudo tee -a $GENERATED_SERVICE_FILE
696+
#echo "midplane-network-npu.service" | sudo tee -a $GENERATED_SERVICE_FILE
695697
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable midplane-network-npu.service
696-
echo "midplane-network-dpu.service" | sudo tee -a $GENERATED_SERVICE_FILE
698+
#echo "midplane-network-dpu.service" | sudo tee -a $GENERATED_SERVICE_FILE
697699
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable midplane-network-dpu.service
698-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha.service
699-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu0.service
700-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu1.service
701-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu2.service
702-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu3.service
703-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu4.service
704-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu5.service
705-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu6.service
706-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu7.service
700+
707701
# According to the issue: https://github.com/systemd/systemd/issues/19106, To disable ManageForeignRoutingPolicyRules to avoid the ip rules being deleted by systemd-networkd
708702
sudo sed -i 's/#ManageForeignRoutingPolicyRules=yes/ManageForeignRoutingPolicyRules=no/g' $FILESYSTEM_ROOT/etc/systemd/networkd.conf
703+
sudo mkdir $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd.service.d
704+
sudo cp $IMAGE_CONFIGS/midplane-network/systemd-networkd.override.conf $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd.service.d/override.conf
709705

710-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/systemd-networkd.service
711-
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable systemd-networkd-wait-online.service
706+
# Copy systemd-networkd-wait-online override configuration
707+
sudo mkdir -p $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd-wait-online.service.d
708+
sudo cp $IMAGE_CONFIGS/midplane-network/systemd-networkd-wait-online.override.conf $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd-wait-online.service.d/override.conf
709+
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable systemd-networkd-wait-online.service
712710

713711
# Copy backend-acl script and service file
714712
sudo cp $IMAGE_CONFIGS/backend_acl/backend-acl.service $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/backend-acl.service
@@ -823,6 +821,11 @@ sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable fstrim.timer
823821
## copy platform rc.local
824822
sudo cp $IMAGE_CONFIGS/platform/rc.local $FILESYSTEM_ROOT/etc/
825823

824+
## Remove network-online.target dependency from rc-local.service debian.conf
825+
if [ -f $FILESYSTEM_ROOT/usr/lib/systemd/system/rc-local.service.d/debian.conf ]; then
826+
sudo sed -i '/^After=network-online.target/d' $FILESYSTEM_ROOT/usr/lib/systemd/system/rc-local.service.d/debian.conf
827+
fi
828+
826829
## copy blacklist file
827830
sudo cp $IMAGE_CONFIGS/platform/linux_kernel_bde.conf $FILESYSTEM_ROOT/etc/modprobe.d/
828831

@@ -1224,3 +1227,5 @@ sudo touch $FILESYSTEM_ROOT_ETC_SONIC/enable_multidb
12241227

12251228
# Install syslog counter plugin
12261229
install_deb_package $debs_path/syslog-counter_*.deb
1230+
1231+

‎files/dsc/dpu.service‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ Requires=local-fs.target
66
After=local-fs.target
77
Requires=ionic-modules.service
88
After=ionic-modules.service
9-
Before=midplane-network-dpu.service
109

1110
[Service]
1211
Type=oneshot

‎files/image_config/midplane-network/define-npu-specific-netdevs.sh‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/bin/bash
2+
3+
set -e
4+
5+
if /bin/bash /usr/local/bin/is-npu-or-dpu.sh -n; then
6+
cp --remove-destination /usr/share/sonic/templates/bridge-midplane.netdev /etc/systemd/network/
7+
cp --remove-destination /usr/share/sonic/templates/dummy-midplane.netdev /etc/systemd/network/
8+
fi

‎files/image_config/midplane-network/is-npu-or-dpu.sh‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#!/bin/bash
2+
3+
OPTIND=1 # Reset in case getopts has been used previously in the shell.
4+
5+
CHECK_ONLY_FOR_DPU=0
6+
CHECK_ONLY_FOR_NPU=0
7+
8+
while getopts "dn" opt; do
9+
case "$opt" in
10+
d) CHECK_ONLY_FOR_DPU=1
11+
;;
12+
n) CHECK_ONLY_FOR_NPU=1
13+
;;
14+
esac
15+
done
16+
17+
PLATFORM=$(grep onie_platform /host/machine.conf | cut -d= -f 2)
18+
19+
if [[ -z $PLATFORM ]]; then
20+
PLATFORM=$(grep aboot_platform /host/machine.conf | cut -d= -f 2)
21+
fi
22+
23+
HAS_DPU=0
24+
HAS_NPU=0
25+
26+
if [[ -f /usr/share/sonic/device/${PLATFORM}/platform.json ]]; then
27+
if jq -e '.DPUS' /usr/share/sonic/device/${PLATFORM}/platform.json >/dev/null; then
28+
HAS_NPU=1
29+
fi
30+
31+
if jq -e '.DPU' /usr/share/sonic/device/${PLATFORM}/platform.json >/dev/null; then
32+
HAS_DPU=1
33+
fi
34+
fi
35+
36+
if [[ ${CHECK_ONLY_FOR_DPU} -eq 0 ]] && [[ ${CHECK_ONLY_FOR_NPU} -eq 0 ]]; then
37+
if [[ ${HAS_DPU} -eq 1 ]] || [[ ${HAS_NPU} -eq 1 ]]; then
38+
exit 0
39+
fi
40+
elif [[ ${CHECK_ONLY_FOR_DPU} -eq 1 ]]; then
41+
if [[ ${HAS_DPU} -eq 1 ]]; then
42+
exit 0
43+
fi
44+
else
45+
if [[ ${HAS_NPU} -eq 1 ]]; then
46+
exit 0
47+
fi
48+
fi
49+
50+
exit 1

‎files/image_config/midplane-network/systemd-networkd-wait-online.override.conf‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[Service]
2+
ExecStart=
3+
ExecStart=/usr/lib/systemd/systemd-networkd-wait-online --timeout=600

‎files/image_config/midplane-network/systemd-networkd.override.conf‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[Service]
2+
ExecCondition=/bin/bash /usr/local/bin/is-npu-or-dpu.sh
3+
ExecStartPre=+/bin/bash /usr/local/bin/define-npu-specific-netdevs.sh

‎src/sonic-host-services‎

Submodule sonic-host-services updated 31 files

‎src/systemd-sonic-generator/Makefile‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
CC=gcc
2-
CFLAGS += -std=gnu99 -D_GNU_SOURCE
32

43
CXX=g++
5-
CXXFLAGS += -std=c++11 -D_GNU_SOURCE -I ./
4+
CXXFLAGS += -std=c++20 -D_GNU_SOURCE -I ./
65
LDFLAGS += -l stdc++ -lpthread -lboost_filesystem -lboost_system -lgtest -ljson-c
76

87
BINARY = systemd-sonic-generator

‎src/systemd-sonic-generator/ssg-test.cc‎

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Copyright (c) 2021 by Cisco Systems, Inc.
77
*------------------------------------------------------------------
88
*/
9+
#include <fstream>
910
#include <mutex>
1011
#include <string>
1112
#include <sys/stat.h>
@@ -71,8 +72,6 @@ const std::vector<std::string> generated_services = {
7172
"test.service", /* A single instance test service
7273
to test dependency creation */
7374
"test.timer", /* A timer service */
74-
"midplane-network-npu.service", /* A midplane network service for smart switch NPU*/
75-
"midplane-network-dpu.service", /* A midplane network service for smart switch DPU*/
7675
"database.service", /* A database service*/
7776
"database@.service", /* A database service for multi instances */
7877
};
@@ -225,15 +224,15 @@ class SsgMainTest : public SsgFunctionTest {
225224
bool find_string_in_file(std::string str,
226225
std::string file_name) {
227226
bool found = false;
228-
std::string line;
227+
std::string line;
229228

230-
std::ifstream file(TEST_UNIT_FILE_PREFIX + file_name);
231-
while (getline(file, line) && !found) {
232-
if (str == line) {
233-
found = true;
234-
break;
235-
}
229+
std::ifstream file(TEST_OUTPUT_DIR + file_name);
230+
while (getline(file, line) && !found) {
231+
if (str == line) {
232+
found = true;
233+
break;
236234
}
235+
}
237236
return found;
238237
}
239238

@@ -263,6 +262,18 @@ class SsgMainTest : public SsgFunctionTest {
263262
}
264263
}
265264

265+
void validate_output_dependency_list_ignore_multi_instance(
266+
std::vector<std::string> strs,
267+
std::string target,
268+
bool expected_result) {
269+
for (std::string str : strs) {
270+
bool finished = false;
271+
EXPECT_EQ(find_string_in_file(str, target),
272+
expected_result)
273+
<< "Error validating " + str + " in " + target;
274+
}
275+
}
276+
266277
/* This function validates if unit file paths in the provided
267278
* list strs exists or not as per expected_result. The unit files
268279
* should exist if expected_result is true.
@@ -302,7 +313,16 @@ class SsgMainTest : public SsgFunctionTest {
302313
* This function validates the generated dependencies in a Unit File.
303314
*/
304315
void validate_depedency_in_unit_file(const SsgMainConfig &cfg) {
305-
std::string test_service = "test.service";
316+
std::string test_service = "test.service.d/multi-asic-dependencies.conf";
317+
318+
if (IS_SINGLE_ASIC(cfg.num_asics) && cfg.num_dpus == 0) {
319+
/* Nothing in this section will apply to single asic, as the file
320+
* won't be created at all.
321+
*/
322+
validate_output_dependency_list(common_dependency_list,
323+
test_service, false, cfg.num_asics);
324+
return;
325+
}
306326

307327
/* Validate Unit file dependency creation for multi instance
308328
* services. These entries should be present for multi asic
@@ -317,21 +337,10 @@ class SsgMainTest : public SsgFunctionTest {
317337
* Despite the split, the final result remains equivalent.
318338
*/
319339
if (cfg.num_dpus > 0) {
320-
/* Validate Unit file dependency creation for single instance
321-
* services. These entries should not be present for multi asic
322-
* system but present for single asic system.
323-
*/
324-
validate_output_dependency_list(single_asic_dependency_list_split,
325-
test_service, IS_SINGLE_ASIC(cfg.num_asics), cfg.num_asics);
326-
validate_output_dependency_list(npu_dependency_list,
327-
"midplane-network-npu.service", true, cfg.num_dpus);
328-
} else {
329-
/* Validate Unit file dependency creation for single instance
330-
* services. These entries should not be present for multi asic
331-
* system but present for single asic system.
332-
*/
333-
validate_output_dependency_list(single_asic_dependency_list,
334-
test_service, IS_SINGLE_ASIC(cfg.num_asics), cfg.num_asics);
340+
for (int i = 0; i < cfg.num_dpus; i++) {
341+
validate_output_dependency_list_ignore_multi_instance(npu_dependency_list,
342+
"database@dpu" + std::to_string(i) + ".service.d/ordering.conf", true);
343+
}
335344
}
336345

337346
/* Validate Unit file dependency creation for single instance
@@ -355,8 +364,6 @@ class SsgMainTest : public SsgFunctionTest {
355364
test_target, true, cfg.num_asics);
356365
validate_output_unit_files(npu_service_list,
357366
test_target, cfg.is_smart_switch_npu, cfg.num_dpus);
358-
validate_output_unit_files(npu_network_service_list,
359-
"network", cfg.is_smart_switch_npu, cfg.num_dpus);
360367
validate_output_unit_files(dpu_service_list,
361368
test_target, cfg.is_smart_switch_dpu, cfg.num_dpus);
362369
validate_output_unit_files(dpu_network_service_list,
@@ -374,19 +381,26 @@ class SsgMainTest : public SsgFunctionTest {
374381

375382
checked_service_list.insert(checked_service_list.end(), common_service_list.begin(), common_service_list.end());
376383
if (cfg.num_dpus > 0) {
377-
checked_service_list.insert(checked_service_list.end(), npu_service_list.begin(), npu_service_list.end());
384+
checked_service_list.insert(checked_service_list.end(), npu_service_list_for_environment_variables.begin(), npu_service_list_for_environment_variables.end());
378385
}
379386
if (cfg.num_asics > 1) {
380387
checked_service_list.insert(checked_service_list.end(), multi_asic_service_list.begin(), multi_asic_service_list.end());
381388
}
382389

383390
for (const auto &target: checked_service_list) {
384-
if (find_string_in_file("[Service]", target)) {
385-
for (const auto& item : env_vars) {
386-
std::string str = "Environment=\"" + item.first + "=" + item.second + "\"";
387-
EXPECT_EQ(find_string_in_file(str, target), true)
388-
<< "Error validating " + str + " in " + target;
391+
if (!target.ends_with(".service")) {
392+
continue;
393+
}
394+
395+
for (const auto& item : env_vars) {
396+
std::string str = "Environment=\"" + item.first + "=" + item.second + "\"";
397+
auto target_unit = target;
398+
if (is_multi_instance(target)) {
399+
/* insert instance id in string */
400+
target_unit = (boost::format{target_unit} % "").str();
389401
}
402+
EXPECT_EQ(find_string_in_file(str, target_unit + ".d/environment.conf"), true)
403+
<< "Error validating " + str + " in " + target_unit;
390404
}
391405
}
392406
}
@@ -466,14 +480,6 @@ class SsgMainTest : public SsgFunctionTest {
466480
/* Save global variables before running tests */
467481
virtual void SetUp() {
468482
SsgFunctionTest::SetUp();
469-
// Create /dev/null symlink for simulation disabled service
470-
std::vector<std::string> disabled_service;
471-
disabled_service.insert(disabled_service.end(), npu_network_service_list.begin(), npu_network_service_list.end());
472-
disabled_service.insert(disabled_service.end(), dpu_network_service_list.begin(), dpu_network_service_list.end());
473-
for (const auto &service : disabled_service) {
474-
fs::create_symlink("/dev/null", TEST_ETC_NETWORK + service);
475-
}
476-
fs::create_symlink("/dev/null", TEST_ETC_SYSTEM + "systemd-networkd.service");
477483
}
478484

479485
/* Restore global vars */
@@ -488,7 +494,7 @@ class SsgMainTest : public SsgFunctionTest {
488494
static const std::vector<std::string> common_service_list;
489495
static const std::vector<std::string> non_smart_switch_service_list;
490496
static const std::vector<std::string> npu_service_list;
491-
static const std::vector<std::string> npu_network_service_list;
497+
static const std::vector<std::string> npu_service_list_for_environment_variables;
492498
static const std::vector<std::string> dpu_service_list;
493499
static const std::vector<std::string> dpu_network_service_list;
494500
static const std::vector<std::string> single_asic_dependency_list;
@@ -541,17 +547,11 @@ SsgMainTest::non_smart_switch_service_list = {
541547
const std::vector<std::string>
542548
SsgMainTest::npu_service_list = {
543549
"database@dpu%1%.service",
544-
"midplane-network-npu.service",
545550
};
546551

547-
/* Systemd service Unit file list for Smart Switch NPU. */
548552
const std::vector<std::string>
549-
SsgMainTest::npu_network_service_list = {
550-
"bridge-midplane.netdev",
551-
"bridge-midplane.network",
552-
"dummy-midplane.netdev",
553-
"dummy-midplane.network",
554-
"midplane-network-npu.network",
553+
SsgMainTest::npu_service_list_for_environment_variables = {
554+
"database@%1%.service",
555555
};
556556

557557
/* Systemd service Unit file list for Smart Switch DPU. */
@@ -609,7 +609,8 @@ SsgMainTest::common_dependency_list = {
609609

610610
const std::vector<std::string>
611611
SsgMainTest::npu_dependency_list = {
612-
"Before=database@dpu%1%.service",
612+
"Requires=systemd-networkd-wait-online@bridge-midplane.service",
613+
"After=systemd-networkd-wait-online@bridge-midplane.service",
613614
};
614615

615616
/* Test get functions for global vasr*/

0 commit comments

Comments
 (0)