Skip to content

Commit d05a645

Browse files
saiarcot895tirupatihemanth
authored and
jasonbridges
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 8edb046 commit d05a645

10 files changed

Lines changed: 331 additions & 416 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
@@ -677,10 +677,16 @@ sudo cp $IMAGE_CONFIGS/config-chassisdb/config-chassisdb $FILESYSTEM_ROOT/usr/bi
677677
echo "config-chassisdb.service" | sudo tee -a $GENERATED_SERVICE_FILE
678678
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable config-chassisdb.service
679679

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

690696
# Disable smart switch unit by default, these units will be controlled by systemd-sonic-generator
691697
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/bridge-midplane.netdev
692-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/bridge-midplane.network
693698
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/dummy-midplane.netdev
694-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/dummy-midplane.network
695-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/midplane-network-npu.network
696-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/network/midplane-network-dpu.network
697-
echo "midplane-network-npu.service" | sudo tee -a $GENERATED_SERVICE_FILE
699+
#echo "midplane-network-npu.service" | sudo tee -a $GENERATED_SERVICE_FILE
698700
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable midplane-network-npu.service
699-
echo "midplane-network-dpu.service" | sudo tee -a $GENERATED_SERVICE_FILE
701+
#echo "midplane-network-dpu.service" | sudo tee -a $GENERATED_SERVICE_FILE
700702
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable midplane-network-dpu.service
701-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha.service
702-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu0.service
703-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu1.service
704-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu2.service
705-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu3.service
706-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu4.service
707-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu5.service
708-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu6.service
709-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/dash-ha@dpu7.service
703+
710704
# According to the issue: https://github.com/systemd/systemd/issues/19106, To disable ManageForeignRoutingPolicyRules to avoid the ip rules being deleted by systemd-networkd
711705
sudo sed -i 's/#ManageForeignRoutingPolicyRules=yes/ManageForeignRoutingPolicyRules=no/g' $FILESYSTEM_ROOT/etc/systemd/networkd.conf
706+
sudo mkdir $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd.service.d
707+
sudo cp $IMAGE_CONFIGS/midplane-network/systemd-networkd.override.conf $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd.service.d/override.conf
712708

713-
sudo ln -s /dev/null $FILESYSTEM_ROOT/etc/systemd/system/systemd-networkd.service
714-
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable systemd-networkd-wait-online.service
709+
# Copy systemd-networkd-wait-online override configuration
710+
sudo mkdir -p $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/systemd-networkd-wait-online.service.d
711+
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
712+
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable systemd-networkd-wait-online.service
715713

716714
# Copy backend-acl script and service file
717715
sudo cp $IMAGE_CONFIGS/backend_acl/backend-acl.service $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM/backend-acl.service
@@ -826,6 +824,11 @@ sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable fstrim.timer
826824
## copy platform rc.local
827825
sudo cp $IMAGE_CONFIGS/platform/rc.local $FILESYSTEM_ROOT/etc/
828826

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

@@ -1227,3 +1230,5 @@ sudo touch $FILESYSTEM_ROOT_ETC_SONIC/enable_multidb
12271230

12281231
# Install syslog counter plugin
12291232
install_deb_package $debs_path/syslog-counter_*.deb
1233+
1234+

‎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/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)