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

BGP AS Path not supporting AS Path Stuffing #932

Open
marenamat opened this issue Jul 28, 2023 · 6 comments
Open

BGP AS Path not supporting AS Path Stuffing #932

marenamat opened this issue Jul 28, 2023 · 6 comments

Comments

@marenamat
Copy link

In openconfig-rib-bgp-attributes.yang, grouping bgp-as-path-attr-state declares member as leaf-list which has unique elements by RFC. This way, AS Stuffed Path (like 65533 65533 65533 65500) generates YANG-invalid data. Dunno how to fix this, maybe even by updating the RFC to allow non-unique elements in leaf-list?

@dplore
Copy link
Member

dplore commented Mar 12, 2024

Thanks for bringing this up. It's not clear to me why we have a nested list of as-path numbers. I expect we would have just one flat, ordered list with an index as the key and one aspath per index. But for some reason the OC model has a nested model where there is a list of leaf-lists.

@aashaikh do you have any history on why we have a nested as-path model?

@marenamat
Copy link
Author

Hello, the as-path structure is mapped 1:1 to RFC 4271, that's fine and it probably shouldn't get flattened. The problem is just the usage/definition of leaf-list.

Also note that by having an index-value structure, you're effectively making the AS path ridiculously unreadable for anybody (and also unreasonably large).

I would suggest to actually update the YANG RFC to add data types of "implicitly indexed ordered list of the same type" or something like that, and then use such a type. This kind of data structure is well representable in XML, JSON and CBOR and it's a pity that every list has to have explicit indexes just for nothing.

@dplore
Copy link
Member

dplore commented Mar 19, 2024

This is not really the forum to suggest changes to the yang language. If there is some format you suggest that would be more readable and legal in yang 1.0, we could entertain that.

@marenamat
Copy link
Author

marenamat commented Mar 19, 2024 via email

@dplore
Copy link
Member

dplore commented Mar 19, 2024

I am interested to see your proposal! Please do share what you are thinking in yang or even as a hypothetical tree structure to get the conversation started before writing/refactoring yang. (for example, the current portion of the tree we are discussing:)

  +--rw network-instances
     +--rw network-instance* [name]
        +--rw protocols
        |  +--rw protocol* [identifier name]
        |     +--rw bgp
        |     |  +--rw global
        |     |  +--ro rib
        |     |     +--ro attr-sets
        |     |     |  +--ro attr-set* [index]
        |     |     |     +--ro as-path
        |     |     |     |  +--ro as-segment* [index]
        |     |     |     |     +--ro index    -> ../state/index
        |     |     |     |     +--ro state
        |     |     |     |        +--ro index?    uint32
        |     |     |     |        +--ro type?     oc-bgpt:as-path-segment-type
        |     |     |     |        +--ro member*   oc-inet:as-number
        |     |     |     +--ro as4-path
        |     |     |     |  +--ro as4-segment* [index]
        |     |     |     |     +--ro index    -> ../state/index
        |     |     |     |     +--ro state
        |     |     |     |        +--ro index?    uint32
        |     |     |     |        +--ro type?     oc-bgpt:as-path-segment-type
        |     |     |     |        +--ro member*   oc-inet:as-number

@LimeHat
Copy link

LimeHat commented Mar 20, 2024

@marenamat re: updating the RFC, this has been solved in yang 1.1 [for non-configuration data], rfc7950.

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

No branches or pull requests

3 participants