From 4615e854966ba389bccc236205f4ee0bd4eb42a2 Mon Sep 17 00:00:00 2001
From: Aaron U'Ren <aauren@gmail.com>
Date: Tue, 5 Jul 2022 18:00:24 -0500
Subject: [PATCH] fix(bgp): set graceful restart on enabled family

Rather than setting BGP Graceful Restart on both IPv4 and IPv6
regardless of which family is enabled, check the current mode via
nrc.isIpv6 and only set on appropriate family.

Note, this mode is exclusive as the current portions of NRC kube-router
code are only meant to work with IPv4 or IPv6 not both at the same time.

Fixes #1323
---
 pkg/controllers/routing/bgp_peers.go          | 91 +++++++++++--------
 .../routing/network_routes_controller.go      |  2 +-
 2 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/pkg/controllers/routing/bgp_peers.go b/pkg/controllers/routing/bgp_peers.go
index a21c90e8..88ecad3d 100644
--- a/pkg/controllers/routing/bgp_peers.go
+++ b/pkg/controllers/routing/bgp_peers.go
@@ -114,31 +114,36 @@ func (nrc *NetworkRoutingController) syncInternalPeers() {
 				LocalRestarting: true,
 			}
 
-			n.AfiSafis = []*gobgpapi.AfiSafi{
-				{
-					Config: &gobgpapi.AfiSafiConfig{
-						Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP, Safi: gobgpapi.Family_SAFI_UNICAST},
-						Enabled: true,
-					},
-					MpGracefulRestart: &gobgpapi.MpGracefulRestart{
-						Config: &gobgpapi.MpGracefulRestartConfig{
+			if nrc.isIpv6 {
+				n.AfiSafis = []*gobgpapi.AfiSafi{
+					{
+						Config: &gobgpapi.AfiSafiConfig{
+							Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP6, Safi: gobgpapi.Family_SAFI_UNICAST},
 							Enabled: true,
 						},
-						State: &gobgpapi.MpGracefulRestartState{},
+						MpGracefulRestart: &gobgpapi.MpGracefulRestart{
+							Config: &gobgpapi.MpGracefulRestartConfig{
+								Enabled: true,
+							},
+							State: &gobgpapi.MpGracefulRestartState{},
+						},
 					},
-				},
-				{
-					Config: &gobgpapi.AfiSafiConfig{
-						Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP6, Safi: gobgpapi.Family_SAFI_UNICAST},
-						Enabled: true,
-					},
-					MpGracefulRestart: &gobgpapi.MpGracefulRestart{
-						Config: &gobgpapi.MpGracefulRestartConfig{
+				}
+			} else {
+				n.AfiSafis = []*gobgpapi.AfiSafi{
+					{
+						Config: &gobgpapi.AfiSafiConfig{
+							Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP, Safi: gobgpapi.Family_SAFI_UNICAST},
 							Enabled: true,
 						},
-						State: &gobgpapi.MpGracefulRestartState{},
+						MpGracefulRestart: &gobgpapi.MpGracefulRestart{
+							Config: &gobgpapi.MpGracefulRestartConfig{
+								Enabled: true,
+							},
+							State: &gobgpapi.MpGracefulRestartState{},
+						},
 					},
-				},
+				}
 			}
 		}
 
@@ -188,8 +193,9 @@ func (nrc *NetworkRoutingController) syncInternalPeers() {
 }
 
 // connectToExternalBGPPeers adds all the configured eBGP peers (global or node specific) as neighbours
-func connectToExternalBGPPeers(server *gobgp.BgpServer, peerNeighbors []*gobgpapi.Peer, bgpGracefulRestart bool,
-	bgpGracefulRestartDeferralTime time.Duration, bgpGracefulRestartTime time.Duration, peerMultihopTTL uint8) error {
+func (nrc *NetworkRoutingController) connectToExternalBGPPeers(server *gobgp.BgpServer, peerNeighbors []*gobgpapi.Peer,
+	bgpGracefulRestart bool, bgpGracefulRestartDeferralTime time.Duration, bgpGracefulRestartTime time.Duration,
+	peerMultihopTTL uint8) error {
 	for _, n := range peerNeighbors {
 
 		if bgpGracefulRestart {
@@ -200,29 +206,34 @@ func connectToExternalBGPPeers(server *gobgp.BgpServer, peerNeighbors []*gobgpap
 				LocalRestarting: true,
 			}
 
-			n.AfiSafis = []*gobgpapi.AfiSafi{
-				{
-					Config: &gobgpapi.AfiSafiConfig{
-						Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP, Safi: gobgpapi.Family_SAFI_UNICAST},
-						Enabled: true,
-					},
-					MpGracefulRestart: &gobgpapi.MpGracefulRestart{
-						Config: &gobgpapi.MpGracefulRestartConfig{
+			if nrc.isIpv6 {
+				n.AfiSafis = []*gobgpapi.AfiSafi{
+					{
+						Config: &gobgpapi.AfiSafiConfig{
+							Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP6, Safi: gobgpapi.Family_SAFI_UNICAST},
 							Enabled: true,
 						},
-					},
-				},
-				{
-					Config: &gobgpapi.AfiSafiConfig{
-						Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP6, Safi: gobgpapi.Family_SAFI_UNICAST},
-						Enabled: true,
-					},
-					MpGracefulRestart: &gobgpapi.MpGracefulRestart{
-						Config: &gobgpapi.MpGracefulRestartConfig{
-							Enabled: true,
+						MpGracefulRestart: &gobgpapi.MpGracefulRestart{
+							Config: &gobgpapi.MpGracefulRestartConfig{
+								Enabled: true,
+							},
 						},
 					},
-				},
+				}
+			} else {
+				n.AfiSafis = []*gobgpapi.AfiSafi{
+					{
+						Config: &gobgpapi.AfiSafiConfig{
+							Family:  &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP, Safi: gobgpapi.Family_SAFI_UNICAST},
+							Enabled: true,
+						},
+						MpGracefulRestart: &gobgpapi.MpGracefulRestart{
+							Config: &gobgpapi.MpGracefulRestartConfig{
+								Enabled: true,
+							},
+						},
+					},
+				}
 			}
 		}
 		if peerMultihopTTL > 1 {
diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go
index 1af73626..184321d2 100644
--- a/pkg/controllers/routing/network_routes_controller.go
+++ b/pkg/controllers/routing/network_routes_controller.go
@@ -1132,7 +1132,7 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error {
 	}
 
 	if len(nrc.globalPeerRouters) != 0 {
-		err := connectToExternalBGPPeers(nrc.bgpServer, nrc.globalPeerRouters, nrc.bgpGracefulRestart,
+		err := nrc.connectToExternalBGPPeers(nrc.bgpServer, nrc.globalPeerRouters, nrc.bgpGracefulRestart,
 			nrc.bgpGracefulRestartDeferralTime, nrc.bgpGracefulRestartTime, nrc.peerMultihopTTL)
 		if err != nil {
 			err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{})