Skip to content

introduce Broadcom legacy image for 7060cx#25192

Merged
StormLiangMS merged 15 commits intosonic-net:masterfrom
lipxu:20260126_publicMaster_sai
Mar 3, 2026
Merged

introduce Broadcom legacy image for 7060cx#25192
StormLiangMS merged 15 commits intosonic-net:masterfrom
lipxu:20260126_publicMaster_sai

Conversation

@lipxu
Copy link
Contributor

@lipxu lipxu commented Jan 26, 2026

Why I did it

Broadcom would still support TH SAI with current version, but no upgrade plan.

Work item tracking
  • Microsoft ADO (number only):
    36193015

How I did it

Introduce the Broadcom legacy image for 7060cx with the latest SONiC code and old SAI binary.

How to verify it

https://elastictest.org/scheduler/testplan/697ecfc0d7617b89998e1669
https://elastictest.org/scheduler/testplan/697ecfc0d7617b89998e1663

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

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

lipxu added 7 commits January 26, 2026 12:29
… version

Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
…est script

Fix build failure caused by missing execute permission.
The platform_asic_checker script is executed during build but had
644 permissions instead of 755.

Error: /bin/bash: line 15: ./platform_asic_checker: Permission denied
Signed-off-by: Liping Xu <xuliping@microsoft.com>
Signed-off-by: Liping Xu <xuliping@microsoft.com>
@lipxu lipxu force-pushed the 20260126_publicMaster_sai branch from 27534e0 to b266db4 Compare January 26, 2026 12:30
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

lipxu added 2 commits January 30, 2026 01:46
Update build system and kernel API compatibility for Trixie kernel 6.12:

Build System Changes (debian/rules):
- Dynamic kernel version detection instead of hardcoded 6.1.0-29-2-amd64
- Use KVER_ARCH and KVER_COMMON variables for flexible kernel version handling
- Add -f flags to rm/ln/cp commands for idempotent builds

Kernel API Compatibility:
- MAX_ORDER -> MAX_PAGE_ORDER (kernel 6.5+)
- PCI_IRQ_LEGACY -> PCI_IRQ_INTX (kernel 6.8+)
- strlcpy() -> strscpy() (kernel 6.12+)
- platform_driver.remove: int -> void return type (kernel 6.11+)
- ethtool_ts_info -> kernel_ethtool_ts_info (kernel 6.11+)

Compiler Flags:
- Add -Wno-declaration-after-statement, -Wno-missing-prototypes,
  -Wno-array-bounds, -fcf-protection for GCC 14 compatibility

Signed-off-by: Liping Xu <xuliping@microsoft.com>
Update kernel version references in debian packaging files:
- debian/control: Depend on linux-image-6.12.41+deb13-sonic-amd64-unsigned
- debian/opennsl-modules.install: Install modules to lib/modules/6.12.41+deb13-sonic-amd64/extra
- debian/opennsl-modules.dirs: Create directory for 6.12.41+deb13-sonic-amd64

Without these fixes:
- Package fails dependency check and gets deinstalled by dpkg
- Kernel modules are installed to wrong directory
- opennsl-modules.service fails to load modules, causing syncd/swss boot failure

Signed-off-by: Liping Xu <xuliping@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

lipxu added 2 commits January 30, 2026 03:08
Remove unnecessary -f flags from rm, ln, and cp commands in build-arch
section to be consistent with saibcm-modules (XGS).

Signed-off-by: Liping Xu <xuliping@microsoft.com>
Add nosemgrep inline comments to suppress Semgrep false positive for
the strlcpy compatibility macro '#define strscpy strlcpy'.

This macro is only used for backward compatibility with kernels < 4.19
and is never compiled on Trixie (kernel 6.12). strlcpy is bounds-checked
and safe; it was removed from kernel 6.12 in favor of strscpy, not due
to security issues.

Signed-off-by: Liping Xu <xuliping@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Add nosemgrep inline comment to suppress Semgrep false positive for
strncpy usage in ngknet_linux.h sal_strncpy function.

This is a SAL (System Abstraction Layer) compatibility wrapper that
provides platform-independent string operations. The strncpy usage
is intentional for backward compatibility and has proper bounds checking.

Signed-off-by: Liping Xu <xuliping@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Fix build failure: 'supervisor-proc-exit-listener': not found

Changes:
1. Remove invalid COPY line for supervisor-proc-exit-listener (file doesn't exist)
2. Fix variable name: docker_syncd_brcm_debs -> docker_syncd_brcm_legacy_debs
3. Update to multi-stage build format to match docker-syncd-brcm
4. Use copy_files macro instead of manual COPY loop

This aligns the legacy Dockerfile with the XGS variant structure.

Signed-off-by: Liping Xu <xuliping@microsoft.com>
@lipxu lipxu force-pushed the 20260126_publicMaster_sai branch from ecce860 to 1cab69d Compare January 30, 2026 06:46
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lipxu lipxu marked this pull request as ready for review February 4, 2026 00:18
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Liping Xu <xuliping@microsoft.com>
Copilot AI review requested due to automatic review settings February 26, 2026 03:10
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 90 out of 585 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (5)

platform/broadcom/saibcm-modules-legacy/sdklt/Makefile:30

  • Typo in help text: CROSS_COPILE should be CROSS_COMPILE to match the exported variable name and common kernel build conventions.
	@echo 'CROSS_COPILE  - Cross-compiler prefix (optional)'

platform/broadcom/saibcm-modules-legacy/debian/control:13

  • Hardcoding an exact kernel-image dependency makes this package brittle across kernel updates. For kernel module packages, it’s usually better to depend on a virtual/kernel ABI–specific package name (or use debhelper mechanisms to inject the correct dependency), so the package can be rebuilt/consumed across supported kernel revisions without editing control metadata.
Depends: linux-image-6.12.41+deb13-sonic-amd64-unsigned

platform/broadcom/saibcm-modules-legacy/GLOG:2

  • This file appears to be a generated log and includes absolute internal build paths. It’s best not to commit build logs that can leak environment details. Suggest removing GLOG from the repo and adding it to .gitignore (and/or ensuring it’s not produced in the source tree during builds).
CMD = [/projects/csg_sonic2/sk411346/repos/SAI/6.5.29/ocp_sai/sdk-src/hsdk-6.5.29/xgs-sdk-6.5.29-gpl-modules/, gtags --single-update systems/linux/kernel/modules/bcm-knet/bcm-knet.c]
CMD = [/projects/csg_sonic2/sk411346/repos/SAI/6.5.29/ocp_sai/sdk-src/hsdk-6.5.29/xgs-sdk-6.5.29-gpl-modules/, gtags --single-update systems/linux/kernel/modules/knet-cb/knet-cb.c]

platform/broadcom/docker-syncd-brcm-legacy/bcmsh:7

  • Typo in usage text: quite should be quiet.
    -q  quite, no banner (default: verbose)"

platform/broadcom/docker-syncd-brcm-legacy/start.sh:18

  • Typo in comment: cacse should be case.
# There is also a possibility that both sai.profile.j2 and config.bcm.j2 are absent.  in that cacse just copy

Comment on lines +90 to +100
# create links
cd /; sudo mkdir -p /lib/modules/$(KVER_ARCH)
cd /; sudo rm /lib/modules/$(KVER_ARCH)/build
cd /; sudo rm /lib/modules/$(KVER_ARCH)/source
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_COMMON)/ /lib/modules/$(KVER_ARCH)/source
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_ARCH)/ /lib/modules/$(KVER_ARCH)/build
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_ARCH)/include/generated/ /usr/src/linux-headers-$(KVER_COMMON)/include/generated
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_ARCH)/arch/x86/include/generated/ /usr/src/linux-headers-$(KVER_COMMON)/arch/x86/include/generated
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_ARCH)/arch/x86/module.lds /usr/src/linux-headers-$(KVER_COMMON)/arch/x86/module.lds
cd /; sudo ln -s /usr/src/linux-headers-$(KVER_ARCH)/include/config/ /usr/src/linux-headers-$(KVER_COMMON)/include/config
cd /; sudo cp /usr/src/linux-headers-$(KVER_ARCH)/Module.symvers /usr/src/linux-headers-$(KVER_COMMON)/Module.symvers
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The Debian package build uses sudo and mutates host paths under /lib/modules and /usr/src. Package builds (including in CI/build containers) are typically non-root and must not alter the build host. Instead, drive the build by passing the correct kernel header paths (e.g., via KDIR/KERNDIR/KERNEL_SRC) without creating global symlinks, or confine any necessary staging under the package build directory (e.g., debian/tmp).

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +8
systems/linux/user/x86-smp_generic_64-2_6/linux-bcm-knet.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-kernel-bde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-user-bde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-knet-cb.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systemd/opennsl-modules.service lib/systemd/system
sdklt/build/bde/linux_ngbde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
sdklt/build/knet/linux_ngknet.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
sdklt/build/knetcb/linux_ngknetcb.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The install destinations hardcode a specific kernel version (6.12.41+deb13-sonic-amd64). This will break as soon as the kernel version changes. Consider generating this .install file during the build from KVERSION/KVERS/KVER_ARCH in debian/rules, or installing into a staging path and letting postinst/depmod place modules under the running kernel version.

Suggested change
systems/linux/user/x86-smp_generic_64-2_6/linux-bcm-knet.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-kernel-bde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-user-bde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-knet-cb.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systemd/opennsl-modules.service lib/systemd/system
sdklt/build/bde/linux_ngbde.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
sdklt/build/knet/linux_ngknet.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
sdklt/build/knetcb/linux_ngknetcb.ko lib/modules/6.12.41+deb13-sonic-amd64/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-bcm-knet.ko lib/modules/*/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-kernel-bde.ko lib/modules/*/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-user-bde.ko lib/modules/*/extra
systems/linux/user/x86-smp_generic_64-2_6/linux-knet-cb.ko lib/modules/*/extra
systemd/opennsl-modules.service lib/systemd/system
sdklt/build/bde/linux_ngbde.ko lib/modules/*/extra
sdklt/build/knet/linux_ngknet.ko lib/modules/*/extra
sdklt/build/knetcb/linux_ngknetcb.ko lib/modules/*/extra

Copilot uses AI. Check for mistakes.
StormLiangMS

This comment was marked as duplicate.

Copy link
Contributor

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

@lipxu Review comments:

Blockers:

  1. *Hardcoded kernel version in \debian/opennsl-modules.install* — Install paths reference \lib/modules/6.12.41+deb13-sonic-amd64/extra. This will break on any kernel version change. Should be generated from \KVERSION\ at build time.

  2. *\sudo\ in \debian/rules* — Package build uses \sudo\ to mutate host paths under /lib/modules\ and /usr/src. This is unsafe in CI and may fail in non-root build containers. Should use \KDIR/\KERNDIR\ to pass kernel header paths instead.

Question:

  1. ~580 vendored source files — The entire \saibcm-modules-legacy/\ kernel module source tree is checked into the repo. Is there a submodule or package alternative to avoid carrying this in-tree? If vendoring is the only option, please confirm in the PR description.

@lipxu
Copy link
Contributor Author

lipxu commented Feb 27, 2026

@lipxu Review comments:

Blockers:

  1. _Hardcoded kernel version in \debian/opennsl-modules.install_ — Install paths reference \lib/modules/6.12.41+deb13-sonic-amd64/extra. This will break on any kernel version change. Should be generated from \KVERSION\ at build time.
  2. _\sudo\ in \debian/rules_ — Package build uses \sudo\ to mutate host paths under /lib/modules\ and /usr/src. This is unsafe in CI and may fail in non-root build containers. Should use \KDIR/\KERNDIR\ to pass kernel header paths instead.

Question:

  1. ~580 vendored source files — The entire \saibcm-modules-legacy/\ kernel module source tree is checked into the repo. Is there a submodule or package alternative to avoid carrying this in-tree? If vendoring is the only option, please confirm in the PR description.

Thank you for your detailed review @StormLiangMS ! I'd like to clarify the scope:

Blockers:

1. Hardcoded kernel version in debian/opennsl-modules.install
2. sudo in debian/rules

Both of these are pre-existing issues that have existed in the codebase since the original implementation and are currently present in:

  • platform/broadcom/saibcm-modules/debian/ (XGS family - in production)
  • platform/broadcom/saibcm-modules-dnx/debian/ (DNX family - in production)

Key points:

  • These patterns were not introduced by this PR
  • These modules have been building and running successfully in production for a long time without issues reported
  • This PR maintains consistency with the established codebase patterns

Risk Assessment:
Given that both existing modules work correctly with these patterns, the risk is minimal.

Recommendation:

  1. Merge this PR - It follows established patterns and adds Legacy ASIC support without introducing new technical debt
  2. Create a follow-up task - Address these architectural concerns across all three modules (saibcm-modules, saibcm-modules-dnx, and saibcm-modules-legacy) in a unified refactoring effort

This approach ensures we don't block the Legacy ASIC feature while acknowledging the technical debt can be improved across the board.

Question:

~665 vendored source files

The saibcm-modules-legacy source tree is currently vendored directly (not a submodule), consistent with the existing saibcm-modules module.

Future plan:
We plan to follow the saibcm-modules-dnx pattern and convert this to a git submodule in the future. However, this refactoring would:

  • Require significant effort and a long time to set up a separate repository
  • Need thorough validation across all xgs and legacy ASIC platforms

Recommendation:
We prefer to merge the current implementation now (matching the established saibcm-modules pattern) to unblock Legacy ASIC support, then improve it in a follow-up effort.

Copy link
Contributor

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Gfrom2016 Gfrom2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@lipxu
Copy link
Contributor Author

lipxu commented Feb 27, 2026

Hi @lipxu what do you think about adding a -th suffix throughout the new "legacy" naming introduced in this PR? For example docker-syncd-brcm-legacy -> docker-syncd-brcm-legacy-th
This should reduce confusion in future when there is a second brcm platform that is EOL which would become another but different legacy version.

Thanks for your suggestion. The image was originally named legacy rather than legacy-th because it was intended for a special-case scenario and was expected to be used rarely. We may consider renaming it to legacy-th if we think a similar solution is needed for other platforms in the future.

Hi, @Ryangwaite ,
I agree with your suggestion and plan to use legacy-th instead of legacy.
However, this change will affect many files and require rebuilding the image and running verification tests, which may take some additional time.
Would it be possible to merge this PR first? I can then create a follow-up PR to handle the renaming.
Thanks a lot for your understanding and support.

@lipxu lipxu requested a review from davidm-arista March 1, 2026 23:38
@Ryangwaite
Copy link
Contributor

Hi @lipxu what do you think about adding a -th suffix throughout the new "legacy" naming introduced in this PR? For example docker-syncd-brcm-legacy -> docker-syncd-brcm-legacy-th
This should reduce confusion in future when there is a second brcm platform that is EOL which would become another but different legacy version.

Thanks for your suggestion. The image was originally named legacy rather than legacy-th because it was intended for a special-case scenario and was expected to be used rarely. We may consider renaming it to legacy-th if we think a similar solution is needed for other platforms in the future.

Hi, @Ryangwaite , I agree with your suggestion and plan to use legacy-th instead of legacy. However, this change will affect many files and require rebuilding the image and running verification tests, which may take some additional time. Would it be possible to merge this PR first? I can then create a follow-up PR to handle the renaming. Thanks a lot for your understanding and support.

Yeah, that's a good approach

Copy link
Contributor

@Ryangwaite Ryangwaite left a comment

Choose a reason for hiding this comment

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

LGTM!

@lipxu
Copy link
Contributor Author

lipxu commented Mar 2, 2026

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 25192 in repo sonic-net/sonic-buildimage

@lipxu
Copy link
Contributor Author

lipxu commented Mar 2, 2026

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lipxu
Copy link
Contributor Author

lipxu commented Mar 3, 2026

PR tests blocked, all PR test cases had been passed in previous run.
image

@StormLiangMS StormLiangMS merged commit d853bee into sonic-net:master Mar 3, 2026
16 of 26 checks passed
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202511: #25878

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants