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

[DPB] BGP_NEIGHBOR yang model causing DPB errors #7959

Closed
alexrallen opened this issue Jun 23, 2021 · 5 comments
Closed

[DPB] BGP_NEIGHBOR yang model causing DPB errors #7959

alexrallen opened this issue Jun 23, 2021 · 5 comments
Labels
Bug 🐛 Port Breakout 🔱 YANG YANG model related changes

Comments

@alexrallen
Copy link
Contributor

alexrallen commented Jun 23, 2021

#6917 introduced a model for BGP_NEIGHBOR in Yang. However:

  1. It does not support defining a neighbor without an explicit VRF
  2. It does not support any fields inside a BGP_NEIGHBOR entry in CONFIG_DB such as asn or holdtime.

Defining Neighbor without VRF

This model expects the neighbors to be defined as

   "BGP_NEIGHBOR": {
        "default|10.0.0.1": {}
   }

where default is the VRF name and does not support defining the neighbor without this. This is because the Yang model defines the vrf name as part of the key for BGP neighbor entries.

Steps to Reproduce

  1. Install latest master on switch
  2. Disable ZTP via ztp disable -y
  3. Update the config if neccesary via /usr/local/bin/sonic-cfggen -H -k [SKU] --write-to-db
  4. Edit config_db to include the following minimal BGP Neighbor config
   "BGP_NEIGHBOR": {
        "10.0.0.1": {}
   }
  1. Run config reload -y
  2. Attempt to break out any port

Observed Results

Breakout fails with the following error....

sonic_yang(3):All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['10.0.0.1'])
sonic_yang(3):exceptionList:[]

Expected Results

Successful Breakout

Does not support fields in neighbor entry

This model also does not support any fields being in the neighbor entry. For example..

   "BGP_NEIGHBOR": {
        "default|10.0.0.1": {
            "asn": "65000"
        }
   }

This will throw an error that the asn key is not matched. This functionality is implemented in this Yang model through the uses and grouping keywords. These do not appear to be functioning in the expected manner because if I copy the leafs manually into the container definitions then it correctly matches the fields.

Steps to Reproduce

  1. Configure your config_db.json to have the following BGP_NEIGHBORS config
"BGP_GLOBALS": {
    "default": {}
},
"BGP_NEIGHBOR": {
    "default|10.0.0.1": {
        "asn": "65000"
    }
}
  1. Run config reload -y
  2. Attempt to break out any port

Observed Results

The breakout fails with the following error....

sonic_yang(3):All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['default|10.0.0.1'])
sonic_yang(3):exceptionList:['"asn"']

Expected Results

Successful breakout

@praveen-li
Copy link
Collaborator

This will throw an error that the asn key is not matched. This functionality is implemented in this Yang model through the uses and grouping keywords. These do not appear to be functioning in the expected manner because if I copy the leafs manually into the container definitions then it correctly matches the fields.

@robocoder99
-- Kindly paste relevant yang models field.

@venkatmahalingam
-- We need to check if grouping is supported in Python yang library or not.
-- We need to add missing fields if these are allowable configs.

@venkatmahalingam
Copy link
Collaborator

venkatmahalingam commented Jun 25, 2021

@robocoder99
It does not support any fields inside a BGP_NEIGHBOR entry in CONFIG_DB such as asn or holdtime.
-> We have these fields in the BGP_NEIGHBOR table, did you get any error for these fields? since the key is different (expects VRF name as well) now in YANG, how did you get to check the fields?

@alexrallen
Copy link
Contributor Author

@venkatmahalingam Please see my "steps to reproduce section" this includes all details to produce a minimal recreation of the bug.

@venkatmahalingam
Copy link
Collaborator

venkatmahalingam commented Jun 28, 2021

Created the PR to fix this issue.

@zhenggen-xu zhenggen-xu added the YANG YANG model related changes label Jul 14, 2021
@dgsudharsan
Copy link
Collaborator

@alexrallen Can you please close this as the fix is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Port Breakout 🔱 YANG YANG model related changes
Projects
None yet
Development

No branches or pull requests

6 participants