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

Upload HLD doc of unifying config.bcm #1630

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZhaohuiS
Copy link

It's HLD doc for unifying broadcom config.bcm files.

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@Staphylo
Copy link
Contributor

It's difficult to comment on a docx but I have one major grief with both proposed design.

Essentially we go from a plug&play model to a model where there will be contention on a single file.
So instead of just adding a bunch of files in a HwSku folder for a particular Product we'll now have to do that and edit a common file.
This make the process more burdensome to add new HwSkus and will generate conflicts when multiple open PRs add new HwSku.
In my opinion it is important to retain the plug&play model of HwSkus as it is a design strength and eases platform bringup. Changing this is likely to have negative effects.

Instead of having one common template file import/enumerate every vendor/product/hwsku could we go the other way around and have each broadcom config hwsku import the chip template ?
Alternatively the proposed model could remain if the HwSku were discovered and not hardcoded.

In my opinion there should be one template for each chip provided in the broadcom common template folder (e.g: th2.j2, td3x2.j2, th5.j2, ...)
From there, every vendor hwsku bcm config would import from this template.

This is an example of what a vendor broadcom config could look like by leveraging some jinja2 features.

{% extends "broadcom/templates/chip/th5.j2 %}

{% block lanemap %}
{% endblock %}

@ZhaohuiS
Copy link
Author

It's difficult to comment on a docx but I have one major grief with both proposed design.

Essentially we go from a plug&play model to a model where there will be contention on a single file. So instead of just adding a bunch of files in a HwSku folder for a particular Product we'll now have to do that and edit a common file. This make the process more burdensome to add new HwSkus and will generate conflicts when multiple open PRs add new HwSku. In my opinion it is important to retain the plug&play model of HwSkus as it is a design strength and eases platform bringup. Changing this is likely to have negative effects.

Instead of having one common template file import/enumerate every vendor/product/hwsku could we go the other way around and have each broadcom config hwsku import the chip template ? Alternatively the proposed model could remain if the HwSku were discovered and not hardcoded.

In my opinion there should be one template for each chip provided in the broadcom common template folder (e.g: th2.j2, td3x2.j2, th5.j2, ...) From there, every vendor hwsku bcm config would import from this template.

This is an example of what a vendor broadcom config could look like by leveraging some jinja2 features.

{% extends "broadcom/templates/chip/th5.j2 %}

{% block lanemap %}
{% endblock %}

@Staphylo Thank you for your comment.
Yes, we would like to leverage this design #699.
In this design, the data in common config.bcm file is fixed, but we can enhance it to j2 format and introduce some logic.
I agree with you that we can use chip level template to unify config.bcm, which is also aligned with my proposal.

Offline discussion is ongoing, will send out the new design once it's done.

Thank you again for your review.

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