Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Mellanox] Change NVIDIA SDK build to cmake method #27

Closed
wants to merge 1 commit into from

Conversation

dprital
Copy link
Owner

@dprital dprital commented Mar 17, 2024

Why I did it

Up to now, building Mellanox SDK debian packages was done by "autotools" method.
Lately, Mellanox SDK team added the support to build by "cmake" method.
The benefit of using "cmake" is with decrease time of SDK build as "cmake" make it faster then "auto build".

When moving to "cmake" method, all SDK debians:

  • applibs
  • sx-complib
  • sx-example
  • sx-gen-utils
  • sx-scew
  • sxd-libs
  • python-sdk-api
  • sx-acl-helper
  • wjh-libs
  • sx-hash-calc
  • sx-obj-desc-lib
  • sxd-libs

will be replaced by single debian - sys_sdk.

Work item tracking
  • Microsoft ADO (number only):

How I did it

  • Update cmake version on sonic slave dockers (bookworm and bullseye)
  • Change SDK Makefiles, delete all Makefiles of removed debians and add new Makefile for building sys_sdk.
  • Change sdk.mk and sdk.dep acordingly.
  • sx-kernel package was remain since it is consumed by the host as well and I didn't want to consume sys_sdk in host.

How to verify it

  • Enter syncd docker and verify the list of SDK packages are sx-kernel and sys-sdk only
  • Run regression tests and verify there is no degradation
  • Install the debian that include dbg-symbols and verify there are debug symbols by setting relevant breakpoints
  • Test the build time when using "cmake" for building SDK packages, compare to "auto build"

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

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)

Comment on lines 427 to 428
RUN sudo apt-get update -y
RUN sudo apt-get install -y libnl-genl-3-dev

Choose a reason for hiding this comment

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

Why need libnl-genl-3-dev?

Copy link
Owner Author

Choose a reason for hiding this comment

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

not required. Verified it and removed. Also from "bullseye" slave docker

sonic-slave-bookworm/Dockerfile.j2 Show resolved Hide resolved
PACKAGE_NAME = sys_sdk

$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :

Choose a reason for hiding this comment

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

Remove empty line

Copy link
Owner Author

Choose a reason for hiding this comment

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

Was removed


$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :

# get sources

Choose a reason for hiding this comment

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

Fix comment indent

Copy link
Owner Author

@dprital dprital Mar 27, 2024

Choose a reason for hiding this comment

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

It is at the same place as previous SDK make files. Right above the treatment to get the sources so I don't see any issue with it.

@@ -19,7 +19,7 @@ export MLNX_SAI_VERSION MLNX_SAI_SOURCE_BASE_URL

MLNX_SAI = mlnx-sai_1.mlnx.$(MLNX_SAI_VERSION)_$(CONFIGURED_ARCH).deb
$(MLNX_SAI)_SRC_PATH = $(PLATFORM_PATH)/mlnx-sai
$(MLNX_SAI)_DEPENDS += $(MLNX_SDK_DEBS) $(LIBNL_ROUTE3_DEV)
$(MLNX_SAI)_DEPENDS += $(MLNX_SDK_DEBS) $(LIBNL3_DEV) $(LIBNL_GENL3_DEV) $(LIBNL_ROUTE3_DEV) $(LIBNL_NF3_DEV)

Choose a reason for hiding this comment

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

Why now new dependency needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Verified that this is redundant so it was removed


rm -rf builds/cmake

cmake -B builds/cmake -DPYTHON_INTERPRETERS=python3 -DSKIP_UNINSTALL_INSTALL_DEPENDENCY=on -DSKIP_DEPMOD_IN_INSTALL_PHASE=on -DUSE_API_TESTER=OFF -DUSE_KERNEL=OFF -DCMAKE_PROJECT_VERSION=$(MLNX_SDK_DEB_VERSION)

Choose a reason for hiding this comment

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

Does it account for SONIC_DEBUGGING_ON/SONIC_PROFILING_ON?

@dprital dprital force-pushed the master_migrate_sdk_build_to_cmake branch from 59d4baf to f469e92 Compare July 29, 2024 18:23
@dprital dprital closed this Jul 29, 2024
@dprital dprital reopened this Jul 29, 2024
sonic-slave-bookworm/Dockerfile.j2 Outdated Show resolved Hide resolved
sonic-slave-bullseye/Dockerfile.j2 Outdated Show resolved Hide resolved
sonic-slave-bookworm/Dockerfile.j2 Outdated Show resolved Hide resolved
@dprital dprital force-pushed the master_migrate_sdk_build_to_cmake branch from c5f7fc6 to c775e40 Compare August 4, 2024 11:39
@dprital dprital closed this Aug 4, 2024
dprital pushed a commit that referenced this pull request Sep 5, 2024
…tically (sonic-net#20083)

#### Why I did it
src/sonic-dash-api
```
* 4919d9e - (HEAD -> master, origin/master, origin/HEAD) Allow `underlay_sip` to coexist with `vnet` in Route table (#27) (4 hours ago) [Lawrence Lee]
```
#### How I did it
#### How to verify it
#### Description for the changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants