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 model changes for Sequential IDF isolation #18597

Merged
merged 4 commits into from
May 23, 2024

Conversation

tjchadaga
Copy link
Contributor

Why I did it

To define a field to track isolation state for the IDF to which the T2 device belongs

Work item tracking
  • Microsoft ADO (number only):

How I did it

Added yang model definition, unit tests, sample config and documentation for the table

How to verify it

Validated config tree generation using "pyang -Vf tree -p /usr/local/share/yang/modules/ietf ./yang-models/sonic-voq-inband-interface.yang"

Built the below python-wheels to validate unit tests and other changes
target/python-wheels/bullseye/sonic_yang_mgmt-1.0-py3-none-any.whl
target/python-wheels/bullseye/sonic_yang_models-1.0-py3-none-any.whl
target/python-wheels/bullseye/sonic_config_engine-1.0-py3-none-any.whl

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

  • 202205 (chassis)

Description for the changelog

Link to config_db schema for YANG module changes

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

@tjchadaga tjchadaga marked this pull request as ready for review April 17, 2024 18:06
@tjchadaga tjchadaga requested a review from arlakshm April 17, 2024 18:06
@qiluo-msft qiluo-msft added the YANG YANG model related changes label Apr 17, 2024
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

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

@tjchadaga tjchadaga added Chassis for 202205 branch PRs needed for 202205 branch in msft repo Included in Chassis for 202205 Branch Indicate PR is already in MSFT repo 202205 branch and removed Chassis for 202205 branch PRs needed for 202205 branch in msft repo labels Apr 17, 2024
@tjchadaga tjchadaga requested a review from gechiang April 17, 2024 21:27
@gechiang gechiang added Chassis for 202205 branch PRs needed for 202205 branch in msft repo and removed Included in Chassis for 202205 Branch Indicate PR is already in MSFT repo 202205 branch labels Apr 17, 2024
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM. Please check with other active reviewers.

Copy link
Contributor

@xincunli-sonic xincunli-sonic 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 65cbd12 into sonic-net:master May 23, 2024
19 checks passed
lguohan pushed a commit that referenced this pull request May 31, 2024
Why I did it
In order to isolate an IDF sequentially (one device at a time), need a mechanism to perform downstream isolation on T2 device. Adding support for two kinds of isolation -

By setting no-export BGP community on all routes advertised to T1 (except loopback), so the routes are not advertised to T0s By withdrawing all routes from T1

How I did it

Added

1. New configuration to support the changes described by Yang model changes for Sequential IDF isolation #18597
Script to update configDB on each ASIC to configure no-export/withdraw-all isolation and unisolation of IDF. This is copied to host
2. BGPcfgd changes to handle configDB events and add/remove the required route-maps
3. UT cases and data file changes for BGPCfgd and jinja templates

How to verify it
Corresponding templates on T2 have to be updated to verify the functionality

Configuration options-

sudo idf_isolation withdraw_all
sudo idf_isolation no-export
sudo idf_isolation unisolated

Validate the routes on T1 are either withdrawn, tagged with no-export community or re-advertised based on the configuration
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
Why I did it
In order to isolate an IDF sequentially (one device at a time), need a mechanism to perform downstream isolation on T2 device. Adding support for two kinds of isolation -

By setting no-export BGP community on all routes advertised to T1 (except loopback), so the routes are not advertised to T0s By withdrawing all routes from T1

How I did it

Added

1. New configuration to support the changes described by Yang model changes for Sequential IDF isolation sonic-net#18597
Script to update configDB on each ASIC to configure no-export/withdraw-all isolation and unisolation of IDF. This is copied to host
2. BGPcfgd changes to handle configDB events and add/remove the required route-maps
3. UT cases and data file changes for BGPCfgd and jinja templates

How to verify it
Corresponding templates on T2 have to be updated to verify the functionality

Configuration options-

sudo idf_isolation withdraw_all
sudo idf_isolation no-export
sudo idf_isolation unisolated

Validate the routes on T1 are either withdrawn, tagged with no-export community or re-advertised based on the configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chassis for 202205 branch PRs needed for 202205 branch in msft repo YANG YANG model related changes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants