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

Set 'origin' and 'AS Path' for T1 SLB routes #13613

Merged
merged 6 commits into from
Feb 8, 2023
Merged

Set 'origin' and 'AS Path' for T1 SLB routes #13613

merged 6 commits into from
Feb 8, 2023

Conversation

jcaiMR
Copy link
Contributor

@jcaiMR jcaiMR commented Feb 3, 2023

Why I did it

Add support for Set 'origin' and 'AS Path' for T1 SLB routes.

When SLB programs routes to T1 (overlay routes), T1 must advertise such routes to match with the existing SLB 'origin' and AS path length. Following changes to be done during T1 advertisement:

'origin' set to 'incomplete' (value 2)
AS path to be appended as "x" + "x" (two prepend) to match the current AS path at T1 layer

How I did it

Add origin and as-path prepend for routemap FROM_SDN_SLB_ROUTES.

How to verify it

Follow test steps in HLD.
image
image

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

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

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

@StormLiangMS
Copy link
Contributor

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

cmds.append("route-map %s permit 100" % ("%s_RM" % rm))
bgp_asn = \
self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)['localhost']['bgp_asn']
cmds.append(" set as-path prepend %s %s" % (bgp_asn, bgp_asn))
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer to have a fixed value for AS prepend instead of prepending the current device's AS value. This could be gotten from /etc/sonic/constants.yml : 2. Not sure if this is significant changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@prsunny
Copy link
Contributor

prsunny commented Feb 6, 2023

@jcaiMR , likely to have conflict for 202012. Would you raise another PR for 202012? @siqbal1986 for viz

@jcaiMR
Copy link
Contributor Author

jcaiMR commented Feb 7, 2023

@jcaiMR , likely to have conflict for 202012. Would you raise another PR for 202012? @siqbal1986 for viz

Sure, I will open the PR today.

@jcaiMR
Copy link
Contributor Author

jcaiMR commented Feb 7, 2023

@jcaiMR , likely to have conflict for 202012. Would you raise another PR for 202012? @siqbal1986 for viz

@prsunny Hi Prince, I checked the code and also tried this diff with 202012 base, there is no gap from code. Maybe we can still waiting for the 'squash merge' into 202012.

@sonic-net sonic-net deleted a comment from jcaiMR Feb 7, 2023
@prsunny
Copy link
Contributor

prsunny commented Feb 8, 2023

@jcaiMR , since this is a dependency on constants.yml, lets ensure a sonic-mgmt test is added so that any future change wont break this implementation.

@prsunny prsunny merged commit c8cf20c into sonic-net:master Feb 8, 2023
@jcaiMR
Copy link
Contributor Author

jcaiMR commented Feb 9, 2023

@jcaiMR , since this is a dependency on constants.yml, lets ensure a sonic-mgmt test is added so that any future change wont break this implementation.

Sure, Storm has write some test cases for this SDN SLB routes and may ready to PR, I will add some extra test cases for this commit.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Feb 9, 2023
* set origin and as-path prepend for routes from SLB
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #13753

yxieca pushed a commit that referenced this pull request Feb 10, 2023
* set origin and as-path prepend for routes from SLB

Co-authored-by: jcaiMR <[email protected]>
qiluo-msft pushed a commit that referenced this pull request Feb 10, 2023
* set origin and as-path prepend for routes from SLB
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Feb 17, 2023
* set origin and as-path prepend for routes from SLB
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #13860

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants