Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[multi-asic] Fixed systemd-sonic-generator for multi-asic #7954

Merged
merged 1 commit into from
Jul 8, 2021
Merged

[multi-asic] Fixed systemd-sonic-generator for multi-asic #7954

merged 1 commit into from
Jul 8, 2021

Conversation

anamehra
Copy link
Contributor

Signed-off-by: Anand Mehra anamehra@cisco.com

Why I did it

  1. systemd-sonic-generator limits multi-asic unit file instances to 10 (single digit instance number 0 - 10). This limitation needs to be removed to handle more than 10 asics.
  2. MAX_NUM_TARGETS and MAX_NUM_INSTALL_LINES limits to 15 which is not sufficient for systems with more than 15 asics.
  3. Inside get_unit_files(), strcmp produce incorrect results due to non null terminated string being compared.

How I did it

  1. In function insert_instance_number instance_string was malloced for 2 char
    size which was limiting the instance number value in instance_name to 1 digit.
    Fixed insert_instance_number to use asprintf to generate instancd_name for
    any number of instances. Added _GNU_SOURCE to CFLAGS for asprintf.

  2. Fixed get_unit_files() to use calloc instead of malloc. Uninitialized memory
    was causing incorrect string mismatch error while comparing unit file name
    string.

  3. Increased MAX_NUM_TARGETS and MAX_NUM_INSTALL_LINES values to 48 to handle more
    asic instances.

  4. Added build UT support for systemd-sonic-generator:
    a. Refactor systemd-sonic-generator.c to be used with UT infra.
    b. Added UT infra to run build UT for systemd-sonic-generator
    c. Added functional level and program level UT class and test cases.

How to verify it

On a single asic system:

Verify Unit file symlinks at following path for single instance system:
/run/systemd/generator/multi-user.target.wants/
/run/systemd/generator/sonic.target.wants/
/run/systemd/generator/swss.service.wants/
On a multi asic system:

Verify Unit file symlinks at following path for multi instance system:
/run/systemd/generator/multi-user.target.wants/
/run/systemd/generator/sonic.target.wants/
/run/systemd/generator/swss@n.service.wants/ where n is instance num 0 - (num_asic-1)

systemd-sonic-generator_1.0.0_armhf.deb.log
systemd-sonic-generator_1.0.0_amd64.deb.log

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

    1. In function insert_instance_number instance_string was malloced for 2 char
       size which was limiting the instance number value in instance_name to 1 digit.
       Fixed insert_instance_number to use asprintf to generate instancd_name for
       any number of instances. Added _GNU_SOURCE to CFLAGS for asprintf.

    2. Fixed get_unit_files() to use calloc instead of malloc. Uninitialized memory
       was causing incorrect string mismatch error while comparing unit file name
       string.

    3. Increased MAX_NUM_TARGETS and MAX_NUM_INSTALL_LINES values to 48 to handle more
       asic instances.

    4. Added build UT support for systemd-sonic-generator:
        a. Refactor systemd-sonic-generator.c to be used with UT infra.
        b. Added UT infra to run build UT for systemd-sonic-generator
        c. Added functional level and program level UT class and test cases.

Signed-off-by: Anand Mehra <anamehra@cisco.com>
@anamehra
Copy link
Contributor Author

@anshuv-mfst Is it possible to trigger an armhf build as well for this PR?

Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local compilation for armhf/arm64 arch is ok.

@judyjoseph
Copy link
Contributor

@lguohan - if I can go ahead and merge this PR, the local compilation for both armhf/arm64 arch is ok with the latest patch.

@anshuv-mfst
Copy link

@judyjoseph - could you please help with merge, thanks

@judyjoseph judyjoseph merged commit 3b30127 into sonic-net:master Jul 8, 2021
judyjoseph pushed a commit that referenced this pull request Jul 13, 2021
Why I did it

systemd-sonic-generator limits multi-asic unit file instances to 10 (single digit instance number 0 - 10). This limitation needs to be removed to handle more than 10 asics.
MAX_NUM_TARGETS and MAX_NUM_INSTALL_LINES limits to 15 which is not sufficient for systems with more than 15 asics.
Inside get_unit_files(), strcmp produce incorrect results due to non null terminated string being compared.

 Added build UT support for systemd-sonic-generator
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
…7954)

Why I did it

systemd-sonic-generator limits multi-asic unit file instances to 10 (single digit instance number 0 - 10). This limitation needs to be removed to handle more than 10 asics.
MAX_NUM_TARGETS and MAX_NUM_INSTALL_LINES limits to 15 which is not sufficient for systems with more than 15 asics.
Inside get_unit_files(), strcmp produce incorrect results due to non null terminated string being compared.

 Added build UT support for systemd-sonic-generator
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants