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-simx] add ability to build simx-compatiable image #2664

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

mykolaf
Copy link
Collaborator

@mykolaf mykolaf commented Mar 12, 2019

Signed-off-by: Mykola Faryma [email protected]

- What I did
Add ability to build an image for Mellanox SimX.

Providing a way to build a SimX image.

Currently a few things need to be changed in the image to make it compatiable with SimX:

  • Disabling FW upgrade
  • Disabling pmon service
  • Patch mlnx-hw-management*

Approach sumary

Use the same image for SimX and for real HW. Introudce new platform strings for SimX.

Add new onie string under devices/mellanox:
├── x86_64-mlnx_msn3700_simx-r0
│   ├── ACS-MSN3700 ../x86_64-mlnx_msn3700-r0/ACS-MSN3700
│   ├── default_sku ../x86_64-mlnx_msn2700-r0/default_sku
│   ├── platform_reboot -> ../x86_64-mlnx_msn2700-r0/platform_reboot
│   ├── platform_wait -> ../x86_64-mlnx_msn2700-r0/platform_wait
│   ├── plugins .../x86_64-mlnx_msn2700-r0/plugins
│   ├── syseeprom.hex
│   └── pmon_daemon_control.json
├── x86_64-mlnx_msn2700_simx-r0
│   ├── ACS-MSN2700 ../x86_64-mlnx_msn2700-r0/ACS-MSN2700
│   ├── default_sku ../x86_64-mlnx_msn2700-r0/default_sku
│   ├── platform_reboot -> ../x86_64-mlnx_msn2700-r0/platform_reboot
│   ├── platform_wait -> ../x86_64-mlnx_msn2700-r0/platform_wait
│   ├── plugins> ../x86_64-mlnx_msn2700-r0/plugins
│   ├── syseeprom.hex
│   └── pmon_daemon_control.json

Disabling fw-upgrade

We can modify the platform/mellanox/mlnx-fw-upgrade.sh to skip the update on SimX in runtime.

Disabling pmon

With the new hooks to dynamically load pmon daemons, we can disable the pmon functionality on our new "devices".

Patching the hw-mgmt

  • make it handle the simulation correctly
  • same hw-mgmt for SimX and HW

Pros

  • Simple
  • Vendor specific changes
  • Allows to use same image for SimX + HW

Verification

The image was verified on msn2700_simx and a real switch. The default SKU was used.

@lguohan
Copy link
Collaborator

lguohan commented Mar 13, 2019

why is this a separate platform? why cannot reuse the mellanox platform?

Copy link
Collaborator

@liat-grozovik liat-grozovik left a comment

Choose a reason for hiding this comment

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

The idea behind new platform is to be able to run ASIC simulator without pmon as well as FW upgrade which is not part of it.
@mykolaf the name says only simx while i believe it should be mellanox-simx. It does everything as mellanox platform but don't run FW upgrade nor start/stop pmon and hw-mgmt service which are unique for mellanox ASIC simulator.
Please also update the commit message with more information about the idea behind the new platform.

@lguohan
Copy link
Collaborator

lguohan commented Mar 13, 2019

I am trying to limit the platform we need to support. I think it can still be mallanox platform since the asic is still mellanox asic. i think in this case we should use the onie platform string to differentiate that.

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

do not introduce a new platform, use mellanox.

@mykolaf mykolaf changed the title [platforms] add simx platform [mellanox-simx] add ability to build simx-compatiable image Mar 20, 2019
Makefile.work Outdated
@@ -118,7 +118,8 @@ SONIC_BUILD_INSTRUCTION := make \
KERNEL_PROCURE_METHOD=$(KERNEL_PROCURE_METHOD) \
HTTP_PROXY=$(http_proxy) \
HTTPS_PROXY=$(https_proxy) \
SONIC_ENABLE_SYSTEM_TELEMETRY=$(ENABLE_SYSTEM_TELEMETRY)
SONIC_ENABLE_SYSTEM_TELEMETRY=$(ENABLE_SYSTEM_TELEMETRY) \
$(SONIC_OVERRIDE_BUILD_VARS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you make this as a run time option. it is better not to build another image at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can not have one image for simx and real hw, because of the hw-management patch.
Can this delayed install feature help with that? Is there some wiki on that feature ?
Maybe we could build both patched and unpatched hw-mgmt and install on system accordingly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

check this

platform/broadcom/one-image.mk:$(SONIC_ONE_IMAGE)_LAZY_INSTALLS += $(DELL_S6000_PLATFORM_MODULE) \
platform/broadcom/platform-modules-s6000.mk:$(DELL_S6000_PLATFORM_MODULE)_PLATFORM = x86_64-dell_s6000_s1220-r0

you can create two hw-mgmt package, one for simx and one for real hw, and then based on the platform onie string, you can selectively to install either one of it.

@@ -6,9 +6,14 @@ Before=ntp-config.service

[Service]
User={{ sonicadmin_user }}
{% if disable_pmon == 'y' %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is better to add some hook to say if onie string matches a specific string, then we do not start pmon docker.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jleveque to give any comments on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with Guohan that it might be easier to maintain if we test for ONIE platform strings here, instead of introducing a new variable.

j2 platform/mellanox/mlnx-fw-upgrade.j2 | sudo tee $FILESYSTEM_ROOT/usr/bin/mlnx-fw-upgrade.sh
{% else %}
echo | sudo tee $FILESYSTEM_ROOT/usr/bin/mlnx-fw-upgrade.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

in the mlnx-fw-upgrade.sh, can you detect the onie string and act accordingly?

@@ -5,8 +5,11 @@ MAIN_TARGET = hw-management_1.mlnx.$(MLNX_HW_MANAGEMENT_VERSION)_amd64.deb

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

ifeq ($(BUILD_SIMX), 'y')
git am 0002-Make-hw-mgmt-SimX-compatiable.diff
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is this patch, in this repo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This patch is not to be upstreamed.

@mykolaf

This comment has been minimized.

@mykolaf mykolaf marked this pull request as ready for review April 5, 2019 11:20
@mykolaf mykolaf dismissed liat-grozovik’s stale review April 5, 2019 13:59

Pull request message updated with approach summary

@mykolaf mykolaf requested a review from lguohan April 5, 2019 13:59
Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

:shipit:

@lguohan lguohan merged commit d993d6f into sonic-net:master Apr 11, 2019
@mykolaf mykolaf deleted the simx branch February 18, 2020 13:14
StormLiangMS added a commit that referenced this pull request May 31, 2023
Why I did it
69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 #2791" DellEMC S6100 Watchdog Support #2854 (8 minutes ago)
4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest #2842 (27 hours ago)
db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit #2852) (33 hours ago)
d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition #2836) (6 days ago)
f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 #2791) (6 days ago)
b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk #2761)" (Support TACACS Accounting #2762) (7 days ago)
3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules #2786) (11 days ago)
e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (#2807) #2837) (2 weeks ago)
33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image #2664) (2 weeks ago)
9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100  #2730) (2 weeks ago)
aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen #2834) (3 weeks ago)
1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process #2818) (3 weeks ago)
3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge #2825) (3 weeks ago)
3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources  #2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch #2832) (3 weeks ago)
2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 #2700) (3 weeks ago)
a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … #2816) (3 weeks ago)
71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads #2830) (3 weeks ago)
1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 #2468) (4 weeks ago) <Julian Chang - TW>
589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 #2827) (4 weeks ago)
634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup #2806) (4 weeks ago)
331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 #2814) (4 weeks ago)
d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil #2824) (4 weeks ago)
ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… #2823) (4 weeks ago)
61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed #2808) (4 weeks ago)
d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " #2817) (4 weeks ago)
88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI #2778) (4 weeks ago)
7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" #2752) (4 weeks ago)
522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. #2753) (4 weeks ago)
b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure #2812) (5 weeks ago)
7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources  #2701) (6 weeks ago)
bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix #2802) (6 weeks ago)
ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738) (6 weeks ago)
88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed #2699) (6 weeks ago)
5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue #2726) (6 weeks ago)
b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error #2796) (6 weeks ago)
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Why I did it
69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 sonic-net#2791" DellEMC S6100 Watchdog Support sonic-net#2854 (8 minutes ago)
4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest sonic-net#2842 (27 hours ago)
db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit sonic-net#2852) (33 hours ago)
d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition sonic-net#2836) (6 days ago)
f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 sonic-net#2791) (6 days ago)
b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk sonic-net#2761)" (Support TACACS Accounting sonic-net#2762) (7 days ago)
3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules sonic-net#2786) (11 days ago)
e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (sonic-net#2807) sonic-net#2837) (2 weeks ago)
33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image sonic-net#2664) (2 weeks ago)
9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100  sonic-net#2730) (2 weeks ago)
aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen sonic-net#2834) (3 weeks ago)
1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process sonic-net#2818) (3 weeks ago)
3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge sonic-net#2825) (3 weeks ago)
3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources  sonic-net#2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch sonic-net#2832) (3 weeks ago)
2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 sonic-net#2700) (3 weeks ago)
a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … sonic-net#2816) (3 weeks ago)
71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads sonic-net#2830) (3 weeks ago)
1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 sonic-net#2468) (4 weeks ago) <Julian Chang - TW>
589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 sonic-net#2827) (4 weeks ago)
634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup sonic-net#2806) (4 weeks ago)
331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 sonic-net#2814) (4 weeks ago)
d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil sonic-net#2824) (4 weeks ago)
ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… sonic-net#2823) (4 weeks ago)
61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed sonic-net#2808) (4 weeks ago)
d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " sonic-net#2817) (4 weeks ago)
88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI sonic-net#2778) (4 weeks ago)
7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" sonic-net#2752) (4 weeks ago)
522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. sonic-net#2753) (4 weeks ago)
b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure sonic-net#2812) (5 weeks ago)
7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources  sonic-net#2701) (6 weeks ago)
bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix sonic-net#2802) (6 weeks ago)
ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738) (6 weeks ago)
88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed sonic-net#2699) (6 weeks ago)
5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue sonic-net#2726) (6 weeks ago)
b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error sonic-net#2796) (6 weeks ago)
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.

4 participants