search

sonic-net / sonic-buildimage

Scripts which perform an installable binary image build for SONiC
Other
736 stars 1.42k forks source link

Bug in frrcfgd in bgp-peer-group-af where command accepts "admin-status" "up" but frrcfgd accepts "true" #18865

Open joolli opened 6 months ago

joolli commented 6 months ago

Description

Steps to reproduce the issue:

  1. Create a bgp-peer-group
  2. Create a bgp-peer-group-af with the knob --admin-status up ==> config bgp-peer-group-af add --admin-status up default UNDERLAY ipv4_unicast

Describe the results you received:

The address family does not get created in FRR config. Verify with -> vtysh -c "show run"

Describe the results you expected:

I expected to see an address-family ipv4 unicast in FRR like so:

 address-family ipv4 unicast
  neighbor UNDERLAY activate
 exit-address-family

Output of show version:

SONiC Software Version: SONiC.202311.537842-1d8111206
SONiC OS Version: 11
Distribution: Debian 11.9
Kernel: 5.10.0-23-2-amd64
Build commit: 1d8111206
Build date: Fri May  3 11:35:30 UTC 2024
Built by: AzDevOps@vmss-soni003LFY

Platform: x86_64-mlnx_msn2700-r0
HwSKU: ACS-MSN2700
ASIC: mellanox
ASIC Count: 1
Serial Number: MT1910X00109
Model Number: MSN2700-CS2F
Hardware Revision: A2
Uptime: 19:02:59 up  3:34,  2 users,  load average: 1.72, 1.60, 1.69
Date: Mon 31 Jul 2023 19:02:59

Docker images:
REPOSITORY                    TAG                       IMAGE ID       SIZE
docker-syncd-mlnx             202311.537842-1d8111206   cc5565facb53   874MB
docker-syncd-mlnx             latest                    cc5565facb53   874MB
docker-platform-monitor       202311.537842-1d8111206   f55d79e0a02d   863MB
docker-platform-monitor       latest                    f55d79e0a02d   863MB
docker-macsec                 latest                    9a143f974dc2   329MB
docker-dhcp-relay             latest                    49d85ed36e49   310MB
docker-eventd                 202311.537842-1d8111206   b43d730fcacb   301MB
docker-eventd                 latest                    b43d730fcacb   301MB
docker-orchagent              202311.537842-1d8111206   def7315f0340   339MB
docker-orchagent              latest                    def7315f0340   339MB
docker-fpm-frr                202311.537842-1d8111206   6ba17e6ea215   359MB
docker-fpm-frr                latest                    6ba17e6ea215   359MB
docker-nat                    202311.537842-1d8111206   1f9a3713fceb   330MB
docker-nat                    latest                    1f9a3713fceb   330MB
docker-sflow                  202311.537842-1d8111206   fb34b34a97fb   328MB
docker-sflow                  latest                    fb34b34a97fb   328MB
docker-teamd                  202311.537842-1d8111206   4b0a59de72fa   327MB
docker-teamd                  latest                    4b0a59de72fa   327MB
docker-snmp                   202311.537842-1d8111206   7610c5f8c5af   340MB
docker-snmp                   latest                    7610c5f8c5af   340MB
docker-router-advertiser      202311.537842-1d8111206   ad72fd6744b0   301MB
docker-router-advertiser      latest                    ad72fd6744b0   301MB
docker-lldp                   202311.537842-1d8111206   67eb7e462c9a   343MB
docker-lldp                   latest                    67eb7e462c9a   343MB
docker-mux                    202311.537842-1d8111206   95e4fe2004f4   349MB
docker-mux                    latest                    95e4fe2004f4   349MB
docker-sonic-gnmi             202311.537842-1d8111206   8c3e356e7c6f   389MB
docker-sonic-gnmi             latest                    8c3e356e7c6f   389MB
docker-database               202311.537842-1d8111206   bdeff783568c   301MB
docker-database               latest                    bdeff783568c   301MB
docker-sonic-mgmt-framework   202311.537842-1d8111206   b852505a02a6   416MB
docker-sonic-mgmt-framework   latest                    b852505a02a6   416MB

Additional info:

Address-family is added to FRR if config_db.json is edited and "up" is changed to "true" and the config is reloaded. However, "config" command stops working because the current configuration can't be validated anymore, as it expects "up".

   "BGP_PEER_GROUP_AF": {
        "default|UNDERLAY|ipv4_unicast": {
            "admin_status": "up",
        }
    },

changed to:

   "BGP_PEER_GROUP_AF": {
        "default|UNDERLAY|ipv4_unicast": {
            "admin_status": "true",
        }
    },

and the config loaded.

The below output shows the log entries in /var/log/frr/zebra.log when the command: config bgp-peer-group-af add --admin-status up default UNDERLAY ipv4_unicast is executed.


Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd:  BGP table handling
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: table : BGP_PEER_GROUP_AF
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: key   : default|UNDERLAY|ipv4_unicast
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: op    : SET
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: data  :
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd:         admin_status - up
Jul 31 18:43:35.867771 sonic-3 DEBUG bgp#frrcfgd: 
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: value for table BGP_PEER_GROUP_AF prefix default key UNDERLAY|ipv4_unicast changed to {'admin_status': (up, ADD)}
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: Set address family for neighbor UNDERLAY to ipv4 unicast
Jul 31 18:43:35.869133 sonic-3 ERR bgp#frrcfgd: Input token up is neither true or false for cmd: {no:no-prefix}neighbor {} activate
Jul 31 18:43:35.869133 sonic-3 ERR bgp#frrcfgd: failed to get upd cmd from value: ('up',)
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: Add admin_status data up to cache
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd:  BGP table handling
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: table : BGP_PEER_GROUP_AF
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: key   : default|UNDERLAY|ipv4_unicast
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: op    : SET
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: data  :
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd:         admin_status - up
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: 
Jul 31 18:43:35.873225 sonic-3 INFO bgp#frrcfgd: value for table BGP_PEER_GROUP_AF prefix default key UNDERLAY|ipv4_unicast changed to {'admin_status': (up, NONE)}
Jul 31 18:43:35.873225 sonic-3 INFO bgp#frrcfgd: Set address family for neighbor UNDERLAY to ipv4 unicast
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: ignore cache update for admin_status because of NO_OP 
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: value for table BGP_PEER_GROUP_AF prefix default key UNDERLAY|ipv4_unicast changed to {'admin_status': (up, ADD)}
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: Set address family for neighbor UNDERLAY to ipv4 unicast
Jul 31 18:43:35.869133 sonic-3 ERR bgp#frrcfgd: Input token up is neither true or false for cmd: {no:no-prefix}neighbor {} activate
Jul 31 18:43:35.869133 sonic-3 ERR bgp#frrcfgd: failed to get upd cmd from value: ('up',)
Jul 31 18:43:35.869133 sonic-3 INFO bgp#frrcfgd: Add admin_status data up to cache
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd:  BGP table handling
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: ----------------------------------
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: table : BGP_PEER_GROUP_AF
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: key   : default|UNDERLAY|ipv4_unicast
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: op    : SET
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: data  :
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd:         admin_status - up
Jul 31 18:43:35.873225 sonic-3 DEBUG bgp#frrcfgd: 
Jul 31 18:43:35.873225 sonic-3 INFO bgp#frrcfgd: value for table BGP_PEER_GROUP_AF prefix default key UNDERLAY|ipv4_unicast changed to {'admin_status': (up, NONE)}
g-balaji1 commented 5 months ago

I am working on this bug.

joolli commented 5 months ago

Just let me know if I can be of help.

g-balaji1 commented 4 months ago

@joolli Thanks, can you provide the CLI commands (the sonic CLI config bgp commands) to simulate the issue. I dont see this command "config bgp-peer-group-af add --admin-status up default UNDERLAY ipv4_unicast"

I have loaded the following

admin@sonic:~$ show ver

SONiC Software Version: SONiC.master.563622-9cbaf42f1 SONiC OS Version: 12 Distribution: Debian 12.5 Kernel: 6.1.0-11-2-amd64 Build commit: 9cbaf42f1 Build date: Tue Jun 4 13:59:08 UTC 2024 Built by: cloudtest@2258ec2bc00000F

Platform: x86_64-kvm_x86_64-r0 HwSKU: Force10-S6000 ASIC: vs ASIC Count: 1 Serial Number: N/A Model Number: N/A Hardware Revision: N/A Uptime: 18:10:09 up 2:02, 2 users, load average: 2.92, 1.85, 1.25 Date: Mon 10 Jun 2024 18:10:09

joolli commented 4 months ago

Hi, sorry for the late response. I will be quicker next time

Yes, you need to load the yang model: sonic-cli-gen generate config sonic-bgp-neighbor sonic-cli-gen generate config sonic-bgp-peergroup sonic-cli-gen generate show sonic-bgp-neighbor sonic-cli-gen generate show sonic-bgp-peergroup

You can find the yang models under /usr/local/yang

g-balaji1 commented 4 months ago

Sorry for the delay in replying, i tried the above commands the python files got generated for each command respectively. The peer-group creation fails and then the peer-group should be activated under ipv4_unicast AF. Tried creating a peer group in default VRF and i got the following error. Kindly let me know if am missing something.

root@sonic:~# config bgp-peer-group add default New
libyang[0]: Leafref "/sonic-bgp-global:sonic-bgp-global/sonic-bgp-global:BGP_GLOBALS/sonic-bgp-global:BGP_GLOBALS_LIST/sonic-bgp-global:vrf_name" of value "default" points to a non-existing leaf. (path: /sonic-bgp-peergroup:sonic-bgp-peergroup/BGP_PEER_GROUP/BGP_PEER_GROUP_LIST[vrf_name='default'][peer_group_name='New']/vrf_name)
libyang[0]: Leafref "/sonic-bgp-global:sonic-bgp-global/sonic-bgp-global:BGP_GLOBALS/sonic-bgp-global:BGP_GLOBALS_LIST/sonic-bgp-global:vrf_name" of value "default" points to a non-existing leaf. (path: /sonic-bgp-peergroup:sonic-bgp-peergroup/BGP_PEER_GROUP/BGP_PEER_GROUP_LIST[vrf_name='default'][peer_group_name='New']/vrf_name)
sonic_yang(3):Data Loading Failed:Leafref "/sonic-bgp-global:sonic-bgp-global/sonic-bgp-global:BGP_GLOBALS/sonic-bgp-global:BGP_GLOBALS_LIST/sonic-bgp-global:vrf_name" of value "default" points to a non-existing leaf.
Error: Failed to validate configuration: Data Loading Failed
Leafref "/sonic-bgp-global:sonic-bgp-global/sonic-bgp-global:BGP_GLOBALS/sonic-bgp-global:BGP_GLOBALS_LIST/sonic-bgp-global:vrf_name" of value "default" points to a non-existing leaf.
Aborted!
root@sonic:~#
root@sonic:~# show bgp-peer-group
VRF NAME    PEER GROUP NAME    SONIC BGP CMN
----------  -----------------  ---------------
root@sonic:~#
root@sonic:~# show vrf
VRF    Interfaces
-----  ------------
root@sonic:~#
joolli commented 4 months ago

You also need to load sonic-bgp-global yang model it seems:

sudo sonic-cli-gen generate config sonic-bgp-global sudo config bgp-globals add --router-id 10.1.1.1 --local-asn 65001 default sudo config bgp-peer-group add default New --local-asn 65001 --peer-type internal

johndexter1 commented 4 months ago

Hey, sorry for off topic, but I'm trying to reproduce @joolli issue and seems that loaded yang modules does not work for me.

As example I load sonic-bgp-global sonic-cli-gen generate config sonic-bgp-global

Then try to execute sudo config bgp-globals add --router-id 10.1.1.1 --local-asn 65001 default

But I cannot see the configuration present when checking through vtysh -c 'show runn'

Although the configuration is visible in config_db.json

And same happens with, as example, bgp-neighbor

What I am missing here? Do I need to enable something else?

joolli commented 4 months ago

You need to set the following under DEVICE_METADATA

        "docker_routing_config_mode": "split-unified",
        "frr_mgmt_framework_config": "true",

The value of "unified" for docker routing config mode might also work, but I haven't tried it.

joolli commented 2 months ago

https://github.com/sonic-net/sonic-buildimage/commit/7502dc8fbf7bf6e79859483696ffff8c99d6b882

After looking over the code I made this tiny change and it seems to fix the problem. I'm no programmer though.