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

[multi-ASIC] util changes with the BGP_INTERNAL_NEIGHBOR table. #5760

Merged
merged 3 commits into from
Nov 9, 2020

Conversation

judyjoseph
Copy link
Contributor

@judyjoseph judyjoseph commented Oct 30, 2020

- Why I did it
Update the routine is_bgp_session_internal() by checking the BGP_INTERNAL_NEIGHBOR table.
Additionally to address the review comment #5520 (comment)
Add timer settings as will in the internal session templates and keep it minimal as these sessions which will always be up.
Updates to the internal tests data + add all of it to template tests.

- How I did it
Updated the APIs and the template files.

- How to verify it
Verified the internal BGP sessions are displayed correctly with show commands with this API is_bgp_session_internal()

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

  • 201811
  • 201911
  • 202006

- Description for the changelog

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

@judyjoseph
Copy link
Contributor Author

retest this please

{# set the bgp neighbor timers if they have not default values #}
{% if (bgp_session['keepalive'] is defined and bgp_session['keepalive'] | int != 60)
or (bgp_session['holdtime'] is defined and bgp_session['holdtime'] | int != 180) %}
neighbor {{ neighbor_addr }} timers {{ bgp_session['keepalive'] | default("60") }} {{ bgp_session['holdtime'] | default("180") }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to use minimal timers here. We expect this sessions are live 100% of time

This is because these are internal sessions which will allways be up.
@judyjoseph
Copy link
Contributor Author

retest vs please

@judyjoseph judyjoseph merged commit c972052 into sonic-net:master Nov 9, 2020
judyjoseph added a commit that referenced this pull request Nov 9, 2020
lguohan pushed a commit that referenced this pull request Nov 9, 2020
judyjoseph added a commit to judyjoseph/sonic-buildimage that referenced this pull request Nov 10, 2020
…c-net#5760)

- Why I did it
Update the routine is_bgp_session_internal() by checking the BGP_INTERNAL_NEIGHBOR table.
Additionally to address the review comment sonic-net#5520 (comment)
Add timer settings as will in the internal session templates and keep it minimal as these sessions which will always be up.
Updates to the internal tests data + add all of it to template tests.

- How I did it
Updated the APIs and the template files.

- How to verify it
Verified the internal BGP sessions are displayed correctly with show commands with this API is_bgp_session_internal()
judyjoseph added a commit that referenced this pull request Nov 10, 2020
Reintroduce #5760, along with the fix needed in the template file for python3 compatibility.
abdosi pushed a commit that referenced this pull request Nov 10, 2020
- Why I did it
Update the routine is_bgp_session_internal() by checking the BGP_INTERNAL_NEIGHBOR table.
Additionally to address the review comment #5520 (comment)
Add timer settings as will in the internal session templates and keep it minimal as these sessions which will always be up.
Updates to the internal tests data + add all of it to template tests.

- How I did it
Updated the APIs and the template files.

- How to verify it
Verified the internal BGP sessions are displayed correctly with show commands with this API is_bgp_session_internal()
@judyjoseph judyjoseph deleted the bgp_internal_changes branch November 11, 2020 07:14
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…c-net#5760)

- Why I did it
Update the routine is_bgp_session_internal() by checking the BGP_INTERNAL_NEIGHBOR table.
Additionally to address the review comment sonic-net#5520 (comment)
Add timer settings as will in the internal session templates and keep it minimal as these sessions which will always be up.
Updates to the internal tests data + add all of it to template tests.

- How I did it
Updated the APIs and the template files.

- How to verify it
Verified the internal BGP sessions are displayed correctly with show commands with this API is_bgp_session_internal()
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…-net#5874)

Reintroduce sonic-net#5760, along with the fix needed in the template file for python3 compatibility.
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Jul 18, 2022
Reintroduce the msft templates for multi-asic(3164) after the following sonic public PR's ( for internal BGP neighbors) are available in internal-201911 branch.

sonic-net#5520
sonic-net#5760
sonic-net/sonic-swss-common#389
sonic-net/sonic-utilities#1224

How did I verify
-----------------
1. Verified the external and internal BGP sessions are up, shown correctly in "show ip bgp summary -d all" and show ip interface commands.

2. Ran related 3164 tests to make sure it is passing.

bgp/test_bgp_fact.py::test_bgp_facts[0-0] PASSED                                                                                                           [ 16%]
bgp/test_bgp_fact.py::test_bgp_facts[0-1] PASSED                                                                                                                [ 33%]
bgp/test_bgp_fact.py::test_bgp_facts[0-2] PASSED                                                                                                                [ 50%]
bgp/test_bgp_fact.py::test_bgp_facts[0-3] PASSED                                                                                                                [ 66%]
bgp/test_bgp_fact.py::test_bgp_facts[0-4] PASSED                                                                                                                [ 83%]
bgp/test_bgp_fact.py::test_bgp_facts[0-5] PASSED                                                                                                                [100%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[0] PASSED                                                                                             [ 57%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[1] PASSED                                                                                             [ 63%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[2] SKIPPED                                                                                            [ 68%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[3] SKIPPED                                                                                            [ 73%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[4] SKIPPED                                                                                            [ 78%]
bgp/test_bgp_multipath_relax.py::test_bgp_multipath_relax[5] SKIPPED                                                                                            [ 84%]
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.

4 participants