Skip to content

Implementation changes for CiscoBgp4MIB - #158

Merged
SuvarnaMeenakshi merged 5 commits into
sonic-net:masterfrom
SuvarnaMeenakshi:bgp
Sep 24, 2020
Merged

SuvarnaMeenakshi merged 5 commits into
sonic-net:masterfrom
SuvarnaMeenakshi:bgp

Conversation

@SuvarnaMeenakshi

@SuvarnaMeenakshi SuvarnaMeenakshi commented Sep 1, 2020 •

Copy link
Copy Markdown
Contributor

- What I did
Currently the data required for CiscoBgp4MIB is retrieved from bgpd deamon. snmp_ax_impl connects to bgpd daemon via tcp socket and retreives the BGP neighbor information required for CiscoBgp4MIB. This design is modified to support multi-asic platform. The data required by CiscoBgp4MIB can be populated in STATE_DB by a new daemon in BGP docker.
This PR captures changes required in snmp_ax_impl to get data from STATE_DB.
This PR has dependency on the below PRs:
sonic-net/sonic-buildimage#5329
sonic-net/sonic-buildimage#5436
- How I did it
Changes made so that snmp_ax_impl can talk to STATE_DB and retrieve NEIGH_STATE_TABLE. This change will affect both single and multi asic platforms. In case of multi-asic platforms, snmp_ax_impl will retrive data from STATE_DB of all namespaces.

- How to verify it
Bring up single asic platform and multi-asic platforms with the changes, the output of iso.3.6.1.4.1.9.9.187.
Verify if the output matches with the information in STATE_DB NEIGH_STATE_TABLE.

- Description for the changelog

STATE_DB instead of getting data from vtysh.
Update unit-test mock table to add new table with BGP Neighbour
information.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
Sort oid list.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
Remove usage of mock vtysh socket and vtysh dummy output.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
not required for CiscoBgp4MIB.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
@lgtm-com

lgtm-com Bot commented Sep 1, 2020

Copy link
Copy Markdown

This pull request fixes 1 alert when merging ade4e38 into 1a2b62a - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable

neigh_str = neigh_key.decode()
neigh_str = neigh_str.split('|')[1]
neigh_info = self.db_conn[db_index].get_all(mibs.STATE_DB, neigh_key, blocking=False)
if neigh_info is None:

@qiluo-msft qiluo-msft Sep 1, 2020 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if neigh_info is None:
if neigh_info:
``` #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Update the condition check/

Comment thread tests/test_bgp.py
from sonic_ax_impl.main import SonicMIB
from sonic_ax_impl.lib.quaggaclient import parse_bgp_summary
from mock_tables.socket import MockGetHostname
from sonic_ax_impl.mibs.vendor.cisco.bgp4 import CiscoBgp4MIB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CiscoBgp4MIB [](start = 49, length = 12)

Not used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

quaggaclient and mock tables are not required anymore.

@qiluo-msft qiluo-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As comments

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
@SuvarnaMeenakshi
SuvarnaMeenakshi marked this pull request as ready for review September 8, 2020 14:58
@lgtm-com

lgtm-com Bot commented Sep 8, 2020

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 99f3b76 into 1a2b62a - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable

@SuvarnaMeenakshi
SuvarnaMeenakshi merged commit 51b09a8 into sonic-net:master Sep 24, 2020
abdosi pushed a commit that referenced this pull request Sep 28, 2020
* [BGP]:  Currently the data required for CiscoBgp4MIB is retrieved from bgpd deamon. snmp_ax_impl connects to bgpd daemon via tcp socket and retreives the BGP neighbor information required for CiscoBgp4MIB. This design is modified to support multi-asic platform. The data required by CiscoBgp4MIB can be populated in STATE_DB by a new daemon in BGP docker. Changes made:
- snmp_ax_impl to retrieve NEIGH_STATE_TABLE from STATE_DB. This change will affect both single and multi asic platforms. 
- Update bgp MIB unit tests to use STATE_DB data instead of using vtysh socket.
ssithaia-ebay pushed a commit to ssithaia-ebay/sonic-snmpagent that referenced this pull request May 23, 2025
* [BGP]:  Currently the data required for CiscoBgp4MIB is retrieved from bgpd deamon. snmp_ax_impl connects to bgpd daemon via tcp socket and retreives the BGP neighbor information required for CiscoBgp4MIB. This design is modified to support multi-asic platform. The data required by CiscoBgp4MIB can be populated in STATE_DB by a new daemon in BGP docker. Changes made:
- snmp_ax_impl to retrieve NEIGH_STATE_TABLE from STATE_DB. This change will affect both single and multi asic platforms. 
- Update bgp MIB unit tests to use STATE_DB data instead of using vtysh socket.
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.

3 participants