From bca8a244c6ef3e58067f71a91da4c1334d7176f4 Mon Sep 17 00:00:00 2001 From: shlomibitton <60430976+shlomibitton@users.noreply.github.com> Date: Sun, 15 May 2022 15:05:29 +0300 Subject: [PATCH] [202012] [Fastboot] Delay LLDP service for better fastboot performance (#10568) (#10744) This PR is to backport a fix #10568 This PR is dependent on PR: #10745 - Why I did it Profiling the system state on init after fast-reboot during create_switch function execution, it is possible to see few python scripts running at the same time. This parallel execution consume CPU time and the duration of create_switch is longer than it should be. Following this finding, and the motivation to ensure these services will not interfere in the future, LLDP is delayed in 90 seconds until the system finish the init flow after fastboot. - How I did it Add a timer for LLDP service. Copy the timer file to the host bin image. - How to verify it Run fast-reboot on MLNX platform and observe faster create_switch execution time. --- files/build_templates/init_cfg.json.j2 | 2 +- files/build_templates/lldp.timer.j2 | 1 + .../per_namespace/lldp.service.j2 | 3 -- .../per_namespace/lldp.timer.j2 | 12 +++++++ .../build_templates/sonic_debian_extension.j2 | 13 ++++++++ files/scripts/syncd.sh | 13 ++++++-- slave.mk | 31 +++++++++++++++++++ 7 files changed, 68 insertions(+), 7 deletions(-) create mode 120000 files/build_templates/lldp.timer.j2 create mode 100644 files/build_templates/per_namespace/lldp.timer.j2 diff --git a/files/build_templates/init_cfg.json.j2 b/files/build_templates/init_cfg.json.j2 index 2b21f270c0..b0e6307711 100644 --- a/files/build_templates/init_cfg.json.j2 +++ b/files/build_templates/init_cfg.json.j2 @@ -21,7 +21,7 @@ {%- set features = [("bgp", "enabled", false, "enabled"), ("database", "always_enabled", false, "always_enabled"), ("dhcp_relay", "enabled", false, "enabled"), - ("lldp", "enabled", false, "enabled"), + ("lldp", "enabled", true, "enabled"), ("pmon", "enabled", false, "enabled"), ("radv", "enabled", false, "enabled"), ("snmp", "enabled", true, "enabled"), diff --git a/files/build_templates/lldp.timer.j2 b/files/build_templates/lldp.timer.j2 new file mode 120000 index 0000000000..8e6950d6f3 --- /dev/null +++ b/files/build_templates/lldp.timer.j2 @@ -0,0 +1 @@ +per_namespace/lldp.timer.j2 \ No newline at end of file diff --git a/files/build_templates/per_namespace/lldp.service.j2 b/files/build_templates/per_namespace/lldp.service.j2 index fca8446293..080f4a825b 100644 --- a/files/build_templates/per_namespace/lldp.service.j2 +++ b/files/build_templates/per_namespace/lldp.service.j2 @@ -21,6 +21,3 @@ ExecStart=/usr/bin/{{docker_container_name}}.sh wait{% if multi_instance == 'tru ExecStop=/usr/bin/{{docker_container_name}}.sh stop{% if multi_instance == 'true' %} %i{% endif %} Restart=always RestartSec=30 - -[Install] -WantedBy=sonic.target diff --git a/files/build_templates/per_namespace/lldp.timer.j2 b/files/build_templates/per_namespace/lldp.timer.j2 new file mode 100644 index 0000000000..67622a3285 --- /dev/null +++ b/files/build_templates/per_namespace/lldp.timer.j2 @@ -0,0 +1,12 @@ +[Unit] +# This delay is for fast/warm reboot performance +Description=Delays LLDP docker until SONiC has started +PartOf=lldp{% if multi_instance == 'true' %}@%i{% endif %}.service + +[Timer] +OnUnitActiveSec=0 sec +OnBootSec=1min 30 sec +Unit=lldp{% if multi_instance == 'true' %}@%i{% endif %}.service + +[Install] +WantedBy=timers.target sonic.target sonic-delayed.target diff --git a/files/build_templates/sonic_debian_extension.j2 b/files/build_templates/sonic_debian_extension.j2 index d42d60398e..95fcf4e24c 100644 --- a/files/build_templates/sonic_debian_extension.j2 +++ b/files/build_templates/sonic_debian_extension.j2 @@ -700,6 +700,19 @@ if [ -f {{service}} ]; then echo "{{service}}" | sudo tee -a $GENERATED_SERVICE_FILE fi {% endfor %} +{% for timer in installer_timers.split(' ') -%} +if [ -f {{timer}} ]; then + sudo cp {{timer}} $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM + + {% if "@" in timer %} + MULTI_INSTANCE="{{timer}}" + SINGLE_INSTANCE=${MULTI_INSTANCE/"@"} + sudo cp $SINGLE_INSTANCE $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM + {% endif %} + + echo "{{timer}}" | sudo tee -a $GENERATED_SERVICE_FILE +fi +{% endfor %} if [ -f iccpd.service ]; then sudo LANG=C chroot $FILESYSTEM_ROOT systemctl disable iccpd.service fi diff --git a/files/scripts/syncd.sh b/files/scripts/syncd.sh index 95c3e3841b..9f7418bbed 100755 --- a/files/scripts/syncd.sh +++ b/files/scripts/syncd.sh @@ -36,12 +36,19 @@ function startplatform() { } function waitplatform() { - + if [[ x"$sonic_asic_platform" == x"mellanox" ]]; then debug "Starting pmon service..." /bin/systemctl start pmon debug "Started pmon service" fi + if [[ x"$BOOT_TYPE" = @(x"fast"|x"warm"|x"fastfast") ]]; then + debug "LLDP service is delayed by a timer for better fast/warm boot performance" + else + debug "Starting lldp service..." + /bin/systemctl start lldp + debug "Started lldp service" + fi } function stopplatform1() { @@ -56,7 +63,7 @@ function stopplatform1() { debug "${TYPE} shutdown syncd process ..." /usr/bin/docker exec -i syncd$DEV /usr/bin/syncd_request_shutdown --${TYPE} - # wait until syncd quits gracefully or force syncd to exit after + # wait until syncd quits gracefully or force syncd to exit after # waiting for 20 seconds start_in_secs=${SECONDS} end_in_secs=${SECONDS} @@ -68,7 +75,7 @@ function stopplatform1() { done if [[ $((end_in_secs - start_in_secs)) -gt $timer_threshold ]]; then - debug "syncd process in container syncd$DEV did not exit gracefully" + debug "syncd process in container syncd$DEV did not exit gracefully" fi /usr/bin/docker exec -i syncd$DEV /bin/sync diff --git a/slave.mk b/slave.mk index 60253d53bf..bda703bc9c 100644 --- a/slave.mk +++ b/slave.mk @@ -975,6 +975,14 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \ $(eval $(docker:-dbg.gz=.gz)_GLOBAL = yes) ) fi + if [ -f files/build_templates/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 ]; then + j2 files/build_templates/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 > $($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer + + # Set the flag GLOBAL_TIMER for all the global system-wide dockers timers. + $(if $(shell ls files/build_templates/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 2>/dev/null),\ + $(eval $(docker:-dbg.gz=.gz)_GLOBAL_TIMER = yes) + ) + fi # Any service template, inside instance directory, will be used to generate .service and @.service file. if [ -f files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).service.j2 ]; then export multi_instance="true" @@ -985,6 +993,16 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \ export multi_instance="false" j2 files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).service.j2 > $($(docker:-dbg.gz=.gz)_CONTAINER_NAME).service fi + # Any timer template, inside instance directory, will be used to generate .timer and @.timer file. + if [ -f files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 ]; then + export multi_instance="true" + j2 files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 > $($(docker:-dbg.gz=.gz)_CONTAINER_NAME)@.timer + $(if $(shell ls files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 2>/dev/null),\ + $(eval $(docker:-dbg.gz=.gz)_TEMPLATE_TIMER = yes) + ) + export multi_instance="false" + j2 files/build_templates/per_namespace/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer.j2 > $($(docker:-dbg.gz=.gz)_CONTAINER_NAME).timer + fi # Any service template, inside share_image directory, will be used to generate -chassis.service file. # TODO: need better way to name the image-shared service if [ -f files/build_templates/share_image/$($(docker:-dbg.gz=.gz)_CONTAINER_NAME).service.j2 ]; then @@ -1017,7 +1035,20 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \ $(eval SERVICES += "$(addsuffix -chassis.service, $($(docker:-dbg.gz=.gz)_CONTAINER_NAME))") ) ) + # Marks template timers with an "@" according to systemd convention + # If the $($docker)_TEMPLATE_TIMER) variable is set, the timer will be treated as a template + # If the $($docker)_GLOBAL_TIMER) and $($docker)_TEMPLATE_TIMER) variables are set the timer will be added both as a global and template timer. + $(foreach docker, $($*_DOCKERS),\ + $(if $($(docker:-dbg.gz=.gz)_TEMPLATE_TIMER),\ + $(if $($(docker:-dbg.gz=.gz)_GLOBAL_TIMER),\ + $(eval TIMERS += "$(addsuffix .timer, $($(docker:-dbg.gz=.gz)_CONTAINER_NAME))")\ + )\ + $(eval TIMERS += "$(addsuffix @.timer, $($(docker:-dbg.gz=.gz)_CONTAINER_NAME))"),\ + $(eval TIMERS += "$(addsuffix .timer, $($(docker:-dbg.gz=.gz)_CONTAINER_NAME))") + ) + ) export installer_services="$(SERVICES)" + export installer_timers="$(TIMERS)" export installer_extra_files="$(foreach docker, $($*_DOCKERS), $(foreach file, $($(docker:-dbg.gz=.gz)_BASE_IMAGE_FILES), $($(docker:-dbg.gz=.gz)_PATH)/base_image_files/$(file)))"