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

[DPB] Allow BGP_NEIGHBOR YANG model with just IP as the key #7992

Merged
merged 46 commits into from
Aug 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
872ac28
Added support to allow/deny packets matching source IP/destination IP
venkatmahalingam May 14, 2020
bceff9f
Added support to allow/deny packets matching source IP/destination IP…
venkatmahalingam May 14, 2020
2410e97
Merge branch 'master' of https://github.com/venkatmahalingam/sonic-bu…
venkatmahalingam May 14, 2020
93d1a2b
Addressed the comment.
venkatmahalingam Jun 3, 2020
a2e31bb
Comments addressed.
venkatmahalingam Jun 30, 2020
77aca99
Addressed the comment
venkatmahalingam Sep 23, 2020
03fa4a4
Fixed the conflict
venkatmahalingam Feb 27, 2021
ce770fe
[YANG] Add to support BGP and route-map YANG models
venkatmahalingam Feb 27, 2021
8b73264
Updated the revision.
venkatmahalingam Feb 28, 2021
8726536
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Mar 31, 2021
f512b88
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Mar 31, 2021
6eb74b1
Added the unit test cases for BGP & route-map tables.
venkatmahalingam Mar 31, 2021
3e50732
Build errors are fixed.
venkatmahalingam Mar 31, 2021
c6fe9ed
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Apr 9, 2021
655b54a
Added negative testcases and addressed the comments.
venkatmahalingam Apr 9, 2021
1997a62
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Apr 12, 2021
cfd09bf
Addressed the review comments and added additional negative test cases.
venkatmahalingam Apr 15, 2021
e2692d5
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Apr 19, 2021
68cabce
Updated the leaf references for VRF table key name change.
venkatmahalingam Apr 19, 2021
981bb5d
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Apr 20, 2021
aed0a16
Fixed the build error.
venkatmahalingam Apr 20, 2021
e87037b
Comment addressed.
venkatmahalingam Apr 21, 2021
6955e31
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Apr 23, 2021
e4c22b2
Merge remote-tracking branch 'upstream/master'
venkatmahalingam May 4, 2021
54c7028
Merge remote-tracking branch 'upstream/master'
venkatmahalingam May 6, 2021
075d760
Added UT fix and updated bgp common YANG to avoid build errors.
venkatmahalingam May 6, 2021
b52b0b8
Fixed the build errors.
venkatmahalingam May 7, 2021
6e4ccb9
Merge remote-tracking branch 'upstream/master'
venkatmahalingam May 13, 2021
6e9ea10
Comments addressed.
venkatmahalingam May 13, 2021
bf17722
Comments addressed.
venkatmahalingam May 13, 2021
b6a50fc
Merge remote-tracking branch 'upstream/master'
venkatmahalingam May 20, 2021
08188ac
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Jun 2, 2021
5abc11a
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Jun 10, 2021
cf933a3
Commented the VLAN leafref and used the VLAN pattern string as a work…
venkatmahalingam Jun 10, 2021
969d9ad
Commented the custom validation until the infra is ready.
venkatmahalingam Jun 10, 2021
71e4a37
Comments addressed.
venkatmahalingam Jun 11, 2021
b7adb38
Removed the file.
venkatmahalingam Jun 17, 2021
0ee82b1
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Jun 17, 2021
157a7d6
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Jun 25, 2021
d176f2e
Merge remote-tracking branch 'upstream/master'
venkatmahalingam Jun 28, 2021
45e526b
Added BGP_NEIGHBOR list with just IP as the key to allow the current …
venkatmahalingam Jun 28, 2021
d4df009
Comment addressed.
venkatmahalingam Jun 29, 2021
daace08
Removed the fields present in the grouping as the infra is not ready.
venkatmahalingam Jun 29, 2021
d7eee7b
Comment addressed.
venkatmahalingam Jul 6, 2021
858b08f
Removed the sample config that depends on BGP_NEIGHBOR grouping fields.
venkatmahalingam Jul 6, 2021
e63af87
Used the 0 and 1 for boolean.
venkatmahalingam Jul 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/sonic-yang-models/tests/files/sample_config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,15 @@
}
},
"BGP_NEIGHBOR": {
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved
"10.0.0.1": {
"asn": "65200",
"holdtime": "180",
Copy link
Collaborator Author

@venkatmahalingam venkatmahalingam Jun 29, 2021

Choose a reason for hiding this comment

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

 "BGP_NEIGHBOR": {
            "default|192.168.1.1": {
                "asn": "65200",
                "holdtime": "180",
                "keepalive": "60",
                "local_addr": "10.0.0.2",
                "name":"PEER2"
            }
        },

@praveen-li These fields are present in the grouping, looks like libyang is not able to parse it, what's your suggestion? shall we remove these fields and merge the PR to unblock DPB testing? or you can fix the infra to handle the fields from grouping as well?

 Read JSON Section: SAMPLE_CONFIG_DB_JSON
sonic_yang(3):All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['default|192.168.1.1'])
sonic_yang(3):exceptionList:['Value not found for neighbor in default|192.168.1.1', "'asn'"]
sonic_yang(3):Data Loading Failed:All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['default|192.168.1.1'])

Copy link
Collaborator

Choose a reason for hiding this comment

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

@venkatmahalingam: Yeah, [current translation doesn't allow all the variation of leaf, only choice and leaf is allowed.] if you are using grouping, please add support at:
https://github.com/Azure/sonic-buildimage/blob/5acf2348ba88c69bad3300adabd58b270e5bc01f/src/sonic-yang-mgmt/sonic_yang_ext.py#L209

Also, about the other BGP_NEIGHBOR table
-- where the relevant config go, as I understand BGP_NEIGHBOR table in config_db is different.
-- which daemon processes it, does it make use of current YANG models.
-- please point to the code, which is using this BGP_NEIGHBOR yang model.

Copy link
Collaborator Author

@venkatmahalingam venkatmahalingam Jun 29, 2021

Choose a reason for hiding this comment

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

New generic BGP config models are being processed by the below frrcfgd daemon in BGP container.
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-frr-mgmt-framework/frrcfgd/frrcfgd.py

@praveen-li I'm not familiar with libyang infra code, can you please add support for grouping?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@venkatmahalingam: can you please point to the function or relevant line in frrcfgd.py, where it is doing config validation against yang. Also from where it reads the config i.e. source of config?.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@praveen-li There is no config validation against YANG in frrcfgd, frrcfgd simply listens for config-DB events and handles them, all configs validations are expected to handle in SONiC YANGs before pushing to config-DB.

"keepalive": "60",
"local_addr": "10.0.0.2",
"name":"PEER1",
"nhopself":"0",
"rrclient":"0"
},
"default|192.168.1.1": {
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved
}
},
Expand Down
53 changes: 53 additions & 0 deletions src/sonic-yang-models/yang-models/sonic-bgp-neighbor.yang
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,60 @@ module sonic-bgp-neighbor {

container sonic-bgp-neighbor {
container BGP_NEIGHBOR {
list BGP_NEIGHBOR_TEMPLATE_LIST {
description "This list is to support template based BGP neighbor configuration handled by bgpcfgd";
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved
key "neighbor";
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved

leaf neighbor {
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved
type inet:ip-address;
description "BGP Neighbor address";
}

leaf asn {
type uint32 {
range "1..4294967295";
}
description "Peer AS number";
}

leaf holdtime {
type uint16;
description "Hold time";
}

leaf keepalive {
type uint16;
description "Keepalive interval";
}

leaf local_addr {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make sure that local address and Peer address belongs to same address family?

Copy link
Collaborator Author

@venkatmahalingam venkatmahalingam Jul 2, 2021

Choose a reason for hiding this comment

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

I dont see any easy way to check this, do you see easy YANG check?

type inet:ip-address;
description "Local source address or interface name to use for connection.";
}

leaf name {
type string;
description "Peer description";
}

leaf nhopself {
venkatmahalingam marked this conversation as resolved.
Show resolved Hide resolved
type uint8 {
range "0..1";
}
description "Nexthop is self, no nexthop calculation";
}

leaf rrclient {
type uint8 {
range "0..1";
}
description "Route reflector client";
}
}

list BGP_NEIGHBOR_LIST {
description "This list is to support generic BGP neighbor configuration handled by frrcfgd and
frr_mgmt_framework_config field should be set to true in DEVICE_METADATA talbe for accepting the generic BGP table configurations.";
key "vrf_name neighbor";

leaf vrf_name {
Expand Down