[multi-ASIC] BGP internal neighbor table support #5520

judyjoseph · 2020-10-01T15:52:26Z

- Why I did it

Introduce a new table for BGP internal neighbor sessions which will be used in case of multi-asic platforms for the IBGP sessions between the BGP instances running in namespaces mapped to ASIC. This is the follow up of the comments added for a fix earlier to create internal BGP peer group ( #4620 (comment) )

- How I did it
Updated the minigraph parsing logic to get the bgp_internal_sessions

- How to verify it
Checked the DB to find that the BGP_INTERNAL_NEIGHBOR table is populated with internal BGP neigibors.

- 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)

src/sonic-bgpcfgd/bgpcfgd

dockers/docker-fpm-frr/frr/bgpd/templates/internal/policies.conf.j2

pavel-shirshov

Can you please add tests for the new templates and update tests for the old?

pavel-shirshov · 2020-10-15T19:22:47Z

Can you please add template tests data in src/sonic-bgpcfgd/tests/data ?

judyjoseph · 2020-10-15T19:28:29Z

Can you please add template tests data in src/sonic-bgpcfgd/tests/data ?

@pavel-shirshov I updated/added tests here in my earlier commit 2b15a57. But let me check if I need to cover more scenarios

judyjoseph · 2020-10-15T21:11:09Z

retest this please

pavel-shirshov

I'd better remove not required entries.

src/sonic-bgpcfgd/tests/data/general/instance.conf/param_all_v4.json

src/sonic-bgpcfgd/tests/data/general/instance.conf/param_all_v6.json

src/sonic-bgpcfgd/tests/data/general/peer-group.conf/param_base.json

src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json

src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json

…l neighbor template files.

judyjoseph · 2020-10-23T04:59:34Z

retest this please

judyjoseph · 2020-10-26T16:21:01Z

retest vsimage please

pavel-shirshov

LGTM

judyjoseph · 2020-10-28T00:26:28Z

@arlakshm , Could you take a look as well ?

arlakshm

lgtm, couple of minor comments

arlakshm · 2020-10-28T19:55:05Z

dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2

+!
+ neighbor {{ neighbor_addr }} remote-as {{ bgp_session['asn'] }}
+ neighbor {{ neighbor_addr }} description {{ bgp_session['name'] }}
+!


Minor comment, should add the lines to add the timer configuration if they are not default.
These configuration
https://github.com/Azure/sonic-buildimage/blob/09d5a62fad22313b29afe1419d329abe45fbef6d/dockers/docker-fpm-frr/frr/bgpd/templates/general/instance.conf.j2#L6-L10

Sure will update. As discussed will raise a follow up PR for adding this change.

arlakshm · 2020-10-28T20:08:36Z

dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2

+ neighbor {{ neighbor_addr }} peer-group INTERNAL_PEER_V4
+!
+{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
+ neighbor {{ neighbor_addr }} route-map FROM_BGP_INTERNAL_PEER_V4 in


Is route-map configuration line repeated? The same line present in the peer_group.conf.j2 configuration

I looked at both .. they are not same.
Here we set it per neighbor_ip_address, while in the peer_group.conf.j2 it is set on the peer group -- similar to how it was done before as well.

- 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()

…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()

* Initial commit for BGP internal neighbor table support. > Add new template named "internal" for the internal BGP sessions > Add a new table in database "BGP_INTERNAL_NEIGHBOR" > The internal BGP sessions will be stored in this new table "BGP_INTERNAL_NEIGHBOR" * Changes in template generation tests with the introduction of internal neighbor template files.

- 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()

* Initial commit for BGP internal neighbor table support. > Add new template named "internal" for the internal BGP sessions > Add a new table in database "BGP_INTERNAL_NEIGHBOR" > The internal BGP sessions will be stored in this new table "BGP_INTERNAL_NEIGHBOR" * Changes in template generation tests with the introduction of internal neighbor template files.

…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()

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%]

pavel-shirshov reviewed Oct 1, 2020

View reviewed changes

src/sonic-bgpcfgd/bgpcfgd Outdated Show resolved Hide resolved

pavel-shirshov requested a review from tahmed-dev October 1, 2020 17:12

pavel-shirshov reviewed Oct 1, 2020

View reviewed changes

dockers/docker-fpm-frr/frr/bgpd/templates/internal/policies.conf.j2 Outdated Show resolved Hide resolved

judyjoseph force-pushed the bgp_internal_neighbor branch from 46dc4ca to 6fd60a6 Compare October 9, 2020 00:14

pavel-shirshov suggested changes Oct 9, 2020

View reviewed changes

judyjoseph force-pushed the bgp_internal_neighbor branch from 6fd60a6 to 2b15a57 Compare October 13, 2020 06:25

judyjoseph marked this pull request as ready for review October 15, 2020 17:43

pavel-shirshov previously approved these changes Oct 16, 2020

View reviewed changes

judyjoseph requested a review from arlakshm October 22, 2020 22:28

judyjoseph dismissed pavel-shirshov’s stale review via d0838bd October 22, 2020 22:56

judyjoseph added 5 commits October 22, 2020 16:25

Initial commit for BGP internal neighbor table support

e5d0120

Update for comments

201d391

Changes in template generation tests with the introduction on interna…

b3f6135

…l neighbor template files.

Take care of unit test failures.

13d6d0c

Remove sub_role from "general" profile based tests

a89643d

judyjoseph force-pushed the bgp_internal_neighbor branch from d0838bd to a89643d Compare October 22, 2020 23:29

pavel-shirshov approved these changes Oct 26, 2020

View reviewed changes

arlakshm approved these changes Oct 28, 2020

View reviewed changes

judyjoseph merged commit 6088bd5 into sonic-net:master Oct 28, 2020

judyjoseph mentioned this pull request Oct 30, 2020

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

Merged

3 tasks

abdosi added Request for 201911 Branch Included in 201911 Branch labels Nov 10, 2020

judyjoseph deleted the bgp_internal_neighbor branch November 11, 2020 07:15

mini-nair-dell mentioned this pull request Nov 12, 2020

S6100/201911/WFB/BGP docker container fails to run #5909

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[multi-ASIC] BGP internal neighbor table support #5520

[multi-ASIC] BGP internal neighbor table support #5520

judyjoseph commented Oct 1, 2020 •

edited by pavel-shirshov

Loading

pavel-shirshov left a comment

pavel-shirshov commented Oct 15, 2020

judyjoseph commented Oct 15, 2020

judyjoseph commented Oct 15, 2020

pavel-shirshov left a comment

judyjoseph commented Oct 23, 2020

judyjoseph commented Oct 26, 2020

pavel-shirshov left a comment

judyjoseph commented Oct 28, 2020

arlakshm left a comment

arlakshm Oct 28, 2020

judyjoseph Oct 28, 2020 •

edited

Loading

arlakshm Oct 28, 2020

judyjoseph Oct 28, 2020

[multi-ASIC] BGP internal neighbor table support #5520

[multi-ASIC] BGP internal neighbor table support #5520

Conversation

judyjoseph commented Oct 1, 2020 • edited by pavel-shirshov Loading

pavel-shirshov left a comment

Choose a reason for hiding this comment

pavel-shirshov commented Oct 15, 2020

judyjoseph commented Oct 15, 2020

judyjoseph commented Oct 15, 2020

pavel-shirshov left a comment

Choose a reason for hiding this comment

judyjoseph commented Oct 23, 2020

judyjoseph commented Oct 26, 2020

pavel-shirshov left a comment

Choose a reason for hiding this comment

judyjoseph commented Oct 28, 2020

arlakshm left a comment

Choose a reason for hiding this comment

arlakshm Oct 28, 2020

Choose a reason for hiding this comment

judyjoseph Oct 28, 2020 • edited Loading

Choose a reason for hiding this comment

arlakshm Oct 28, 2020

Choose a reason for hiding this comment

judyjoseph Oct 28, 2020

Choose a reason for hiding this comment

judyjoseph commented Oct 1, 2020 •

edited by pavel-shirshov

Loading

judyjoseph Oct 28, 2020 •

edited

Loading