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

Configuring send-community per AFI-SAFI at neighbor/peer-group (#969) #976

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

missaesasaya
Copy link
Contributor

@missaesasaya missaesasaya commented Oct 13, 2023

This is a continuation of #843, where the wrong grouping was changed

Change Scope

  • Allowing send-community to be configured not only per neighbor/peer-group, but also per AFI-SAFI inside neighbor/peerg-group making the configuration more flexible.
  • This change is backwards compatible.

Platform Implementations

  • Huawei [https://support.huawei.com/enterprise/en/doc/EDOC1100008283/bbae5056/peer-advertise-community]: the peer configuration is done inside the address-family, so the peer-advertise-community configuration is done per peer+afi/safi
  • Juniper [https://www.juniper.net/documentation/en_US/junose15.1/topics/reference/command-summary/neighbor-send-community.html]: Configuration mode indicates the config can be done per address-family
  • FRR [https://docs.frrouting.org/en/latest/bgp.html]: Then configuration example show send-community parameter is also configured in address-family mode
module: openconfig-network-instance
  +--rw network-instances
     +--rw network-instance* [name]
        +--rw name                              -> ../config/name
        +--rw protocols
           +--rw protocol* [identifier name]
              +--rw identifier          -> ../config/identifier
              +--rw name                -> ../config/name
              +--rw bgp
                 +--rw global
                 |  +--rw afi-safis
                 |     +--rw afi-safi* [afi-safi-name]
                 |        +--rw afi-safi-name              -> ../config/afi-safi-name
                 |        +--rw config
                 |        |  +--rw afi-safi-name?         identityref
                 |        |  +--rw enabled?               boolean
                 |        |  +--rw send-community-type   oc-bgp-types:community-type
                 |        +--ro state
                 |           +--ro afi-safi-name?         identityref
                 |           +--ro enabled?               boolean
                 |           +--ro send-community-type   oc-bgp-types:community-type
                 |           +--ro total-paths?           uint32
                 |           +--ro total-prefixes?        uint32
                 +--rw neighbors
                 |  +--rw neighbor* [neighbor-address]
                 |     +--rw neighbor-address      -> ../config/neighbor-address
                 |     +--rw config
                 |     |  +--rw peer-group?            -> ../../../../peer-groups/peer-group/peer-group-name
                 |     |  +--rw ...
                 |     |  **+--rw send-community-type*   oc-bgp-types:community-type**
                 |     +--ro state
                 |     |  +--ro peer-group?                   -> ../../../../peer-groups/peer-group/peer-group-name
                 |     |  +--ro ...
                 |     |  **+--ro send-community-type*          oc-bgp-types:community-type**
                 |     +--rw afi-safis
                 |        +--rw afi-safi* [afi-safi-name]
                 |           +--rw afi-safi-name           -> ../config/afi-safi-name
                 |           +--rw config
                 |           |  +--rw afi-safi-name?         identityref
                 |           |  +--rw enabled?               boolean
                 |           |  **+--rw send-community-type*   oc-bgp-types:community-type**   <-- NEW LEAF-LIST
                 |           +--ro state
                 |              +--ro afi-safi-name?         identityref
                 |              +--ro enabled?               boolean
                 |              **+--ro send-community-type*   oc-bgp-types:community-type**   <-- NEW LEAF-LIST
                 |              +--ro active?                boolean
                 +--rw peer-groups
                    +--rw peer-group* [peer-group-name]
                       +--rw peer-group-name       -> ../config/peer-group-name
                       +--rw config
                       |  +--rw peer-group-name?       string
                       |  +--rw ...
                       |  **+--rw send-community-type*   oc-bgp-types:community-type**
                       +--ro state
                       |  +--ro peer-group-name?       string
                       |  +--ro ...
                       |  **+--ro send-community-type*   oc-bgp-types:community-type**
                       +--rw afi-safis
                          +--rw afi-safi* [afi-safi-name]
                             +--rw afi-safi-name           -> ../config/afi-safi-name
                             +--rw config
                             |  +--rw afi-safi-name?         identityref
                             |  +--rw enabled?               boolean
                             |  **+--rw send-community-type*   oc-bgp-types:community-type**   <-- NEW LEAF-LIST
                             +--ro state
                                +--ro afi-safi-name?         identityref
                                +--ro enabled?               boolean
                                **+--ro send-community-type*   oc-bgp-types:community-type**   <-- NEW LEAF-LIST

@missaesasaya missaesasaya requested a review from a team as a code owner October 13, 2023 12:05
@OpenConfigBot
Copy link

OpenConfigBot commented Oct 13, 2023

No major YANG version changes in commit bba2375

@OpenConfigBot
Copy link

Compatibility Report for commit bb975e9:
pyangbind@1c28486

@dplore
Copy link
Member

dplore commented Oct 13, 2023

Will review on Oct 17, 2023 OC Operators meeting

@dplore dplore self-assigned this Oct 13, 2023
@missaesasaya
Copy link
Contributor Author

Any updates from the Operators meeting?

@dplore
Copy link
Member

dplore commented Nov 2, 2023

This didn't make the OC Operators review on Oct 17, 2023. We will review at the OC Community meeting on Nov 2, 2023 08:00 PST

@missaesasaya missaesasaya force-pushed the afi-safi-send-community branch from 2024fe2 to b07f5b0 Compare November 2, 2023 21:03
@missaesasaya
Copy link
Contributor Author

As agreed, I added the deprecated status to the leaf-list added by mistake and updated the revisions.

@missaesasaya missaesasaya force-pushed the afi-safi-send-community branch from b07f5b0 to 71bbca3 Compare November 9, 2023 17:56
@dplore
Copy link
Member

dplore commented Nov 28, 2023

Hi @missaesasaya , please update the PR description with a rendering of the OC tree to make this faster to review, thank you. (using pyang for example)

@missaesasaya missaesasaya force-pushed the afi-safi-send-community branch from 71bbca3 to 1191717 Compare November 29, 2023 11:39
@missaesasaya
Copy link
Contributor Author

Hi @missaesasaya , please update the PR description with a rendering of the OC tree to make this faster to review, thank you. (using pyang for example)

Done. Please let me know if this is sufficient.

@dplore
Copy link
Member

dplore commented Dec 27, 2023 via email

@missaesasaya
Copy link
Contributor Author

missaesasaya commented Dec 27, 2023

We've asserted that the hierarchy should be "Global -> PeerGroup -> Neighbor" and the most specific configuration has precedence.

Yes, but in this case, the configuration is not global, it is per afi/safi

@missaesasaya missaesasaya force-pushed the afi-safi-send-community branch from 841a722 to 2a96699 Compare December 30, 2023 11:02
@missaesasaya missaesasaya force-pushed the afi-safi-send-community branch from 2a96699 to fff8e4a Compare December 30, 2023 11:06
@missaesasaya
Copy link
Contributor Author

missaesasaya commented Dec 30, 2023

We've asserted that the hierarchy should be "Global -> PeerGroup -> Neighbor" and the most specific configuration has precedence.

@dplore Change done

@dplore
Copy link
Member

dplore commented Jan 18, 2024

This will merge on Jan 19, 2024

@dplore dplore merged commit ce2ebe9 into openconfig:master Jan 19, 2024
14 checks passed
romeyod pushed a commit to romeyod/aftNextHop that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants