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

[yang] sonic-mirror-session.yang does not align with ConfigDb #12397

Closed
ganglyu opened this issue Oct 13, 2022 · 11 comments
Closed

[yang] sonic-mirror-session.yang does not align with ConfigDb #12397

ganglyu opened this issue Oct 13, 2022 · 11 comments
Assignees
Labels
BRCM Triaged this issue has been triaged YANG YANG model related changes

Comments

@ganglyu
Copy link
Contributor

ganglyu commented Oct 13, 2022

Description

sonic-mirror-session.yang uses leaf-list for for src_port:
https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-mirror-session.yang#L157

But src_port is not a list in ConfigDB:
admin@vlab-01:~$ sudo config mirror_session erspan add mrr_port 1.2.3.4 20.21.22.23 8 100 1234 0 Ethernet0

admin@vlab-01:~$ redis-cli -n 4 hgetall "MIRROR_SESSION|mrr_port"

  1. "type"
  2. "ERSPAN"
  3. "src_ip"
  4. "1.2.3.4"
  5. "dst_ip"
  6. "20.21.22.23"
  7. "dscp"
  8. "8"
  9. "ttl"
  10. "100"
  11. "gre_type"
  12. "1234"
  13. "queue"
  14. "0"
  15. "src_port"
  16. "Ethernet0"
  17. "direction"
  18. "BOTH"

Steps to reproduce the issue:

  1. Config MIRROR_SESSION table
  2. Dump ConfigDB

Describe the results you received:

sonic-mirror-session.yang does not align with ConfigDb

Describe the results you expected:

SONiC Yang should align with ConfigDB

Output of show version:

(paste your output here)

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):

@ganglyu ganglyu added the YANG YANG model related changes label Oct 13, 2022
@qiluo-msft
Copy link
Collaborator

@praveen-li @Junchao-Mellanox Could you help investigate this issue? It is similar to port lanes attribute.

@Junchao-Mellanox
Copy link
Collaborator

Junchao-Mellanox commented Oct 14, 2022

I suppose this is by design. Redis hash type does not support list value. So, any field defined as leaf-list will be translated into a comma separated string. @praveen-li , correct me if I am wrong.

https://stackoverflow.com/questions/29203717/redis-storing-list-inside-hash

@ganglyu
Copy link
Contributor Author

ganglyu commented Oct 21, 2022

@rupesh-k will fix this issue

@rupesh-k
Copy link
Contributor

Working on this issue. ETA 11/01

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Oct 27, 2022
@qiluo-msft
Copy link
Collaborator

@rupesh-k any update?

@zhangyanzhao
Copy link
Collaborator

@rupesh-k @adyeung would you please help to provide an update on this issue? Thanks.

@zhangyanzhao
Copy link
Collaborator

@adyeung can you please help to find someone in BRCM to take a look? Thanks.

@adyeung
Copy link
Collaborator

adyeung commented Jan 5, 2023

@zhangyanzhao I will followup on the fix

@zhangyanzhao
Copy link
Collaborator

@adyeung any progress on this PR? Thanks.

@adyeung
Copy link
Collaborator

adyeung commented Jan 19, 2023

Expecting a fix from rupesh-k by 1/26

qiluo-msft pushed a commit that referenced this issue Feb 16, 2023
…13578)

Fixes Mirror Yang model src_port to match CONFIG_DB

#### Why I did it
Fixes issue #12397 (comment)
@zhangyanzhao
Copy link
Collaborator

PR is merged and close this issue.

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

No branches or pull requests

6 participants