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

BGPv1: Adds Peer Port Support #25809

Merged
merged 1 commit into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions api/v1/models/bgp_peer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/v1/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3534,6 +3534,11 @@ definitions:
Time To Live (TTL) value used in BGP packets sent to the eBGP neighbor.
0 if eBGP multi-hop feature is disabled.
type: integer
peer-port:
description: TCP port number of peer
type: integer
minimum: 1
maximum: 65535
session-state:
description: |
BGP peer operational state as described here
Expand Down
12 changes: 12 additions & 0 deletions api/v1/server/embedded_spec.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cilium/cmd/bgp_peer_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func printSummary(peers []*models.BgpPeer) {
for _, peer := range peers {
fmt.Fprintf(w, "%d\t", peer.LocalAsn)
fmt.Fprintf(w, "%d\t", peer.PeerAsn)
fmt.Fprintf(w, "%s\t", peer.PeerAddress)
fmt.Fprintf(w, "%s:%d\t", peer.PeerAddress, peer.PeerPort)
fmt.Fprintf(w, "%s\t", peer.SessionState)

// Time is rounded to nearest second
Expand Down
12 changes: 12 additions & 0 deletions pkg/bgpv1/agent/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/sirupsen/logrus"
"k8s.io/utils/pointer"

"github.com/cilium/workerpool"

Expand Down Expand Up @@ -435,6 +436,9 @@ func (c *Controller) applyPolicyDefaults(policy *v2alpha1api.CiliumBGPPeeringPol
for _, r := range p.Spec.VirtualRouters {
for j := range r.Neighbors {
n := &r.Neighbors[j]
// Set neighbor default port for peering if unspecified. This is done by kube-apiserver
// but added here for non-k8s environments, tests, etc.
setDefaultPeerPort(n)
if n.ConnectRetryTime.Duration == 0 {
n.ConnectRetryTime.Duration = types.DefaultBGPConnectRetryTime
}
Expand Down Expand Up @@ -478,3 +482,11 @@ func (c *Controller) validatePolicy(policy *v2alpha1api.CiliumBGPPeeringPolicy)
}
return nil
}

// setDefaultPeerPort sets the PeerPort of the provided neighbor
// to defaultPeerPort (179) if unset.
func setDefaultPeerPort(n *v2alpha1api.CiliumBGPNeighbor) {
if n.PeerPort == nil {
n.PeerPort = pointer.Int(types.DefaultPeerPort)
}
}
5 changes: 4 additions & 1 deletion pkg/bgpv1/agent/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/cilium/cilium/pkg/bgpv1/agent"
"github.com/cilium/cilium/pkg/bgpv1/mock"
"github.com/cilium/cilium/pkg/bgpv1/types"
v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
v1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1"
nodeaddr "github.com/cilium/cilium/pkg/node"
Expand Down Expand Up @@ -141,7 +142,9 @@ func TestControllerSanity(t *testing.T) {
defaulted := false
for _, r := range p.Spec.VirtualRouters {
for _, n := range r.Neighbors {
if n.ConnectRetryTime.Duration != 0 &&
if n.PeerPort != nil &&
*n.PeerPort == types.DefaultPeerPort &&
n.ConnectRetryTime.Duration != 0 &&
n.HoldTime.Duration != 0 &&
n.KeepAliveTime.Duration != 0 &&
n.GracefulRestart.RestartTime.Duration != 0 {
Expand Down
29 changes: 21 additions & 8 deletions pkg/bgpv1/gobgp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (g *GoBGPServer) AddNeighbor(ctx context.Context, n types.NeighborRequest)
Peer: peer,
}
if err = g.server.AddPeer(ctx, peerReq); err != nil {
return fmt.Errorf("failed while adding peer %v %v: %w", n.Neighbor.PeerAddress, n.Neighbor.PeerASN, err)
return fmt.Errorf("failed while adding peer %v:%v with ASN %v: %w", n.Neighbor.PeerAddress, *n.Neighbor.PeerPort, n.Neighbor.PeerASN, err)
}
return nil
}
Expand All @@ -128,12 +128,12 @@ func (g *GoBGPServer) UpdateNeighbor(ctx context.Context, n types.NeighborReques
}
updateRes, err := g.server.UpdatePeer(ctx, peerReq)
if err != nil {
return fmt.Errorf("failed while updating peer %v %v: %w", n.Neighbor.PeerAddress, n.Neighbor.PeerASN, err)
return fmt.Errorf("failed while updating peer %v:%v with ASN %v: %w", n.Neighbor.PeerAddress, *n.Neighbor.PeerPort, n.Neighbor.PeerASN, err)
}

// perform full / soft peer reset if necessary
if needsHardReset || updateRes.NeedsSoftResetIn {
g.logger.Infof("Resetting peer %s (ASN %d) due to a config change", peer.Conf.NeighborAddress, peer.Conf.PeerAsn)
g.logger.Infof("Resetting peer %s:%v (ASN %d) due to a config change", peer.Conf.NeighborAddress, *n.Neighbor.PeerPort, peer.Conf.PeerAsn)
resetReq := &gobgp.ResetPeerRequest{
Address: peer.Conf.NeighborAddress,
Communication: "Peer configuration changed",
Expand All @@ -143,7 +143,7 @@ func (g *GoBGPServer) UpdateNeighbor(ctx context.Context, n types.NeighborReques
resetReq.Direction = gobgp.ResetPeerRequest_IN
}
if err = g.server.ResetPeer(ctx, resetReq); err != nil {
return fmt.Errorf("failed while resetting peer %v %v: %w", n.Neighbor.PeerAddress, n.Neighbor.PeerASN, err)
return fmt.Errorf("failed while resetting peer %v:%v in ASN %v: %w", n.Neighbor.PeerAddress, *n.Neighbor.PeerPort, n.Neighbor.PeerASN, err)
}
}

Expand All @@ -160,6 +160,11 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
}
peerAddr := prefix.Addr()

peerPort := uint32(types.DefaultPeerPort)
if n.PeerPort != nil {
peerPort = uint32(*n.PeerPort)
}

var existingPeer *gobgp.Peer
if isUpdate {
// If this is an update, try retrieving the existing Peer.
Expand All @@ -172,8 +177,13 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
}
// use only necessary parts of the existing peer struct
peer = &gobgp.Peer{
Conf: existingPeer.Conf,
AfiSafis: existingPeer.AfiSafis,
Conf: existingPeer.Conf,
Transport: existingPeer.Transport,
AfiSafis: existingPeer.AfiSafis,
}
// Update the peer port if needed.
if existingPeer.Transport.RemotePort != peerPort {
peer.Transport.RemotePort = peerPort
}
} else {
// Create a new peer
Expand All @@ -182,6 +192,9 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
NeighborAddress: peerAddr.String(),
PeerAsn: uint32(n.PeerASN),
},
Transport: &gobgp.Transport{
RemotePort: peerPort,
},
// tells the peer we are capable of unicast IPv4 and IPv6
// advertisements.
AfiSafis: []*gobgp.AfiSafi{
Expand All @@ -203,9 +216,9 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
// when calling AddPeer / UpdatePeer / ListPeer, we set it explicitly to a wildcard address
// based on peer's address family, to not cause unnecessary connection resets upon update.
if peerAddr.Is4() {
peer.Transport = &gobgp.Transport{LocalAddress: wildcardIPv4Addr}
peer.Transport.LocalAddress = wildcardIPv4Addr
} else {
peer.Transport = &gobgp.Transport{LocalAddress: wildcardIPv6Addr}
peer.Transport.LocalAddress = wildcardIPv6Addr
}

// Enable multi-hop for eBGP if non-zero TTL is provided
Expand Down
4 changes: 4 additions & 0 deletions pkg/bgpv1/gobgp/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func (g *GoBGPServer) GetPeerState(ctx context.Context) (types.GetPeerStateRespo

peerState := &models.BgpPeer{}

if peer.Transport != nil {
peerState.PeerPort = int64(peer.Transport.RemotePort)
}

if peer.Conf != nil {
peerState.LocalAsn = int64(peer.Conf.LocalAsn)
peerState.PeerAddress = peer.Conf.NeighborAddress
Expand Down
24 changes: 24 additions & 0 deletions pkg/bgpv1/gobgp/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"testing"
"time"

"k8s.io/utils/pointer"

"github.com/cilium/cilium/api/v1/models"
"github.com/cilium/cilium/pkg/bgpv1/types"
v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
Expand All @@ -25,6 +27,7 @@ var (
neighbor64125 = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64125,
PeerAddress: "192.168.0.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 99 * time.Second},
HoldTime: metav1.Duration{Duration: 9 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 3 * time.Second},
Expand All @@ -34,6 +37,7 @@ var (
neighbor64125Update = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64125,
PeerAddress: "192.168.0.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 101 * time.Second},
HoldTime: metav1.Duration{Duration: 9 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 3 * time.Second},
Expand All @@ -43,6 +47,7 @@ var (
neighbor64125UpdateGR = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64125,
PeerAddress: "192.168.0.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 101 * time.Second},
HoldTime: metav1.Duration{Duration: 30 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 10 * time.Second},
Expand All @@ -56,6 +61,7 @@ var (
neighbor64125UpdateGRTimer = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64125,
PeerAddress: "192.168.0.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 101 * time.Second},
HoldTime: metav1.Duration{Duration: 30 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 10 * time.Second},
Expand All @@ -68,6 +74,7 @@ var (
neighbor64126 = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64126,
PeerAddress: "192.168.66.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 99 * time.Second},
HoldTime: metav1.Duration{Duration: 9 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 3 * time.Second},
Expand All @@ -77,6 +84,7 @@ var (
neighbor64126Update = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64126,
PeerAddress: "192.168.66.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 99 * time.Second},
HoldTime: metav1.Duration{Duration: 12 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 4 * time.Second},
Expand All @@ -103,6 +111,7 @@ var (
neighbor64128 = &v2alpha1api.CiliumBGPNeighbor{
PeerASN: 64128,
PeerAddress: "192.168.77.1/32",
PeerPort: pointer.Int(types.DefaultPeerPort),
ConnectRetryTime: metav1.Duration{Duration: 99 * time.Second},
HoldTime: metav1.Duration{Duration: 9 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 3 * time.Second},
Expand Down Expand Up @@ -131,6 +140,21 @@ func TestGetPeerState(t *testing.T) {
localASN: 64124,
errStr: "",
},
{
name: "test add neighbor with port",
neighbors: []*v2alpha1api.CiliumBGPNeighbor{
{
PeerASN: 64125,
PeerAddress: "192.168.0.1/32",
PeerPort: pointer.Int(175),
ConnectRetryTime: metav1.Duration{Duration: 99 * time.Second},
HoldTime: metav1.Duration{Duration: 9 * time.Second},
KeepAliveTime: metav1.Duration{Duration: 3 * time.Second},
},
},
localASN: 64124,
errStr: "",
},
{
name: "test add + update neighbors",
neighbors: []*v2alpha1api.CiliumBGPNeighbor{
Expand Down