[GCU] bgp speaker rollback failure

[wen587]

Description

SONiC doesn't allow two bgp speaker have the same listen range. But it might happen during the GCU.
Let's say SPEAKER_CONFIG_A has IP_RANGE_A. We want to replace it with SPEAKER_CONFIG_B with IP_RANGE_A.
If the GCU sorting algorithm give out this patch order:
{op_1: add SPEAKER_CONFIG_B; op_2: remove SPEAKER_CONFIG_A}
Then after the op_1, there are two config share the same ip range, which result in the failure.

Steps to reproduce the issue

1. Env: kvm-t0.
   original speaker config:

admin@vlab-01:~/speaker/test$ sudo config checkpoint init

{
"BGP_PEER_RANGE": {
        "BGPSLBPassive": {
            "ip_range": [
                "10.255.0.0/25"
            ],
            "name": "BGPSLBPassive",
            "src_address": "10.1.0.32"
        },
        "BGPVac": {
            "ip_range": [
                "192.168.0.0/21"
            ],
            "name": "BGPVac",
            "src_address": "10.1.0.32"
        }
    }
}

2. Apply test config to speaker

admin@vlab-01:~/speaker/test$ cat tc_add.json
[
 {
  "op": "add",
  "path": "/BGP_PEER_RANGE",
  "value": {
   "BGPSLBPassive": {
    "ip_range": [
     "192.168.0.0/21"
    ],
    "name": "BGPSLBPassive",
    "src_address": "10.1.0.32"
   },
   "BGPSLBPassiveV6": {
    "ip_range": [
     "fc02:1000::/64"
    ],
    "name": "BGPSLBPassiveV6",
    "src_address": "fc00:1::32"
   }
  }
 }
]
admin@vlab-01:~/speaker/test$ sudo config apply-patch tc_add.json
...
Patch Applier: Applying 4 changes in order:
Patch Applier:   * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPVac"}]
Patch Applier:   * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPSLBPassiveV6", "value": {"ip_range": ["fc02:1000::/64"], "name": "BGPSLBPassiveV6", "src_address": "fc00:1::32"}}]
Patch Applier:   * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassive"}]
Patch Applier:   * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPSLBPassive", "value": {"ip_range": ["192.168.0.0/21"], "name": "BGPSLBPassive", "src_address": "10.1.0.32"}}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.

Config after the patch:

{
  "BGP_PEER_RANGE": {
        "BGPSLBPassive": {
            "ip_range": [
                "192.168.0.0/21"
            ],
            "name": "BGPSLBPassive",
            "src_address": "10.1.0.32"
        },
        "BGPSLBPassiveV6": {
            "ip_range": [
                "fc02:1000::/64"
            ],
            "name": "BGPSLBPassiveV6",
            "src_address": "fc00:1::32"
        }
    }
}

3. rollback to init config and check syslog err

admin@vlab-01:~/speaker/test$ sudo config rollback init
Patch Applier: Applying 4 changes in order:
Patch Applier:   * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassiveV6"}]

========Below "add" is ahead of "remove" operation results in two same ip range occur at the same time, which is incorrect======

Patch Applier:   * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPVac", "value": {"ip_range": ["192.168.0.0/21"], "name": "BGPVac", "src_address": "10.1.0.32"}}]
Patch Applier:   * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassive"}]
Patch Applier:   * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPSLBPassive", "value": {"ip_range": ["10.255.0.0/25"], "name": "BGPSLBPassive", "src_address": "10.1.0.32"}}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Config Replacer: Verifying config replacement is reflected on ConfigDB.
Config Replacer: Config replacement completed.
Config Rollbacker: Config rollbacking completed.
Config rolled back successfully.

admin@vlab-01:~$ show logging -f | grep ERR
...
Mar 28 07:45:54.686979 vlab-01 ERR bgp#bgpcfgd: command execution returned 13. Command: '['vtysh', '-f', '/tmp/tmp5d_n7k5w']', stdout: '% Same listen range is attached to peer-group BGPSLBPassive#012', stderr: 'line 37: Failure to communicate[13] to bgpd, line:   bgp listen range 192.168.0.0/21 peer-group BGPVac#012#012'
Mar 28 07:45:54.686979 vlab-01 ERR bgp#bgpcfgd: ConfigMgr::commit(): can't push configuration from file='/tmp/tmp5d_n7k5w', rc='13', stdout='% Same listen range is attached to peer-group BGPSLBPassive#012', stderr='line 37: Failure to communicate[13] to bgpd, line:   bgp listen range 192.168.0.0/21 peer-group BGPVac#012#012'

Describe the results you received

Describe the results you expected

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

Output of show version

(paste your output here)

[ghooo]

This is a YANG validation issue, it is not correctly validating the ip_ranges are different. We need to create an issue to fix the yang. If the YANG issue will take a long time to fix, we can hard-code the fix in GCU similar to what we did for the lanes validator, and remove the hard-coding once yang model validation isupdated.

[wen587]

Link yang issue here.
sonic-net/sonic-buildimage#10376

[ghooo]

BGPD code that throws the error: https://github.com/Azure/sonic-frr/blob/c69608a68083d1017257977bd0260bebdb12322f/bgpd/bgp_vty.c#L3498-L3507