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

NEIGH_STATE_TABLE in STATE_DB is possibly overwriten #6547

Closed
bingwang-ms opened this issue Jan 25, 2021 · 1 comment · Fixed by #6546
Closed

NEIGH_STATE_TABLE in STATE_DB is possibly overwriten #6547

bingwang-ms opened this issue Jan 25, 2021 · 1 comment · Fixed by #6546
Assignees

Comments

@bingwang-ms
Copy link
Contributor

Description

It is possible that BGP neighbors in IPv4 and IPv6 address families share the same name.
For example, if belowing config is applied to creare a bgp monitor:

{
    "BGP_MONITORS": {
        "11.0.0.1": {
            "admin_status": "up",
                "asn": "4200065100",
                "holdtime": "10",
                "keepalive": "3",
                "local_addr": "10.1.0.32",
                "name": "bgp_monitor",
                "nhopself": "0",
                "rrclient": "0"
        }
    }
}

Then a bgp neighbor named 11.0.0.1 will exist in both ipv4 and ipv6 neighbors.

root@str-7260cx3-acs-2:/var/log# show ip bgp sum

IPv4 Unicast Summary:
......

Neighbhor      V          AS    MsgRcvd    MsgSent    TblVer    InQ    OutQ  Up/Down    State/PfxRcd    NeighborName
-----------  ---  ----------  ---------  ---------  --------  -----  ------  ---------  --------------  --------------
......
11.0.0.1       4  4200065100          0          0         0      0       0  never      Active          bgp_monitor

Total number of neighbors 5
root@str-7260cx3-acs-2:/var/log# show ipv6 bgp sum

IPv6 Unicast Summary:
......


Neighbhor      V          AS    MsgRcvd    MsgSent    TblVer    InQ    OutQ  Up/Down    State/PfxRcd    NeighborName
-----------  ---  ----------  ---------  ---------  --------  -----  ------  ---------  --------------  --------------
11.0.0.1       4  4200065100          0          0         0      0       0  never      Active          bgp_monitor
......

Total number of neighbors 5

The NEIGH_STATE_TABLE is updated in bgpmon.py, which uses NEIGH_STATE_TABLE|x.x.x.x as keys in STATE_DB.

root@str-7260cx3-acs-2:~# redis-cli -n 6 keys "NEIGH_STATE_TABLE|*"
1) "NEIGH_STATE_TABLE|fc00::2e"
2) "NEIGH_STATE_TABLE|10.0.0.33"
3) "NEIGH_STATE_TABLE|10.0.0.37"
4) "NEIGH_STATE_TABLE|10.0.0.35"
5) "NEIGH_STATE_TABLE|fc00::2a"
6) "NEIGH_STATE_TABLE|fc00::26"
7) "NEIGH_STATE_TABLE|10.0.0.39"
8) "NEIGH_STATE_TABLE|fc00::22"

If ipv4 and ipv6 neighbors have duplicated keys, the value will be overwriten.

Steps to reproduce the issue:

  1. Run test_bgpmon

Describe the results you received:
NEIGH_STATE_TABLE in STATE_DB is possibly overwriten

Describe the results you expected:

Additional information you deem important (e.g. issue happens only occasionally):

**Output of `show version`:**
SONiC Software Version: SONiC.HEAD.289-abb01394
Distribution: Debian 10.7
Kernel: 4.19.0-9-2-amd64
Build commit: abb01394
Build date: Wed Jan 20 20:29:01 UTC 2021
Built by: johnar@jenkins-worker-22

Platform: x86_64-arista_7260cx3_64
HwSKU: Arista-7260CX3-D108C8
ASIC: broadcom
ASIC Count: 1
Serial Number: SSJ17432414
Uptime: 08:09:31 up  7:33,  3 users,  load average: 4.60, 3.13, 3.02

Docker images:
REPOSITORY                    TAG                 IMAGE ID            SIZE
docker-syncd-brcm             HEAD.289-abb01394   3e626b50848d        640MB
docker-syncd-brcm             latest              3e626b50848d        640MB
docker-snmp                   HEAD.289-abb01394   36f4b680e640        434MB
docker-snmp                   latest              36f4b680e640        434MB
docker-teamd                  HEAD.289-abb01394   a4a5c8951271        404MB
docker-teamd                  latest              a4a5c8951271        404MB
docker-sonic-mgmt-framework   HEAD.289-abb01394   76c1abe9ee58        609MB
docker-sonic-mgmt-framework   latest              76c1abe9ee58        609MB
docker-router-advertiser      HEAD.289-abb01394   30a2754fe3dd        394MB
docker-router-advertiser      latest              30a2754fe3dd        394MB
docker-platform-monitor       HEAD.289-abb01394   7b6c43e14d73        601MB
docker-platform-monitor       latest              7b6c43e14d73        601MB
docker-macsec                 HEAD.289-abb01394   5f1a5e85427c        407MB
docker-macsec                 latest              5f1a5e85427c        407MB
docker-lldp                   HEAD.289-abb01394   f1348fb11a36        434MB
docker-lldp                   latest              f1348fb11a36        434MB
docker-dhcp-relay             HEAD.289-abb01394   941ab107cf5d        401MB
docker-dhcp-relay             latest              941ab107cf5d        401MB
docker-database               HEAD.289-abb01394   ca95eb123e3d        394MB
docker-database               latest              ca95eb123e3d        394MB
docker-orchagent              HEAD.289-abb01394   8a02c9dd224a        422MB
docker-orchagent              latest              8a02c9dd224a        422MB
docker-nat                    HEAD.289-abb01394   4772183f4f29        407MB
docker-nat                    latest              4772183f4f29        407MB
docker-sonic-telemetry        HEAD.289-abb01394   fdf0a243f2ca        468MB
docker-sonic-telemetry        latest              fdf0a243f2ca        468MB
docker-sflow                  HEAD.289-abb01394   2504380e5d6d        405MB
docker-sflow                  latest              2504380e5d6d        405MB
docker-fpm-frr                HEAD.289-abb01394   563e23fe3b69        422MB
docker-fpm-frr                latest              563e23fe3b69        422MB
**Attach debug file `sudo generate_dump`:**

```
(paste your output here)
```
@gechiang
Copy link
Collaborator

With the new fix in place to address the duplicate entry by converting the peer list to a peer set:
#6546 (review)
This issue should already been addressed.

Marking this issue is fixed by (#6546 (review))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants