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

Yang model changes for Sequential IDF isolation #18597

Merged
merged 4 commits into from
May 23, 2024
Merged

Yang model changes for Sequential IDF isolation #18597

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96099c7
Merged PR 9686034: Yang model changes for Sequential IDF isolation
tjchadaga Mar 14, 2024
66d2358
Add T2 check for idf_isolation_state init
tjchadaga Apr 8, 2024
7f9c7ae
Remove spinerouter check
tjchadaga Apr 17, 2024
15481e5
Remove additional white spaces
tjchadaga Apr 18, 2024
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
3 changes: 2 additions & 1 deletion files/build_templates/init_cfg.json.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
},
"BGP_DEVICE_GLOBAL": {
"STATE": {
"tsa_enabled": "false"
"tsa_enabled": "false",
"idf_isolation_state": "unisolated"
}
},
{%- set features = [("bgp", "{% if not DEVICE_RUNTIME_METADATA['ETHERNET_PORTS_PRESENT'] or ('CHASSIS_METADATA' in DEVICE_RUNTIME_METADATA and DEVICE_RUNTIME_METADATA['CHASSIS_METADATA']['module_type'] in ['supervisor']) %}disabled{% else %}enabled{% endif %}", false, "enabled"),
Expand Down
12 changes: 7 additions & 5 deletions src/sonic-yang-models/doc/Configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,16 +384,18 @@ The **BGP_BBR** table contains device-level BBR state.
```
### BGP Device Global

The **BGP_DEVICE_GLOBAL** table contains device-level BGP global state.
It has a STATE object containing device state like **tsa_enabled**
which is set to true if device is currently isolated using
traffic-shift-away (TSA) route-maps in BGP
The **BGP_DEVICE_GLOBAL** table contains device-level BGP global state.
It has a STATE object containing device state like **tsa_enabled**
which is set to true if device is currently isolated using
traffic-shift-away (TSA) route-maps in BGP. It also holds IDF isolation state
which could be one of isolated_no_export, isolated_withdraw_all or unisolated

```
{
"BGP_DEVICE_GLOBAL": {
"STATE": {
"tsa_enabled": "true"
"tsa_enabled": "true",
"idf_isolation_state": "isolated_no_export"
}
}
```
Expand Down
3 changes: 2 additions & 1 deletion src/sonic-yang-models/tests/files/sample_config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,8 @@
},
"BGP_DEVICE_GLOBAL": {
"STATE": {
"tsa_enabled": "false"
"tsa_enabled": "false",
"idf_isolation_state": "unisolated"
}
},
"BGP_PEER_RANGE": {
Expand Down
18 changes: 16 additions & 2 deletions src/sonic-yang-models/tests/yang_model_tests/tests/bgp_device_global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,23 @@
"BGP_DEVICE_GLOBAL_WITH_TSB_TEST": {
"desc": "Load bgp device global table with tsa_enabled set to false"
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE": {
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_1": {
"desc": "Load bgp device global table with idf_isolation_state set to isolated_no_export"
},
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_2": {
"desc": "Load bgp device global table with idf_isolation_state set to isolated_withdraw_all"
},
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_3": {
"desc": "Load bgp device global table with idf_isolation_state set to unisolated"
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE_1": {
"desc": "Load bgp device global table with invalid value",
"eStrKey": "InvalidValue",
"eStr": ["tsa_enabled"]
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE_2": {
"desc": "Load bgp device global table with invalid value",
"eStrKey": "InvalidValue",
"eStr": ["idf_isolation_state"]
}
}
}
54 changes: 49 additions & 5 deletions src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp_device_global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "false"
"tsa_enabled": "false",
"idf_isolation_state": "unisolated"
}
}
}
Expand All @@ -12,7 +13,8 @@
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "true"
"tsa_enabled": "true",
"idf_isolation_state": "unisolated"
}
}
}
Expand All @@ -21,16 +23,58 @@
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "false"
"tsa_enabled": "false",
"idf_isolation_state": "unisolated"
}
}
}
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE": {
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_1": {
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "FALSE"
"tsa_enabled": "true",
"idf_isolation_state": "isolated_no_export"
}
}
}
},
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_2": {
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "false",
"idf_isolation_state": "isolated_withdraw_all"
}
}
}
},
"BGP_DEVICE_GLOBAL_WITH_IDF_ISOLATION_TEST_3": {
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "false",
"idf_isolation_state": "unisolated"
}
}
}
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE_1": {
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "FALSE",
"idf_isolation_state": "unisolated"
}
}
}
},
"BGP_DEVICE_GLOBAL_WITH_INVALID_VALUE_2": {
"sonic-bgp-device-global:sonic-bgp-device-global": {
"sonic-bgp-device-global:BGP_DEVICE_GLOBAL": {
"STATE":{
"tsa_enabled": "false",
"idf_isolation_state": "isolated"
}
}
}
Expand Down
16 changes: 15 additions & 1 deletion src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,26 @@ module sonic-bgp-device-global {
container BGP_DEVICE_GLOBAL {
container STATE {
description "BGP device-specific global data";
leaf tsa_enabled {
leaf tsa_enabled {
tjchadaga marked this conversation as resolved.
Show resolved Hide resolved
type boolean;
default "false";
description
"When set to true, Traffic is shifted away (TSA), i.e, BGP routes are not advertised to neighboring routers";
}
leaf idf_isolation_state {
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

idf_isolation_state

Is there any limitation of this field on M0/T0/T1 devices? If no, could we generalize the isolation for all roles, not to mention IDF? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IDF isolation is only supported on T2 and the config will not have any effect on T0/T1 devices.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then can we enforce a value for M0/T0/T1? The yang model could help the enforcement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the field is initialized to "unisolated" on all devices (including M0/T0/T1) in init_cfg.json.j2. Changing this configuration through CLI/Script is blocked on non-T2 devices. Is that sufficient?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the backend behavior if a T0 got config which is not unisolated? will it fail or ignore?

If the backend will treat this as a failure, we can also hardening yang models, so when ConfigDB is validated by yang models, the failure could be detected earlier, and stop the configuration process immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only T2 will handle this config change. T0/T1 will ignore it

type enumeration {
enum isolated_no_export {
description "IDF isolated using no-export community tag";
tjchadaga marked this conversation as resolved.
Show resolved Hide resolved
}
enum isolated_withdraw_all {
description "IDF isolated by withdrawing routes";
tjchadaga marked this conversation as resolved.
Show resolved Hide resolved
}
enum unisolated {
description "IDF un-isolated";
}
}
default unisolated;
}
} /* end of STATE container */
} /* end of BGP_DEVICE_GLOBAL container */

Expand Down
Loading