-
Notifications
You must be signed in to change notification settings - Fork 6
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
Test submit OTSS and Syncd-ot code #8
Conversation
Makefile
Outdated
@@ -29,8 +29,9 @@ ifeq ($(NOBULLSEYE),0) | |||
BUILD_BULLSEYE=1 | |||
endif | |||
|
|||
PLATFORM_PATH := platform/$(if $(PLATFORM),$(PLATFORM),$(CONFIGURED_PLATFORM)) | |||
PLATFORM_CHECKOUT := platform/checkout | |||
PLATFORM_ROOT := $(if $(shell echo $(PLATFORM) | grep 'ot-'),ot-platform,platform) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grep 'ot-' will pick up all the platform name include the substring "ot-", such as "slot-", "hot-"
Makefile
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all platforms (total 9) in sonic are under 'platform' directory. ot-platform folder diverges from existing directory structure? I don't see any technical reason for that.
Makefile.work
Outdated
@@ -136,7 +136,13 @@ endif | |||
rules/config.user: | |||
$(Q)echo -n "" | |||
|
|||
#include different configurations based on target platform | |||
ifneq ($(shell echo $(CONFIGURED_PLATFORM) | grep 'ot-'),) | |||
include rules/config-ot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rule/config is meant for each vendor to edit to fit their device features. We can always add more nobs, No need to introduce a new file config-ot. I don't see rules/config-ot file in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is generated file, should not be checked in.
$(LIBOTAIVS_DBG) | ||
|
||
$(DOCKER_SYNCD_BASE)_VERSION = 1.0.0 | ||
$(DOCKER_SYNCD_BASE)_PACKAGE_NAME = syncd-ot-vs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker package name should not include platform name, as all platforms should have the same syncd-ot as image/package name. as in existing sonic, all platform has docker "syncd".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not check in buildinfo.
ot-platform/ot-vs/kvm-image.mk
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this working in the demo? this is to generate a kvm image bounded with onie image. so you can start kvm sonic in one step. But I saw that ALi demo, still two steps: 1) start onie 2) install sonic-ot-vs.
ot-platform/ot-vs/kvm-image.mk
Outdated
SONIC_KVM_IMAGE = sonic-ot-vs.img.gz | ||
$(SONIC_KVM_IMAGE)_INSTALLS += $(SYSTEMD_SONIC_GENERATOR) \ | ||
$(KVM_4_SLOT_FACTORY_CONFIG) | ||
$(SONIC_KVM_IMAGE)_MACHINE = ot-vs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change name to ot-kvm, you need build an new onie. because we use existing onie for vs, so the kvm image has to be ended with "vs"
ot-platform/ot-vs/onie.mk
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ONIE should be build by each device vendor to preinstall on their product.
In our case a new onie should be build for ot-kvm, see https://github.com/jjin62/onie/tree/2018.11br. you can compile it or just use the iso. (https://github.com/jjin62/onie/tree/2018.11br/build/images) ONIE for Molex device (based on x86): https://github.com/jjin62/onie/tree/2022.08br.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files in template directory are duplicated from /platform/template. Can we make a soft link to original directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is for Mininet topology. remove?
… as global service on otn platform
rules/config-ot
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to have a separate config file file for OT. This file is meant for vendor to customize. In reality, no one would build a SONiC as is, Vendors always have their own distribution.
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)