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

Adding mmu buffer yang files #7838

Merged
merged 35 commits into from
Nov 10, 2021
Merged

Conversation

AmitKaushik7
Copy link
Contributor

Why I did it

Added Buffer yang files derived from sonic/mgmt-cvl/testdata/schema from sonic-mgmt-common,  used by mgmt-framework.

Updated BUFFER_PG|({ifname},)*|{pg_num} to BUFFER_PG|{ifname}|{pg_num} in sonic-buffer-pg.yang.
This change is required for configuration migration for dynamic port breakout operation.

Added sonic-buffer-queue.yang for BUFFER_QUEUE

Tables: BUFFER_POOL, BUFFER_PROFILE, BUFFER_PG, BUFFER_QUEUE.

How I did it

Defined Yang models for BUFFER tables based on Guideline doc:
https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md
and
https://github.com/Azure/sonic-utilities/blob/master/doc/Command-Reference.md

How to verify it

sonic_yang_models package build.

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

Description for the changelog

MMU Buffer yang files (BUFFER_POOL/BUFFER_PROFILE/BUFFER_PG/BUFFER_QUEUE)

@anshuv-mfst anshuv-mfst added the YANG YANG model related changes label Jun 10, 2021
@lguohan lguohan requested a review from neethajohn June 10, 2021 17:34
@AmitKaushik7
Copy link
Contributor Author

Iguohan/Neethajohn Could you review the PR and share you feedback

Copy link
Collaborator

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

Updated comments from YANG WG.

src/sonic-yang-models/yang-models/sonic-buffer-pg.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-pg.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-pg.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-pg.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-queue.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-queue.yang Outdated Show resolved Hide resolved
src/sonic-yang-models/yang-models/sonic-buffer-queue.yang Outdated Show resolved Hide resolved
type string;
}

leaf static_th {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can have either static or dynamic, we may need choice statement here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will add a choice statement here.

Copy link
Contributor Author

@AmitKaushik7 AmitKaushik7 Jul 14, 2021

Choose a reason for hiding this comment

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

Praveen I tried following choice/case statement for the threshold, but got the error mentioned below.
Statement-
choice threshold_type {
mandatory true;
case static {
leaf static_th {
type uint64;
description "The maximum size of the buffer pool the port can occupy";
}
}
case dynamic {
leaf dynamic_th {
type int32 {
range "-7..3";
}
description "The maximum proportion of the free size of the buffer pool the port can occupy (Alpha value)";
}
}
}

Error-

       self.root = None
       self.sysLog(msg="Data Loading Failed:{}".format(str(e)), \
        debug=syslog.LOG_ERR, doPrint=True)
     raise SonicYangException("Data Loading Failed\n{}".format(str(e)))

E sonic_yang_ext.SonicYangException: Data Loading Failed
E string indices must be integers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Praveen, Do you see any issue with the above yang block for choice.


leaf profile { //Hash reference key
type leafref {
path "/bpf:sonic-buffer-profile/bpf:BUFFER_PROFILE/bpf:BUFFER_PROFILE_LIST/bpf:name";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This PR has a dependency on sonic-net/sonic-swss#1754, please merge this PR once the dependent PR is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Venkat, sonic-net/sonic-swss#1754 is still not merged. Does the current PR has dependency on #1754.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the current backend expectation is to have a table name (BUFFER_PROFILE) also while referencing to profile table from BUFFER_QUEUE and other tables.

127.0.0.1:6379[4]> hgetall BUFFER_QUEUE|Ethernet38|5-6

  1. "profile"
  2. "[BUFFER_PROFILE|egress_lossy_profile]"

@zhangyanzhao
Copy link
Collaborator

@venkatmahalingam will take a further look, then we can merge.

@venkatmahalingam
Copy link
Collaborator

@venkatmahalingam will take a further look, then we can merge.

This PR has a dependency on sonic-net/sonic-swss#1754

@zhangyanzhao
Copy link
Collaborator

BRCM team will follow-up this.

@zhangyanzhao
Copy link
Collaborator

Please check with @stephenxs on the approval. Thanks.

@AmitKaushik7
Copy link
Contributor Author

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 7838 in repo Azure/sonic-buildimage

@AmitKaushik7
Copy link
Contributor Author

@neethajohn Can you see if this PR can be merged. Stephen had approved the changes. Builds & tests failure were not related to the changes in this PR. After rebase all builds are successful and tests are passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants