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.

Sign up for GitHub

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

Jump to bottom

SONiC Yang model support for Mirror #7877

Merged
merged 17 commits into from
Dec 2, 2021
Merged

SONiC Yang model support for Mirror #7877

merged 17 commits into from
Dec 2, 2021

Conversation

rupesh-k
Copy link
Contributor

Why I did it

Created SONiC Yang model for Mirror.
Tables: MIRROR_SESSION

How I did it

Defined Yang models for COPP 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

'''
============================= test session starts ==============================
platform linux -- Python 3.7.3, pytest-3.10.1, py-1.7.0, pluggy-0.8.0
rootdir: /sonic/src/sonic-yang-models, inifile:
plugins: cov-2.6.0
collected 3 items

tests/test_sonic_yang_models.py .. [ 66%]
tests/yang_model_tests/test_yang_model.py . [100%]

=============================== warnings summary ===============================

module: sonic-mirror-session
+--rw sonic-mirror-session
+--rw MIRROR_SESSION
+--rw MIRROR_SESSION_LIST* [name]
+--rw name string
+--rw type? string
+--rw src_ip? inet:ipv4-address
+--rw dst_ip? inet:ipv4-address
+--rw gre_type? string
+--rw dscp? uint8
+--rw ttl? uint8
+--rw queue? uint8
+--rw dst_port? -> /port:sonic-port/PORT/PORT_LIST/name
+--rw src_port? union
+--rw direction? string

'''

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

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@rupesh-k rupesh-k requested a review from lguohan as a code owner June 15, 2021 15:43
@rupesh-k
Copy link
Contributor Author

@prsunny , @lguohan

Can you please help review. Thank you.

kwangsuk pushed a commit to kwangsuk/sonic-buildimage that referenced this pull request Jun 15, 2021
@lguohan
[frr]: upgrade frr to frr-7.5-s2 tag (sonic-net#6628)
5c8b70e 
pick-up upstream fixes. important ones

- bgpd: Removing "neighbor <peer-group> allowas-in"

complete lists

*   6b2b5cce3 2021-01-29 | Merge pull request sonic-net#7977 from ton31337/fix/allowas_in_reset_value_7.5 (HEAD -> frr/7.5, tag: frr-7.5-s2, upstream/stable/7.5, origin/frr/7.5, stable/7.5) [Donald Sharp]
|\
| * f403534df 2021-01-28 | bgpd: Removing "neighbor <peer-group> allowas-in" [Kishore Kunal]
* |   86e2f106c 2021-01-28 | Merge pull request sonic-net#7962 from ton31337/fix/bgpd_validate_community_7.5 [Donald Sharp]
|\ \
| |/
|/|
| * e182af45c 2021-01-05 | bgpd: separate lcommunity validation from tokenizer [Wesley Coakley]
| * 2cf37d594 2020-12-30 | bgpd: Validate community list if they are not malformed [Donatas Abraitis]
|/
*   a4af08a19 2021-01-22 | Merge pull request sonic-net#7912 from idryzhov/7.5-backports-2021-01 [Donald Sharp]
|\
| * 160021013 2021-01-12 | bgpd : multiple memory leak fixes in show commands [Sarita Patra]
| * 46a2b560f 2021-01-19 | tools: fix frr-reload BFD profile support [Rafael Zalamena]
| * 7f6c81fca 2020-12-02 | ospfd: fix area removal at interface level [ckishimo]
| * f7db4dfb1 2021-01-08 | bfdd: update vrf of received packet [Philippe Guibert]
| * 4d470f3ef 2021-01-08 | bfdd: enable bfd session if vrf interface available [Philippe Guibert]
| * c656985fb 2021-01-08 | bfdd: socket should be bound to vrf interface by default [Philippe Guibert]
| * f30c002b8 2021-01-15 | bgpd: Allow peer-groups to have `ttl-security hops` configured [Donald Sharp]
| * 85ff76513 2021-01-15 | configure.ac: Correct library name for sysrepo [Bo Zhang]
| * d00c543f1 2020-12-04 | bgpd: Handle IPv6 prefixes with IPv4 nexthops for zebra [Donatas Abraitis]
| * 87b9b2973 2021-01-06 | zebra: zebra route-map delay-timer is global not per vrf [Donald Sharp]
| * 91e1adec9 2021-01-05 | bgpd: Fix default-originate clearing from peer-groups. [zyxwvu Shi]
| * 0f2f32fa1 2021-01-05 | isisd: When last area address is removed, resign if we were DR [Karen Schoener]
| * 842e99d49 2021-01-02 | vrrpd.yang bug fix: modify augment path to comply with rfc 7950 [Bo Zhang]
| * 9616ef937 2020-12-24 | ospfd: fix no show database output when selecting vrf [Louis Scalbert]
| * 4c4764e36 2020-11-30 | ospf6d: ospfv3 disable on the interface, but interface prefix still shown in the output [Yash Ranjan]
| * 1870dbd86 2020-12-14 | ospf6d: Link LSA is not updated when router priority is modified [Mobashshera Rasool]
| * 4883a06c3 2020-12-10 | bgpd: fix evpn route-map vni filter at origin [Chirag Shah]
|/
*   9c087052a 2021-01-15 | Merge pull request sonic-net#7877 from vishaldhingra/static_7_5 [Mark Stapp]
|\
| * a687b6b27 2021-01-15 | staticd: Backend cofiguration code to fix table-id problem [vdhingra]
| * 52370b494 2021-01-15 | staticd: autogenerated code modifications due to yang changes [vdhingra]
| * f9d6511f2 2021-01-15 | staticd: make table-id as the key for path-list [vdhingra]
|/

Signed-off-by: Guohan Lu <lguohan@gmail.com>
@rupesh-k
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

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

@amin-alavi
Copy link

@prsunny , @lguohan ,
Can you please help review? Thank you.

@anshuv-mfst anshuv-mfst added the YANG YANG model related changes label Jul 1, 2021
venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang Outdated

import sonic-port {
prefix port;
revision-date 2019-07-01;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this revision date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed.

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang Outdated

leaf type {
type string {
pattern "ERSPAN|SPAN";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang Outdated
}

leaf dst_port {
type leafref {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add "when" condition to configure this field only when type set to SPAN? the similar condition needs to be done ERSPAN fields as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. added ERSPAN and SPAN conditions. Some fields are valid for both SPAN and ERSPAN.

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang
}

leaf queue {
type uint8;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any valid queue range?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is ASIC specific and we cant have the validation in yang model.
Please see for more details. #8189

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang
leaf name {
type string {
pattern '[a-zA-Z0-9]{1}([-a-zA-Z0-9_]{0,31})';
length 1..32 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it max len 32, any backend restriction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No Backend limitation. But as discussed in meeting, keeping this check same.

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang Outdated

leaf direction {
type string {
pattern "RX|TX|BOTH";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be enum as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

venkatmahalingam
venkatmahalingam reviewed Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang
leaf dst_port {
type leafref {
path "/port:sonic-port/port:PORT/port:PORT_LIST/port:name";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

CPU dst_port is not supported?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add dst_port as discussed in the meeting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added CPU to dst_port

dgsudharsan
dgsudharsan suggested changes Jul 8, 2021
View reviewed changes
src/sonic-yang-models/yang-models/sonic-mirror-session.yang
leaf dst_port {
type leafref {
path "/port:sonic-port/port:PORT/port:PORT_LIST/port:name";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add dst_port as discussed in the meeting

src/sonic-yang-models/yang-models/sonic-mirror-session.yang
"Mirror session type.";
}

leaf src_ip {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add the must check for src_ip and dst_ip ERSPAN as discussed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to ip-address and enforce it is only ipv4 so that in future when supported we can remove the check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added ERSPAN checks. Changed to ip-address and added must condition to have only IPv4 Address for now.

@anshuv-mfst anshuv-mfst requested a review from abdosi July 8, 2021 18:07
@anshuv-mfst
Copy link

Hi @abdosi - could you please take a look, thanks

@zhangyanzhao
Copy link
Collaborator

@rupesh-k please help to address the build failure and conflict.

@zhangyanzhao
Copy link
Collaborator

BRCM is still working on this and ETA is end of next week.

@zhangyanzhao
Copy link
Collaborator

BRCM team is working on addressing the comments, ETA is 9/30.

@rupesh-k
Copy link
Contributor Author

Taken care of all comments in review meeting and in this PR.
Please review .

@zhangyanzhao
Copy link
Collaborator

branch conflict need be resolved before merge. @rupesh-k can you please fix those conflicts? Thanks.

@zhangyanzhao
Copy link
Collaborator

Will merge once the build succeed

@zhangyanzhao
Copy link
Collaborator

@rupesh-k would you please check and fix the build failure? Thanks.

@rupesh-k
Copy link
Contributor Author

rupesh-k commented Nov 4, 2021

Last time I saw the build failures are in not related to my changes. But Now the build is deleted. So Pushed after rebasing to latest master. Will follow up on current builds.

@rupesh-k
Copy link
Contributor Author

Hi @zhangyanzhao

All checks are successful. Please help with merging

Thanks

ganglyu
ganglyu approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@ganglyu ganglyu left a comment

Choose a reason for hiding this comment

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

lgtm

@qiluo-msft qiluo-msft merged commit a3d76bd into sonic-net:master Dec 2, 2021
@qiluo-msft qiluo-msft added the Request for 202111 Branch For PRs being requested for 202111 branch label Dec 2, 2021
judyjoseph pushed a commit that referenced this pull request Dec 27, 2021
@rupesh-k @judyjoseph
SONiC Yang model support for Mirror (#7877)
90b74d5 
#### Why I did it
Created SONiC Yang model for Mirror.
Tables: MIRROR_SESSION

#### How I did it

Defined Yang models for COPP 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
'''
============================= test session starts ==============================
platform linux -- Python 3.7.3, pytest-3.10.1, py-1.7.0, pluggy-0.8.0
rootdir: /sonic/src/sonic-yang-models, inifile:
plugins: cov-2.6.0
collected 3 items

tests/test_sonic_yang_models.py ..                                       [ 66%]
tests/yang_model_tests/test_yang_model.py .                              [100%]

=============================== warnings summary ===============================

module: sonic-mirror-session
  +--rw sonic-mirror-session
     +--rw MIRROR_SESSION
        +--rw MIRROR_SESSION_LIST* [name]
           +--rw name         string
           +--rw type?        string
           +--rw src_ip?      inet:ipv4-address
           +--rw dst_ip?      inet:ipv4-address
           +--rw gre_type?    string
           +--rw dscp?        uint8
           +--rw ttl?         uint8
           +--rw queue?       uint8
           +--rw dst_port?    -> /port:sonic-port/PORT/PORT_LIST/name
           +--rw src_port?    union
           +--rw direction?   string

'''
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this pull request Dec 23, 2022
@rupesh-k
SONiC Yang model support for Mirror (sonic-net#7877)
6f478c1 
Created SONiC Yang model for Mirror.
Tables: MIRROR_SESSION

Defined Yang models for COPP 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

'''
============================= test session starts ==============================
platform linux -- Python 3.7.3, pytest-3.10.1, py-1.7.0, pluggy-0.8.0
rootdir: /sonic/src/sonic-yang-models, inifile:
plugins: cov-2.6.0
collected 3 items

tests/test_sonic_yang_models.py ..                                       [ 66%]
tests/yang_model_tests/test_yang_model.py .                              [100%]

=============================== warnings summary ===============================

module: sonic-mirror-session
  +--rw sonic-mirror-session
     +--rw MIRROR_SESSION
        +--rw MIRROR_SESSION_LIST* [name]
           +--rw name         string
           +--rw type?        string
           +--rw src_ip?      inet:ipv4-address
           +--rw dst_ip?      inet:ipv4-address
           +--rw gre_type?    string
           +--rw dscp?        uint8
           +--rw ttl?         uint8
           +--rw queue?       uint8
           +--rw dst_port?    -> /port:sonic-port/PORT/PORT_LIST/name
           +--rw src_port?    union
           +--rw direction?   string

'''
 Conflicts:
	src/sonic-yang-models/setup.py
	src/sonic-yang-models/tests/files/sample_config_db.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgsudharsan dgsudharsan dgsudharsan approved these changes

@venkatmahalingam venkatmahalingam venkatmahalingam approved these changes

@ganglyu ganglyu ganglyu approved these changes

@lguohan lguohan Awaiting requested review from lguohan

@abdosi abdosi Awaiting requested review from abdosi

Assignees
No one assigned
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@rupesh-k @amin-alavi @anshuv-mfst @zhangyanzhao @dgsudharsan @venkatmahalingam @ganglyu @qiluo-msft @judyjoseph