Multi-asic changes for config bgp commands and utilities.#910
Multi-asic changes for config bgp commands and utilities.#910judyjoseph merged 4 commits intosonic-net:masterfrom
Conversation
|
LGTM. Wait for Guohan and Pavel to review. |
|
retest this please. |
…icitly in BGP commands so that existing single ASIC platform implementation is unchanged.
|
Retest this please |
pavel-shirshov
left a comment
pavel-shirshov
left a comment
There was a problem hiding this comment.
I left some comments. Can you please check?
| def _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts=False): | ||
| """Returns list of strings containing IP addresses of all BGP neighbors | ||
| if the flag ignore_local_hosts is set to True, additional check to see if | ||
| if the BGP neighbor AS number is same as local BGP AS number, if so ignore that neigbor. |
There was a problem hiding this comment.
so the name of the function is missleading, because you're filtering out peer with local bgp AS?
Also, we could have dynamic bgp neighbors and bgp_monitor entries.
Should we include them here?
There was a problem hiding this comment.
This was an existing function, to which I added a new parameter ignore_local_hosts to do bgp AS based filtering. I will change the name of the function.
Currently all the functions look in the BGP_NEIGHBOR table alone, we need change in all API's to check the other BGP tables as well. BGP DYNAMIC PEERS and BGP MONITORS. We could do it separately in a different PR if that is ok
|
|
||
| if not ip_addrs: | ||
| click.get_current_context().fail("Could not locate neighbor '{}'".format(ipaddr_or_hostname)) | ||
| return False |
There was a problem hiding this comment.
why the error message was removed?
There was a problem hiding this comment.
These error message are not removed, the "same" message is now moved into the bgp command handlers ( eg: https://github.com/Azure/sonic-utilities/blob/33899cfe36018b8c6b03e9dbd125dfe117c8b56a/config/main.py#L1709 ). This is because for a specific neighbor shut/start case in multi-asic platform, we check in all bgp instances if the neighbor exists before declaring "not exist" with the ERROR message.
* Multi-asic changes for config bgp commands and utilities. * Review comments update * Optimized the logic of checking the internal hosts using AS number. * API changes due to rebase. Also setting ignore_local_hosts=false explicitly in BGP commands so that existing single ASIC platform implementation is unchanged.
) * Multi-asic changes for config bgp commands and utilities. * Review comments update * Optimized the logic of checking the internal hosts using AS number. * API changes due to rebase. Also setting ignore_local_hosts=false explicitly in BGP commands so that existing single ASIC platform implementation is unchanged.
Revert "[config] Add 'interface transceiver' subgroup with 'lpmode' and 'reset' subcommands (sonic-net#904)" Multi-asic changes for config bgp commands and utilities. (sonic-net#910)
5 participants
- What I did
Changes to config bgp commands for Multi-ASIC devices.
- How I did it
The changes done here are based on the namespace support added in SonicDBConfig/ConfigDBConnector/SonicV2Connector classes in the PR sonic-net/sonic-py-swsssdk#63
The following are the changes introduced in this PR
(a) added a new parameter of config_db to the bgp neighbor helper API's. This approach is better, as now we have to do this once config_db = ConfigDBConnector() in the CLI handler and the config_db instance is passed down as argument to the API.
(b) Modified the API _get_all_neighbor_ipaddresses() to accept an additional parameter ignore_hosts if there are any hosts to be ignored when forming the ip_addr neighbor list.
(c) config bgp commands : In multi-ASIC devices we have both internal BGP sessions between the bgp dockers running in different namespaces and EBGP sessions with external neighbors.
- How to verify it
Verified on a multi-ASIC devices, by running the Config commands.
- Previous command output (if the output of a command-line utility has changed)
- New command output (if the output of a command-line utility has changed)