[Mellanox] Add support for new "nvidia-bluefield" platform. by oleksandrivantsiv · Pull Request #18466 · sonic-net/sonic-buildimage

oleksandrivantsiv · 2024-03-26T00:59:45Z

Why I did it

Add support for new Nvidia-bluefield BF-3 DPU.

Work item tracking

Microsoft ADO (number only):

How I did it

Define a new nvidia-bluefield platform with a set of makefiles to compile an image with all dependencies. Impleme PMON platform API

How to verify it

The added changes don't affect the compilation of the existing platforms.

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

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)

lguohan · 2024-03-26T18:29:13Z

@prgeor , @saiarcot895 , can you help to review it?

saiarcot895 · 2024-03-27T17:20:57Z

platform/nvidia-bluefield/docker-syncd-bluefield-rpc/Dockerfile.j2

+ && cd /opt             \
+ && wget https://raw.githubusercontent.com/p4lang/ptf/master/ptf_nn/ptf_nn_agent.py \
+ && apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y \
+ && rm -rf /root/deps


I know that most of this came from another file (either Dockerfile or sonic-mgmt test case), but can this be replaced with steps to install nanomsg-utils or libnanomsg5 from the Debian repos instead?

Also, should this still be Python 2, or can it be changed to Python 3?

We don't have a use case for the PRC image right now. I removed all related files. When it will be needed we will submit it in a separate PR.

saiarcot895 · 2024-03-27T17:22:14Z

platform/nvidia-bluefield/docker-syncd-bluefield/Dockerfile.j2

+{% endif %}
+
+## Clean up
+RUN apt-get clean -y && \


Please remove python3-pip, python3-dev, and python3-setuptools.

saiarcot895 · 2024-03-27T17:54:26Z

platform/nvidia-bluefield/dpu-sai/Makefile

+	pushd mlnx_sai
+
+	# Build the package 
+	debuild -e 'make_extra_flags="DEFS=-DACS_OS -DCONFIG_SYSLOG"' -us -uc -b -j$(SONIC_CONFIG_MAKE_JOBS) --admindir $(SONIC_DPKG_ADMINDIR)


Cosnidering the rest of the repo uses dpkg-buildpackage, can that be used here instead of debuild?

Do you see any issues with the debuild utility? All the makefiles we have here are well-tested. Any change to the make files will require us to run the regression and qualify the changes.

No specific issues, just consistency.

saiarcot895 · 2024-03-27T17:56:30Z

platform/nvidia-bluefield/mft/Makefile

+	# put a lock here because dpkg does not allow installing packages in parallel
+	while true; do
+	if mkdir $(DEST)/dpkg_lock &> /dev/null; then
+	{ sudo dpkg -i $(SRC_DEB) && rm -d $(DEST)/dpkg_lock && break; } || { rm -d $(DEST)/dpkg_lock && exit 1 ; }


If these packages need to be installed as build dependencies, can they be specified as such via the approprate rules.mk file, instead of installing them here?

It is impossible to do via rules.mk because the package installed here is a part of the archive we download. Theoretically, we can split this into two targets and create dependencies between them but this will require downloading the archive twice.

What about having a Makefile that only downloads the archive, extracts the packages, and then copies it to target/debs/...? rules.mk would call that Makefile and then expects the packages to get installed.

This is a good idea. I updated the implementation

saiarcot895 · 2024-03-27T17:57:51Z

platform/nvidia-bluefield/sdk-src/flexio/Makefile

+	patch -p1 < ../0001-Remove-meson-from-dependencies.patch
+
+	# Build the Debs
+	PATH=/usr/bin/:$(PATH) dpkg-buildpackage -us -uc -b -j$(SONIC_CONFIG_MAKE_JOBS) --admindir $(SONIC_DPKG_ADMINDIR)


Why the PATH change here?

In this Makefile it is not needed. Removed

saiarcot895 · 2024-03-27T18:01:04Z

platform/nvidia-bluefield/sdk-src/ofed/Makefile

+	pushd $(OFED_SRC)/
+	find . -type f -exec touch {} +
+
+	sudo mv /bin/uname /bin/uname.orig


If this needs to be done, I would highly recommend instead having this in some manually-created directory, and updating PATH so that that directory is checked first. (Ideally, any package build should allow for specifying the kernel version via an argument or an environment variable, as is usually done for external kernel module builds.)

I reworked the Makefile. Now this is removed.

oleksandrivantsiv · 2024-04-10T20:47:33Z

@saiarcot895 can you please check this PR?

saiarcot895 · 2024-04-11T22:38:12Z

platform/nvidia-bluefield/platform-api/sonic_platform/utils.py

FYI, Python 3.9 and newer have this functionality in the stdlib: https://docs.python.org/3/library/functools.html#functools.cache

saiarcot895 · 2024-04-11T22:47:33Z

@prgeor for review of platform APIs

liat-grozovik

please check the comment on all new files.

liat-grozovik · 2024-04-14T13:27:13Z

platform/nvidia-bluefield/bfscripts/Makefile

new file cannot have such header.

liat-grozovik · 2024-04-14T13:27:31Z

platform/nvidia-bluefield/bluefield-platform-modules/Makefile

new file cannot have such header

Signed-off-by: Oleksandr Ivantsiv <[email protected]> Co-authored-by: Vivek Reddy <[email protected]> Co-authored-by: Yakiv Huryk <[email protected]>

prgeor · 2024-04-22T21:44:45Z

I was indeed late to review this PR...@liat-grozovik I didn't understand the urgency in merging this PR

oleksandrivantsiv requested a review from lguohan as a code owner March 26, 2024 00:59

oleksandrivantsiv mentioned this pull request Mar 26, 2024

[nvidia-bluefield] Add support for Nvidia-9009d3b600CVAA and Nvidia-9009d3b600SVAA BF-3 DPUs. #18467

Merged

11 tasks

saiarcot895 reviewed Mar 27, 2024

View reviewed changes

oleksandrivantsiv force-pushed the nvidia-bluefield-platform branch from dc2b7dc to 60630b8 Compare April 9, 2024 21:20

oleksandrivantsiv requested review from qiluo-msft and xumia as code owners April 9, 2024 21:20

oleksandrivantsiv force-pushed the nvidia-bluefield-platform branch from 60630b8 to a0871d1 Compare April 10, 2024 20:42

oleksandrivantsiv mentioned this pull request Apr 10, 2024

[nvidia-bluefield] Add dependencies required for Nvidia-bluefield image compilation. #18649

Merged

11 tasks

saiarcot895 reviewed Apr 11, 2024

View reviewed changes

saiarcot895 approved these changes Apr 11, 2024

View reviewed changes

liat-grozovik requested changes Apr 14, 2024

View reviewed changes

liat-grozovik changed the title ~~[nvidia-bluefield] Add support for new "nvidia-bluefield" platform.~~ [Mellanox] Add support for new "nvidia-bluefield" platform. Apr 14, 2024

liat-grozovik added the Platform: Mellanox label Apr 14, 2024

[nvidia-bluefield] Add support for new "nvidia-bluefield" platform.

1f6e45c

Signed-off-by: Oleksandr Ivantsiv <[email protected]> Co-authored-by: Vivek Reddy <[email protected]> Co-authored-by: Yakiv Huryk <[email protected]>

oleksandrivantsiv force-pushed the nvidia-bluefield-platform branch from a0871d1 to 1f6e45c Compare April 16, 2024 00:55

liat-grozovik approved these changes Apr 16, 2024

View reviewed changes

liat-grozovik merged commit e7cf8cf into sonic-net:master Apr 16, 2024

Conversation

oleksandrivantsiv commented Mar 26, 2024

Why I did it

Work item tracking

How I did it

How to verify it

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

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)

Uh oh!

lguohan commented Mar 26, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleksandrivantsiv commented Apr 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saiarcot895 commented Apr 11, 2024

Uh oh!

liat-grozovik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prgeor commented Apr 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants