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.

Sign up for GitHub

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

Jump to bottom

Feat(eos_designs): Support for IPv4 p2p_links in an RFC5549 Fabric #4379

Open
1 task done
nathanmusser opened this issue Aug 19, 2024 · 4 comments · May be fixed by #4491
Open
1 task done

Feat(eos_designs): Support for IPv4 p2p_links in an RFC5549 Fabric #4379

nathanmusser opened this issue Aug 19, 2024 · 4 comments · May be fixed by #4491
Labels
type: enhancement New feature or request

Comments

@nathanmusser
Copy link
Contributor

nathanmusser commented Aug 19, 2024 •
edited
Loading

Enhancement summary

Currently if include_in_underlay_protocol is true then the p2p_link is given the same peer group as the rest of the underlay. If the underlay fabric is an RFC5549 fabric using IPv6 but the p2p_link is IPv4, then due to the peer group it gets the attribute neighbor IPv4-UNDERLAY-PEERS next-hop address-family ipv6 originate set under the IPv4 address family. This will prevent routes from being shared.

Which component of AVD is impacted

eos_designs, eos_cli_config_gen

Use case example

Two DC fabrics that use RFC5549 as their individual underlays but with a desire to make all connections out of the fabric IPv4, including the DCI.

Describe the solution you would like

The quickest solution (from my naive perspective) is to add a key to allow overriding the peer-group used on the p2p_link. This would allow multiple benefits, not only could one control the ipv6 originate knob, but in the case of the DCI they could more easily tune other knobs independent of the rest of the underlay.

Alternatively the existing ipv6_enable key controls the interface itself but has no impact on the neighborship AFAIK. This key could be used to automatically add the no ... next-hop address-family ipv6 originate for the neighbor if the underlay is set to RFC5549, or to add logic to create an RFC5549 underlay peer-group as well as a non-rfc5549 peer group (ideally only if needed). This solution adds more nested logic and would have to be careful to make sure it's backwards compatible. Even though I'm not sure how it would be working, existing installations might be using rfc5549 with ipv6 set to false and this would be a change in behavior.

Describe alternatives you have considered

  • Adding the manual config to the bgp ipv4 address family to override next-hop address-family ipv6 originate for the specific p2p neighbors. Feels confusing and not clear to have it defined in the peer group and then override it.
  • Manually build out all the neighborships for the DCI without using p2p_links. Undesirable, but likely the short term solution

Additional context

I'm willing to work on a PR that implements this. Just looking for guidance on the best way to approach this issue before I start working.

Contributing Guide

  • I agree to follow this project's Code of Conduct
@nathanmusser nathanmusser added the type: enhancement New feature or request label Aug 19, 2024
@nathanmusser nathanmusser changed the title Support for IPv4 p2p_links in an RFC5549 Fabric Feat(eos_designs): Support for IPv4 p2p_links in an RFC5549 Fabric Aug 19, 2024
@ClausHolbechArista
Copy link
Contributor

ClausHolbechArista commented Aug 20, 2024 •
edited
Loading

EDIT: After inspecting the code you need both, but that should solve it.

Thank you for a thoroughly described issue.
Please try to remove the include_in_underlay and set routing_protocol: ebgp instead and report back. (If it does not work then also try with both).

Snip from the docs:

          # Enables deviation of the routing protocol used on this link from the fabric underlay default.
          # - ebgp: Enforce plain IPv4 BGP peering
          routing_protocol: <str; "ebgp">

This is at the bottom of the doc which makes it easy to miss.

@nathanmusser
Copy link
Contributor Author

I was actually already using the routing_protocol key set to ebgp with the same results.

Here's some quick tests I ran just now, none of these are achieving the desired results, but all of them having different behavior.

Model:

l3_edge:
  p2p_links_profiles:
  - name: dci_link
    ip_pool: dci_links
    as: ["65120.11200", "65120.12200"]
    routing_protocol: "ebgp"
    include_in_underlay_protocol: true
    ipv6_enable: false

Result:

interface Ethernet3/1
   ip address 10.210.226.8/31
!
router bgp 65120.11200
   neighbor 10.210.226.9 peer group IPv4-UNDERLAY-PEERS
   address-family ipv4
      neighbor IPv4-UNDERLAY-PEERS next-hop address-family ipv6 originate

Model:

l3_edge:
  p2p_links_profiles:
  - name: dci_link
    ip_pool: dci_links
    as: ["65120.11200", "65120.12200"]
    #routing_protocol: "ebgp"
    include_in_underlay_protocol: true
    ipv6_enable: false

Result:

interface Ethernet3/1
   ip address 10.210.226.8/31
   ipv6 enable  !!IPv6 enabled even though set to false
!
router bgp 65120.11200
   !!Neighbor not listed 
   address-family ipv4
      neighbor IPv4-UNDERLAY-PEERS next-hop address-family ipv6 originate

Model:

l3_edge:
  p2p_links_profiles:
  - name: dci_link
    ip_pool: dci_links
    as: ["65120.11200", "65120.12200"]
    #routing_protocol: "ebgp"
    include_in_underlay_protocol: false
    ipv6_enable: false

Result:

interface Ethernet3/1
   ip address 10.210.226.8/31
!
router bgp 65120.11200
   !!Neighbor not listed 
   address-family ipv4
      neighbor IPv4-UNDERLAY-PEERS next-hop address-family ipv6 originate

I've only just glanced at the related code but the obstacle to my goal is that the underlay peers always get the same underlay peer group. So while I can get the interface to not be enabled for IPv6, and I can get bgp config generated, I still have to find a way to disable the ipv6 originate.

avd/python-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/router_bgp.py

Lines 41 to 46 in 92ec3a8

neighbor = {
"remote_as": p2p_link["data"]["peer_bgp_as"],
"peer": p2p_link["data"]["peer"],
"description": p2p_link["data"]["peer"],
"peer_group": self.shared_utils.bgp_peer_groups["ipv4_underlay_peers"]["name"],
}

@ClausHolbechArista
Copy link
Contributor

We could change the current behavior so giving routing_protocol: ebgp and include_in_underlay_protocol: false will render it as a stand-alone neighbor outside of the peer-group. That would not be a breaking change, since the neighbor would not be rendered at all Today.

@nathanmusser nathanmusser linked a pull request Sep 18, 2024 that will close this issue
Open
4 tasks
@nathanmusser
Copy link
Contributor Author

Added a PR based on your suggestion. We've temporarily worked around this issue in our deployment until AVD gets support for it. But we have settled on the desire to be able to control the peer group for a specific l3_edge connection. Specifically this would allow granular control over DCI connections in BGP.

I wanted to go ahead and get the base PR in to cover the generic use case, but would like to continue to discuss the idea of adding a new structure.

l3_edge:
  p2p_links_bgp_peer_group:
    - name: <str; required; unique>
      # peer group settings
  p2p_links:
    - nodes: 
      # P2P bgp peer group name. Profile defined under p2p_links_bgp_peer_group.
      bgp_peer_group: <str>

After briefly glancing over the code I do understand that this is a more extensive feature request, but I plan to attempt to tackle it myself as long as there are no objections to the structure of it.

Alternatively, I was unable to find a way to render structured configuration to the bgp neighborship itself or raw eos cli. Both these options would be less elegant. But if adding the peer group key is undesirable perhaps a way to get structured config into the neighborship would be sufficient. Or maybe there is a way that I was unable to find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feat(eos_designs): Exclude l3_edge neighbors from peer_group nathanmusser/avd
2 participants
@ClausHolbechArista @nathanmusser