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

Implementation changes for CiscoBgp4MIB #158

Merged
merged 5 commits into from
Sep 24, 2020

Conversation

SuvarnaMeenakshi
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi commented Sep 1, 2020

- 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
Copy link

lgtm-com bot commented Sep 1, 2020

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:
Copy link
Contributor

@qiluo-msft qiluo-msft Sep 1, 2020

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

@@ -18,8 +17,7 @@
from ax_interface.constants import PduTypes
from sonic_ax_impl.mibs.ietf import rfc4363
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
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
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.

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

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
Copy link

lgtm-com bot commented Sep 8, 2020

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