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] BGP monitor changes for multi asic #6920

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

arlakshm
Copy link
Contributor

@arlakshm arlakshm commented Mar 1, 2021

Why I did it

Add support for BGP Monitors on multi asic SONiC platforms.

How I did it

On multi ASIC SONiC platforms, BGP monitor session will be established from Backend ASIC.
To achieve this following changes are done

  • Add BGP monitor configuration on the backend ASIC.
    The BGP monitor configuration is present in the DPG of the device in minigraph.xml of multi-ASIC device, so this configuration will be added to the config_db of the host, when the minigraph is loaded.
    To add configuration for this in the Backend ASIC, a new class MultiAsicBgpMonCfg is added to the hostcfgd service to update the config_db of the backend ASIC when the BGP_MONITOR table of the host config_db is updated.
    This way incremental BGP_MONITOR configuration can also be handled.

  • Changes to establish BGP session with bgp monitor.

    • Add route in host main routing table to go to one of pre-define backend asic
    • Add IP table rule on front asic to mark the BGP packets with destination as IPv4 Loopback.
    • Add IP rule in front asic namespace to match mark BGP packet and lookup default table
    • Program the default route in FrontEnd asic name space docker default table as part of start.sh of the BGP container.
      It need to be done as part of start.sh otherwise FRR default route will get over-written.

How to verify it

Run the tests test_bgpmon.py for multi asic.

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

  • 201811
  • [X ] 201911
  • 202006
  • [X ] 202012

Description for the changelog

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

arlakshm and others added 3 commits February 11, 2021 20:04
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
to make BGPMON work for multi-asic platforms.

Signed-off-by: Abhishek Dosi <[email protected]>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@arlakshm arlakshm requested a review from lguohan as a code owner March 1, 2021 02:24
@arlakshm arlakshm requested review from abdosi and rlhui March 1, 2021 02:24
@abdosi
Copy link
Contributor

abdosi commented Mar 2, 2021

@prsunny made some changes for IPTable class to make it generic so can be use for other rules also. please check.

@abdosi
Copy link
Contributor

abdosi commented Mar 5, 2021

@arlakshm Config DB programming part looks good. Can you review the ip table rule programming part.

@arlakshm
Copy link
Contributor Author

arlakshm commented Mar 5, 2021

@abdosi, the ip table programming looks good to me.

abdosi
abdosi previously approved these changes Mar 5, 2021
rlhui
rlhui previously approved these changes Mar 5, 2021
from sonic_py_common import device_info
from swsssdk import ConfigDBConnector
from sonic_py_common import device_info,multi_asic
from swsssdk import ConfigDBConnector, SonicDBConfig
Copy link
Collaborator

Choose a reason for hiding this comment

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

for master we no longer use swsssdk, it is depracating. @qiluo-msft

Copy link
Collaborator

Choose a reason for hiding this comment

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

@qiluo-msft , are you going to refactor this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@arlakshm Could you just replace swsssdk with swsscommon.swsscommon ? we are deprecating swsssdk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change in latest commit

@lguohan
Copy link
Collaborator

lguohan commented Mar 5, 2021

others looks good.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@arlakshm arlakshm dismissed stale reviews from rlhui and abdosi via 1b5d811 March 5, 2021 22:31
qiluo-msft
qiluo-msft previously approved these changes Mar 5, 2021
@@ -406,6 +512,9 @@ class HostConfigDaemon:
self.kdumpCfg = KdumpCfg(self.config_db)
self.kdumpCfg.load(self.config_db.get_table('KDUMP'))

Copy link
Contributor

Choose a reason for hiding this comment

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

@arlakshm
we need to do self.masicBgpMonCfg = None .
SIngle asic is getting exception.

Please update.

Copy link
Contributor

Choose a reason for hiding this comment

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

@arlakshm updated in latest commit. Please check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abdosi, lgtm.

Signed-off-by: Abhishek Dosi <[email protected]>
abdosi
abdosi previously approved these changes Mar 6, 2021
abdosi pushed a commit to sonic-net/sonic-utilities that referenced this pull request Mar 6, 2021
The sonic-net/sonic-buildimage#6920 adds support of adding BGP monitor session on one of the backend ASICs of Multi ASIC platforms.
This PR enchances the show ip bgp summary command to display this session

Also added unit tests for 'show ip bgp summary' for multi asic platforms
@abdosi
Copy link
Contributor

abdosi commented Mar 6, 2021

@arlakshm we need to monitor KVM-T0 test failure. I have re-run couple of time but it is failing. Have triggered new re-run.

Meanwhile to get unblock created PR for 201911 #6977. Please review.

abdosi added a commit to abdosi/sonic-buildimage that referenced this pull request Mar 6, 2021
This PRis cherry-pick of master
sonic-net#6920

Signed-off-by: Abhishek Dosi <[email protected]>
Co-authored-by: Arvind <[email protected]>
@arlakshm
Copy link
Contributor Author

arlakshm commented Mar 6, 2021

@arlakshm we need to monitor KVM-T0 test failure. I have re-run couple of time but it is failing. Have triggered new re-run.

Meanwhile to get unblock created PR for 201911 #6977. Please review.

@abdosi, The T0 failures doesn't seem to related to the changes in the PR. I have retrigger again, I will monitor the tests

abdosi added a commit that referenced this pull request Mar 7, 2021
This PR is cherry-pick of master
#6920

Why I did it
Add support for BGP Monitors on multi asic SONiC platforms.

How I did it
On multi ASIC SONiC platforms, BGP monitor session will be established from Backend ASIC.
To achieve this following changes are done

Add BGP monitor configuration on the backend ASIC.
The BGP monitor configuration is present in the DPG of the device in minigraph.xml of multi-ASIC device, so this configuration will be added to the config_db of the host, when the minigraph is loaded.
To add configuration for this in the Backend ASIC, a new class MultiAsicBgpMonCfg is added to the hostcfgd service to update the config_db of the backend ASIC when the BGP_MONITOR table of the host config_db is updated.
This way incremental BGP_MONITOR configuration can also be handled.

Changes to establish BGP session with bgp monitor.

Add route in host main routing table to go to one of pre-define backend asic
Add IP table rule on front asic to mark the BGP packets with destination as IPv4 Loopback.
Add IP rule in front asic namespace to match mark BGP packet and lookup default table
Program the default route in FrontEnd asic name space docker default table as part of start.sh of the BGP container.
It need to be done as part of start.sh otherwise FRR default route will get over-written.
How to verify it

Signed-off-by: Abhishek Dosi <[email protected]>
Co-authored-by: Arvind <[email protected]>
@arlakshm
Copy link
Contributor Author

arlakshm commented Mar 8, 2021

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@arlakshm arlakshm requested a review from xumia as a code owner April 1, 2021 19:32
@lgtm-com
Copy link

lgtm-com bot commented Apr 1, 2021

This pull request introduces 1 alert when merging 43dc774 into aa63c71 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

qiluo-msft pushed a commit to sonic-net/sonic-swss-common that referenced this pull request Apr 29, 2021
The PR sonic-net/sonic-buildimage#6920 adds the support for swsscommon in `hostcfgd`. The PR test failure are seen because the `config_db.subscribe` doesnt work with swsscommon. This PR adds the support for the following python APIs
 - `config_db.subscribe`
 - `config_db.unsubscribe`
@arlakshm
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

qiluo-msft pushed a commit to sonic-net/sonic-swss-common that referenced this pull request Sep 14, 2021
The PR sonic-net/sonic-buildimage#6920 adds the support for swsscommon in `hostcfgd`. The PR test failure are seen because the `config_db.subscribe` doesnt work with swsscommon. This PR adds the support for the following python APIs
 - `config_db.subscribe`
 - `config_db.unsubscribe`
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.

6 participants