BGP suppress fib pending script #7430

echuawu · 2023-02-10T07:55:18Z

Add BGP suppress fib pending script
docs/testplan/BGP-Suppress-FIB-Pending-test-plan.md

Description of PR

Summary:
Add a new script to cover BGP suppress fib pending function test.

Fixes # (issue)

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

This functionality provides feedback to FRR so that routes are not advertised until they are installed in the FIB. Support for bgp suppress-fib-pending s required in SONIC.

Supported testbed topology if it's a new test case?

T1 topology testbed

tests/bgp/test_bgp_suppress_fib.py

mssonicbld · 2023-03-27T03:49:27Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/bgp/bgp_helpers.py:241:121: E501 line too long (124 > 120 characters) tests/bgp/test_bgp_suppress_fib.py:317:121: E501 line too long (133 > 120 characters)

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-03-27T03:54:47Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/bgp/test_bgp_suppress_fib.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/bgp/bgp_helpers.py:241:121: E501 line too long (124 > 120 characters)

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-03-27T04:54:03Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/bgp/bgp_helpers.py Fixing tests/bgp/test_bgp_suppress_fib.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

liat-grozovik · 2023-05-10T18:32:31Z

@echuawu could you please handle conflicts?

echuawu · 2023-05-11T03:23:54Z

@echuawu could you please handle conflicts?

Hi @liat-grozovik , done.

echuawu · 2023-05-12T06:29:20Z

/azpw run Azure.sonic-mgmt

mssonicbld · 2023-05-12T06:29:22Z

/AzurePipelines run Azure.sonic-mgmt

azure-pipelines · 2023-05-12T06:29:31Z

Azure Pipelines successfully started running 1 pipeline(s).

echuawu · 2023-05-12T09:20:05Z

/azpw run Azure.sonic-mgmt

mssonicbld · 2023-05-12T09:20:07Z

/AzurePipelines run Azure.sonic-mgmt

azure-pipelines · 2023-05-12T09:20:16Z

Azure Pipelines successfully started running 1 pipeline(s).

tests/bgp/test_bgp_suppress_fib.py

StormLiangMS · 2023-05-23T14:03:32Z

@jcaiMR could you take a look and comment?

jcaiMR · 2023-05-24T09:04:05Z

tests/bgp/bgp_helpers.py

+        bgp_neighbor_ipv6 = duthost.shell(cmd_v6.format(vm_name))['stdout'][1:-1]
+    logging.info("BGP neighbor of {} is {}".format(vm_name, bgp_neighbor_ip))
+    logging.info("IPv6 BGP neighbor of {} is {}".format(vm_name, bgp_neighbor_ipv6))
+


There is already a function to get bgp neighbor ips, something like:
bgp_facts = duthost.bgp_facts()['ansible_facts']
bgp_facts['bgp_neighbors']

Would this method can be use in your case ?

Hi @jcaiMR ,
Checked with this method, it could not list the bgp neighbors under user defined vrf:
(Pdb) duthost.bgp_facts()
Thursday 25 May 2023 12:56:05 +0300 (0:01:45.335) 0:24:46.708 **********
{u'invocation': {u'module_args': {u'instance_id': None, u'num_npus': 1}}, 'failed': False, 'changed': False, '_ansible_no_log': False, u'ansible_facts': {u'bgp_statistics': {u'ipv4_idle': 0, u'ipv6_idle': 0, u'ipv4_admin_down': 0, u'ipv6_admin_down': 0, u'ipv6': 0, u'ipv4': 0}, u'bgp_neighbors': {}}}

c-panther-01# show ip bgp vrf Vrf1 summary

IPv4 Unicast Summary (VRF Vrf1):
BGP router identifier 10.0.0.62, local AS number 65100 vrf-id 417
BGP table version 11346
RIB entries 12853, using 2310 KiB of memory
Peers 48, using 34 MiB of memory
Peer groups 2, using 128 bytes of memory

Neighbor V 10.0.0.1 10.0.0.5 10.0.0.9 10.0.0.13 10.0.0.17 10.0.0.21 10.0.0.25 10.0.0.29 10.0.0.33 10.0.0.35 10.0.0.37 10.0.0.39 10.0.0.41 10.0.0.43 10.0.0.45 10.0.0.47 10.0.0.49 10.0.0.51 10.0.0.53 10.0.0.55 10.0.0.57 10.0.0.59 10.0.0.61 10.0.0.63 fc00::2 fc00::a fc00::12 fc00::1a fc00::22 fc00::2a fc00::32 fc00::3a fc00::42 fc00::46 fc00::4a fc00::4e fc00::52 fc00::56 fc00::5a fc00::5e fc00::62 fc00::66 fc00::6a fc00::6e fc00::72 fc00::76 fc00::7a fc00::7e AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd PfxSnt Desc
4 65200 3262 78 0 0 0 00:03:38 6370 4 ARISTA01T2
4 65200 3262 79 0 0 0 00:03:38 6370 4 ARISTA03T2
4 65200 3262 79 0 0 0 00:03:39 6370 4 ARISTA05T2
4 65200 3262 78 0 0 0 00:03:37 6370 4 ARISTA07T2
4 65200 3262 78 0 0 0 00:03:38 6370 4 ARISTA09T2
4 65200 3262 81 0 0 0 00:03:37 6370 4 ARISTA11T2
4 65200 3262 78 0 0 0 00:03:38 6370 4 ARISTA13T2
4 65200 3262 78 0 0 0 00:03:38 6370 4 ARISTA15T2
4 64001 80 79 0 0 0 00:03:39 4 4 ARISTA01T0
4 64002 78 78 0 0 0 00:03:38 3 4 ARISTA02T0
4 64003 79 78 0 0 0 00:03:38 4 4 ARISTA03T0
4 64004 78 78 0 0 0 00:03:38 3 4 ARISTA04T0
4 64005 78 78 0 0 0 00:03:38 3 4 ARISTA05T0
4 64006 78 79 0 0 0 00:03:39 3 4 ARISTA06T0
4 64007 78 78 0 0 0 00:03:37 3 4 ARISTA07T0
4 64008 78 78 0 0 0 00:03:38 3 4 ARISTA08T0
4 64009 78 78 0 0 0 00:03:38 3 4 ARISTA09T0
4 64010 78 78 0 0 0 00:03:37 3 4 ARISTA10T0
4 64011 78 78 0 0 0 00:03:38 3 4 ARISTA11T0
4 64012 80 78 0 0 0 00:03:38 5 4 ARISTA12T0
4 64013 78 78 0 0 0 00:03:38 3 4 ARISTA13T0
4 64014 78 78 0 0 0 00:03:38 3 4 ARISTA14T0
4 64015 78 78 0 0 0 00:03:38 3 4 ARISTA15T0
4 64016 78 78 0 0 0 00:03:38 3 4 ARISTA16T0
4 65200 3260 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA01T2
4 65200 3263 190 0 0 0 00:03:39 NoNeg NoNeg ARISTA03T2
4 65200 3260 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA05T2
4 65200 3260 129 0 0 0 00:03:31 NoNeg NoNeg ARISTA07T2
4 65200 3260 129 0 0 0 00:03:32 NoNeg NoNeg ARISTA09T2
4 65200 3260 130 0 0 0 00:03:32 NoNeg NoNeg ARISTA11T2
4 65200 3260 129 0 0 0 00:03:31 NoNeg NoNeg ARISTA13T2
4 65200 3262 189 0 0 0 00:03:38 NoNeg NoNeg ARISTA15T2
4 64001 75 123 0 0 0 00:03:28 NoNeg NoNeg ARISTA01T0
4 64002 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA02T0
4 64003 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA03T0
4 64004 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA04T0
4 64005 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA05T0
4 64006 75 123 0 0 0 00:03:28 NoNeg NoNeg ARISTA06T0
4 64007 75 123 0 0 0 00:03:29 NoNeg NoNeg ARISTA07T0
4 64008 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA08T0
4 64009 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA09T0
4 64010 75 123 0 0 0 00:03:29 NoNeg NoNeg ARISTA10T0
4 64011 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA11T0
4 64012 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA12T0
4 64013 75 128 0 0 0 00:03:29 NoNeg NoNeg ARISTA13T0
4 64014 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA14T0
4 64015 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA15T0
4 64016 76 129 0 0 0 00:03:30 NoNeg NoNeg ARISTA16T0

Total number of neighbors 48

I see, thanks for the information.

zhangyanzhao · 2023-05-25T05:40:49Z

This is the last open PR for the feature. Can reviewers help to finish the review? Thanks.

Add BGP suppress fib pending script docs/testplan/BGP-Suppress-FIB-Pending-test-plan.md

StormLiangMS

LGTM

StormLiangMS

LGTM

bingwang-ms · 2023-07-06T23:20:28Z

@echuawu The test case is consistently failing on SN4600 T1 testbed. Did you see the issue on same platform?

Add BGP suppress fib pending script docs/testplan/BGP-Suppress-FIB-Pending-test-plan.md Description of PR Summary: Add a new script to cover BGP suppress fib pending function test. Fixes # (issue) Type of change Bug fix Testbed and Framework(new/improvement) Test case(new/improvement) Back port request 201911 202012 202205 Approach What is the motivation for this PR? This functionality provides feedback to FRR so that routes are not advertised until they are installed in the FIB. Support for bgp suppress-fib-pending s required in SONIC. Supported testbed topology if it's a new test case? T1 topology testbed

nhe-NV previously approved these changes Feb 10, 2023

View reviewed changes

echuawu mentioned this pull request Feb 15, 2023

Add BGP Suppress FIB Pending test plan #7475

Merged

6 tasks

stepanblyschak mentioned this pull request Feb 17, 2023

FIB Suppress Announcements of Routes Not Installed in HW sonic-net/SONiC#1103

Merged

stepanblyschak mentioned this pull request Mar 19, 2023

[fpmsyncd] Implement pending route suppression feature sonic-net/sonic-swss#2551

Merged

stepanblyschak reviewed Mar 24, 2023

View reviewed changes

tests/bgp/test_bgp_suppress_fib.py Outdated Show resolved Hide resolved

echuawu dismissed nhe-NV’s stale review via fe70af7 March 27, 2023 03:48

echuawu force-pushed the bgp_suppress_fib_script branch 2 times, most recently from a3f4cd7 to 068c0e5 Compare March 27, 2023 04:53

echuawu force-pushed the bgp_suppress_fib_script branch from 068c0e5 to 669c75c Compare March 27, 2023 05:52

echuawu force-pushed the bgp_suppress_fib_script branch from d3f0fea to 54bd5c5 Compare May 11, 2023 03:08

StormLiangMS reviewed May 23, 2023

View reviewed changes

tests/bgp/test_bgp_suppress_fib.py Outdated Show resolved Hide resolved

StormLiangMS requested a review from jcaiMR May 23, 2023 14:03

jcaiMR reviewed May 24, 2023

View reviewed changes

BGP suppress fib pending script

43084ef

Add BGP suppress fib pending script docs/testplan/BGP-Suppress-FIB-Pending-test-plan.md

echuawu force-pushed the bgp_suppress_fib_script branch from 54bd5c5 to 43084ef Compare June 2, 2023 05:36

StormLiangMS approved these changes Jun 2, 2023

View reviewed changes

StormLiangMS merged commit fe63723 into sonic-net:master Jun 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BGP suppress fib pending script #7430

BGP suppress fib pending script #7430

echuawu commented Feb 10, 2023 •

edited

Loading

mssonicbld commented Mar 27, 2023

mssonicbld commented Mar 27, 2023

mssonicbld commented Mar 27, 2023

liat-grozovik commented May 10, 2023

echuawu commented May 11, 2023

echuawu commented May 12, 2023

mssonicbld commented May 12, 2023

azure-pipelines bot commented May 12, 2023

echuawu commented May 12, 2023

mssonicbld commented May 12, 2023

azure-pipelines bot commented May 12, 2023

StormLiangMS commented May 23, 2023

jcaiMR May 24, 2023

echuawu May 25, 2023

jcaiMR Jun 2, 2023

zhangyanzhao commented May 25, 2023

StormLiangMS left a comment

StormLiangMS left a comment

bingwang-ms commented Jul 6, 2023

BGP suppress fib pending script #7430

BGP suppress fib pending script #7430

Conversation

echuawu commented Feb 10, 2023 • edited Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

Supported testbed topology if it's a new test case?

mssonicbld commented Mar 27, 2023

mssonicbld commented Mar 27, 2023

mssonicbld commented Mar 27, 2023

liat-grozovik commented May 10, 2023

echuawu commented May 11, 2023

echuawu commented May 12, 2023

mssonicbld commented May 12, 2023

azure-pipelines bot commented May 12, 2023

echuawu commented May 12, 2023

mssonicbld commented May 12, 2023

azure-pipelines bot commented May 12, 2023

StormLiangMS commented May 23, 2023

jcaiMR May 24, 2023

Choose a reason for hiding this comment

echuawu May 25, 2023

Choose a reason for hiding this comment

jcaiMR Jun 2, 2023

Choose a reason for hiding this comment

zhangyanzhao commented May 25, 2023

StormLiangMS left a comment

Choose a reason for hiding this comment

StormLiangMS left a comment

Choose a reason for hiding this comment

bingwang-ms commented Jul 6, 2023

echuawu commented Feb 10, 2023 •

edited

Loading