Skip to content

Enable BGP next-hop-self - #666

Draft
nikatza wants to merge 2 commits into
mainfrom
bgp-nexthop-self
Draft

nikatza wants to merge 2 commits into
mainfrom
bgp-nexthop-self

Conversation

@nikatza

@nikatza nikatza commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

See commit message for design and implementation details.

---
apiVersion: networking.metal.ironcore.dev/v1alpha1
kind: BGPPeer
metadata:
  labels:
    app.kubernetes.io/name: network-operator
    app.kubernetes.io/managed-by: kustomize
  name: leaf1-spine1-next-hop-self
spec:
  deviceRef:
    name: leaf1
  bgpRef:
    name: bgp
  address: 10.0.0.4
  asNumber: 65000
  localAddress:
    interfaceRef:
      name: lo0
  addressFamilies:
    ipv4Unicast:
      enabled: true
      outboundRoutingPolicyRef:
        name: bgp-next-hop-self
---
apiVersion: networking.metal.ironcore.dev/v1alpha1
kind: RoutingPolicy
metadata:
  labels:
    app.kubernetes.io/name: network-operator
    app.kubernetes.io/managed-by: kustomize
  name: bgp-import-policy-next-hop-self
spec:
  deviceRef:
    name: leaf1
  name: BGP-NEXT-HOP-SELF
  statements:
    - sequence: 10
      actions:
        routeDisposition: AcceptRoute
        bgpActions:
          setNextHop:
            type: Self
 ---

Tested with a cisco nxos device:

router bgp 65000
  router-id 10.0.0.10
  address-family ipv4 unicast
    maximum-paths 4
  address-family ipv6 unicast
    maximum-paths 2
  address-family l2vpn evpn
    retain route-target all
  template peer __operator-managed--default__
  neighbor 10.0.0.4
    remote-as 65000
    update-source loopback0
    address-family ipv4 unicast
      route-map BGP-NEXT-HOP-SELF out
      next-hop-self

@felix-kaestner felix-kaestner added the review-scheduled The PR is being reviewed. label Oct 22, 2026
@nikatza
nikatza force-pushed the bgp-nexthop-self branch 5 times, most recently from db044a8 to 8fbcb5d Compare October 23, 2026 17:48
This commit enables users to configure the next-hop attribute of routes
advertised to BGP peers. The scope of this change is southbound
connectivity and is currently limited to Cisco NX-OS.

OpenConfig models the BGP next hop as an action on a routing policy
statement. The action may be conditional and apply only to routes
accepted by selected statements. The Kubernetes API follows this
OpenConfig model, which does not directly match the NX-OS DME model.

NX-OS DME instead exposes `nh-self` as a BGP peer address-family flag.
Its scope is broader because it applies to every route advertised for
that address family. It is stored in the shared `ctrl` leaf together
with controls such as `rr-client`, which must be preserved when
next-hop-self is added or removed.

To realize the action on NX-OS devices, the controller resolves
referenced routing policies and passes them to the provider. The policy
name alone is insufficient because NX-OS must inspect its actions when
configuring the peer. Routing policies are configured independently and
may change after a peer has been realized, so policy generation changes
also trigger reconciliation of affected peers.

Interpretation of the resolved actions remains the responsibility of
each platform-specific provider. The NX-OS provider translates an
outbound `Self` action only when every permitted route has the same
behavior. It rejects inbound actions, explicit addresses, and mixed
policies that cannot be represented without changing their meaning.

The OpenConfig and IOS-XR providers return an unsupported-field error
instead of silently ignoring the new action.

Signed-off-by: Pujol <enric.pujol@sap.com>
Represent BGP peer address-family control as a typed bitmask instead of
comparing comma-separated strings. String comparison can cause an
unnecessary device update on every reconciliation when the flag order
returned by the device differs from the desired order.

On NX-OS 10.6(3), the device canonicalizes "rr-client,nh-self" to
"nh-self,rr-client" and returns null when no control flags are set.

Sorting only the outgoing string still relies on the device returning
the same order. Handling unset markers globally in Option.UnmarshalJSON
is also unsafe because it changes the meaning of unset values for
unrelated NX-OS fields: it would turn unset values into empty values

This commit handles unset values locally in the BGP control type. Unit
tests verify that flag order does not matter and that both marker and
null values are treated as unset.

Signed-off-by: Pujol <enric.pujol@sap.com>
@github-actions

Copy link
Copy Markdown

Caution

No coverage for base commit 66fe5c5 was found, so this report compares against its ancestor b7e7c94 (1 commit earlier)

Merging this branch changes the coverage (2 decrease, 5 increase)

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1 2.38% (-0.01%) 👎
github.com/ironcore-dev/network-operator/internal/controller/core 62.46% (+0.42%) 👍
github.com/ironcore-dev/network-operator/internal/controller/evpn 80.00% (-0.90%) 👎
github.com/ironcore-dev/network-operator/internal/controller/overlay 70.91% (+10.91%) 🎉
github.com/ironcore-dev/network-operator/internal/controller/pool 73.63% (+1.05%) 👍
github.com/ironcore-dev/network-operator/internal/provider 1.75% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr 40.43% (+0.04%) 👍
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos 20.61% (+2.84%) 👍
github.com/ironcore-dev/network-operator/internal/provider/openconfig 9.51% (-0.03%) 👎
github.com/ironcore-dev/network-operator/internal/webhook/core/v1alpha1 95.04% (ø)
Coverage details

Compared to commit b7e7c94 (run #39940220055)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1/routingpolicy_types.go 12.50% (ø) 8 1 7
github.com/ironcore-dev/network-operator/api/core/v1alpha1/zz_generated.deepcopy.go 0.00% (ø) 2982 (+13) 0 2982 (+13)
github.com/ironcore-dev/network-operator/internal/controller/core/bgp_peer_controller.go 66.94% (+7.00%) 360 (-2) 241 (+24) 119 (-26) 👍
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/route_policies.go 72.63% (-0.49%) 95 (+2) 69 (+1) 26 (+1) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/bgp.go 45.74% (+25.74%) 129 (+44) 59 (+42) 70 (+2) 🌟
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go 6.97% (+2.65%) 2569 (+43) 179 (+70) 2390 (-27) 👍
github.com/ironcore-dev/network-operator/internal/provider/openconfig/bgppeer.go 2.70% (ø) 74 2 72
github.com/ironcore-dev/network-operator/internal/provider/openconfig/routingpolicy.go 0.00% (ø) 26 (+2) 0 26 (+2)
github.com/ironcore-dev/network-operator/internal/provider/provider.go 1.75% (ø) 57 1 56

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files:

  • github.com/ironcore-dev/network-operator/internal/controller/core/bgp_peer_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/suite_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/bgp_test.go
  • github.com/ironcore-dev/network-operator/internal/webhook/core/v1alpha1/routingpolicy_webhook_test.go

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-scheduled The PR is being reviewed. size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants