Repository: metallb/metallb
Pull request #1017 (2021-10-30 20:55:40)

Add keepalivetime configuration option to metallb
Added BGP peer keepalivetime configuration option
the current metallb stack assumes its always 1/3 of the holdtime while this the recommended default setting by rfc4271 but it doesn't have to stay like that, this change export the config option so FRR can make use of it while not changing the current
keepalive alg used by Metallb native stack.
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>




Patch:
diff --git a/internal/bgp/bgp.go b/internal/bgp/bgp.go
index 82ba7c17..a4f245cb 100644
--- a/internal/bgp/bgp.go
+++ b/internal/bgp/bgp.go
@@ -44,5 +44,5 @@ type Session interface {
 }
 
 type SessionManager interface {
-	NewSession(logger log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, hold time.Duration, password string, myNode string) (Session, error)
+	NewSession(logger log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, hold time.Duration, keepalive time.Duration, password string, myNode string) (Session, error)
 }
diff --git a/internal/bgp/frr/frr.go b/internal/bgp/frr/frr.go
index 9ccf83a0..e114ea7d 100644
--- a/internal/bgp/frr/frr.go
+++ b/internal/bgp/frr/frr.go
@@ -27,6 +27,7 @@ type session struct {
 	srcAddr        net.IP
 	asn            uint32
 	holdTime       time.Duration
+	keepaliveTime  time.Duration
 	logger         log.Logger
 	password       string
 	advertised     []*bgp.Advertisement
@@ -108,7 +109,7 @@ func (s *session) Close() error {
 //
 // The session will immediately try to connect and synchronize its
 // local state with the peer.
-func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, holdTime time.Duration, password string, myNode string) (bgp.Session, error) {
+func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, holdTime time.Duration, keepaliveTime time.Duration, password string, myNode string) (bgp.Session, error) {
 	s := &session{
 		myASN:          myASN,
 		routerID:       routerID,
@@ -117,6 +118,7 @@ func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP,
 		srcAddr:        srcAddr,
 		asn:            asn,
 		holdTime:       holdTime,
+		keepaliveTime:  keepaliveTime,
 		logger:         log.With(l, "peer", addr, "localASN", myASN, "peerASN", asn),
 		password:       password,
 		advertised:     []*bgp.Advertisement{},
@@ -168,9 +170,13 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
 
 	config := &frrConfig{
 		Hostname: hostname,
-		Loglevel: "informational", // TODO - make loglevel configurable via envvar.
+		Loglevel: "informational",
 		Routers:  make(map[string]*routerConfig),
 	}
+	frrLogLevel, found := os.LookupEnv("FRR_LOGGING_LEVEL")
+	if found {
+		config.Loglevel = frrLogLevel
+	}
 
 	for _, s := range sm.sessions {
 		var router *routerConfig
@@ -206,7 +212,7 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
 				Addr:           host,
 				Port:           uint16(portUint),
 				HoldTime:       uint64(s.holdTime / time.Second),
-				KeepaliveTime:  uint64(s.holdTime / (3 * time.Second)), // TODO use 1/3 of holdtime till we can configure it.
+				KeepaliveTime:  uint64(s.keepaliveTime / time.Second),
 				Password:       s.password,
 				Advertisements: make([]*advertisementConfig, 0),
 			}
diff --git a/internal/bgp/native/native.go b/internal/bgp/native/native.go
index 69a932ad..072569c7 100644
--- a/internal/bgp/native/native.go
+++ b/internal/bgp/native/native.go
@@ -36,6 +36,7 @@ type session struct {
 	asn              uint32
 	peerFBASNSupport bool
 	holdTime         time.Duration
+	keepaliveTime    time.Duration
 	logger           log.Logger
 	password         string
 
@@ -64,19 +65,20 @@ func NewSessionManager() *sessionManager {
 //
 // The session will immediately try to connect and synchronize its
 // local state with the peer.
-func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, holdTime time.Duration, password string, myNode string) (bgp.Session, error) {
+func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP, myASN uint32, routerID net.IP, asn uint32, holdTime time.Duration, keepaliveTime time.Duration, password string, myNode string) (bgp.Session, error) {
 	ret := &session{
-		addr:        addr,
-		srcAddr:     srcAddr,
-		myASN:       myASN,
-		routerID:    routerID.To4(),
-		myNode:      myNode,
-		asn:         asn,
-		holdTime:    holdTime,
-		logger:      log.With(l, "peer", addr, "localASN", myASN, "peerASN", asn),
-		newHoldTime: make(chan bool, 1),
-		advertised:  map[string]*bgp.Advertisement{},
-		password:    password,
+		addr:          addr,
+		srcAddr:       srcAddr,
+		myASN:         myASN,
+		routerID:      routerID.To4(),
+		myNode:        myNode,
+		asn:           asn,
+		holdTime:      holdTime,
+		keepaliveTime: keepaliveTime,
+		logger:        log.With(l, "peer", addr, "localASN", myASN, "peerASN", asn),
+		newHoldTime:   make(chan bool, 1),
+		advertised:    map[string]*bgp.Advertisement{},
+		password:      password,
 	}
 	ret.cond = sync.NewCond(&ret.mu)
 	go ret.sendKeepalives()
diff --git a/internal/config/config.go b/internal/config/config.go
index f2119b50..6d3cdfa9 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -45,6 +45,7 @@ type peer struct {
 	SrcAddr       string         `yaml:"source-address"`
 	Port          uint16         `yaml:"peer-port"`
 	HoldTime      string         `yaml:"hold-time"`
+	KeepaliveTime string         `yaml:"keepalive-time"`
 	RouterID      string         `yaml:"router-id"`
 	NodeSelectors []nodeSelector `yaml:"node-selectors"`
 	Password      string         `yaml:"password"`
@@ -108,6 +109,8 @@ type Peer struct {
 	Port uint16
 	// Requested BGP hold time, per RFC4271.
 	HoldTime time.Duration
+	// Requested BGP keepalive time, per RFC4271.
+	KeepaliveTime time.Duration
 	// BGP router ID to advertise to the peer
 	RouterID net.IP
 	// Only connect to this peer on nodes that match one of these
@@ -196,6 +199,19 @@ func parseHoldTime(ht string) (time.Duration, error) {
 	return rounded, nil
 }
 
+func parseKeepaliveTime(ht time.Duration, ka string) (time.Duration, error) {
+	// If keepalive time not set lets use 1/3 of holdtime.
+	if ka == "" {
+		return ht / 3, nil
+	}
+	d, err := time.ParseDuration(ka)
+	if err != nil {
+		return 0, fmt.Errorf("invalid keepalive time %q: %s", ka, err)
+	}
+	rounded := time.Duration(int(d.Seconds())) * time.Second
+	return rounded, nil
+}
+
 // Parse loads and validates a Config from bs.
 func Parse(bs []byte) (*Config, error) {
 	var raw configFile
@@ -272,6 +288,14 @@ func parsePeer(p peer) (*Peer, error) {
 	if err != nil {
 		return nil, err
 	}
+	keepaliveTime, err := parseKeepaliveTime(holdTime, p.KeepaliveTime)
+	if err != nil {
+		return nil, err
+	}
+	// keepalive must be lower than holdtime
+	if keepaliveTime > holdTime {
+		return nil, fmt.Errorf("invalid keepaliveTime %q", p.KeepaliveTime)
+	}
 	port := uint16(179)
 	if p.Port != 0 {
 		port = p.Port
@@ -318,6 +342,7 @@ func parsePeer(p peer) (*Peer, error) {
 		SrcAddr:       src,
 		Port:          port,
 		HoldTime:      holdTime,
+		KeepaliveTime: keepaliveTime,
 		RouterID:      routerID,
 		NodeSelectors: nodeSels,
 		Password:      password,
diff --git a/internal/k8s/k8s.go b/internal/k8s/k8s.go
index 1b3b1fa4..6b19fb1e 100644
--- a/internal/k8s/k8s.go
+++ b/internal/k8s/k8s.go
@@ -8,6 +8,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 
 	"go.universe.tf/metallb/internal/config"
@@ -310,7 +311,7 @@ func New(cfg *Config) (*Client, error) {
 
 	http.Handle("/metrics", promhttp.Handler())
 	go func(l log.Logger) {
-		err := http.ListenAndServe(fmt.Sprintf("%s:%d", cfg.MetricsHost, cfg.MetricsPort), nil)
+		err := http.ListenAndServe(net.JoinHostPort(cfg.MetricsHost, fmt.Sprint(cfg.MetricsPort)), nil)
 		if err != nil {
 			level.Error(l).Log("op", "listenAndServe", "err", err, "msg", "cannot listen and serve", "host", cfg.MetricsHost, "port", cfg.MetricsPort)
 		}
diff --git a/manifests/metallb-frr.yaml b/manifests/metallb-frr.yaml
index c74780cf..091bda27 100644
--- a/manifests/metallb-frr.yaml
+++ b/manifests/metallb-frr.yaml
@@ -497,6 +497,11 @@ spec:
               value: /etc/frr_reloader/reloader.pid
             - name: METALLB_BGP_TYPE
               value: frr
+            # FRR_LOGGING_LEVEL used to set logging level for all running frr processes.
+            # Possible settings are :-
+            #  informational, warning, errors and debugging.
+            - name: FRR_LOGGING_LEVEL
+              value: informational
             - name: METALLB_NODE_NAME
               valueFrom:
                 fieldRef:
diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go
index febbb8ed..4fcdc292 100644
--- a/speaker/bgp_controller.go
+++ b/speaker/bgp_controller.go
@@ -63,6 +63,10 @@ newPeers:
 			if ep == nil {
 				continue
 			}
+			if ep.cfg.KeepaliveTime != 0 && c.bgpType == bgpNative {
+				level.Error(l).Log("op", "setConfig", "peer", ep.cfg.Addr, "configuration ignored", "keep alive time configuration is not supported")
+				continue
+			}
 			if reflect.DeepEqual(p, ep.cfg) {
 				newPeers = append(newPeers, ep)
 				c.peers[i] = nil
@@ -201,7 +205,7 @@ func (c *bgpController) syncPeers(l log.Logger) error {
 			if p.cfg.RouterID != nil {
 				routerID = p.cfg.RouterID
 			}
-			s, err := c.sessionManager.NewSession(c.logger, net.JoinHostPort(p.cfg.Addr.String(), strconv.Itoa(int(p.cfg.Port))), p.cfg.SrcAddr, p.cfg.MyASN, routerID, p.cfg.ASN, p.cfg.HoldTime, p.cfg.Password, c.myNode)
+			s, err := c.sessionManager.NewSession(c.logger, net.JoinHostPort(p.cfg.Addr.String(), strconv.Itoa(int(p.cfg.Port))), p.cfg.SrcAddr, p.cfg.MyASN, routerID, p.cfg.ASN, p.cfg.HoldTime, p.cfg.KeepaliveTime, p.cfg.Password, c.myNode)
 			if err != nil {
 				level.Error(l).Log("op", "syncPeers", "error", err, "peer", p.cfg.Addr, "msg", "failed to create BGP session")
 				errs++


Test patch:
diff --git a/e2etest/bgp_test.go b/e2etest/bgp_test.go
index e6d708cc..87c7d85a 100644
--- a/e2etest/bgp_test.go
+++ b/e2etest/bgp_test.go
@@ -230,7 +230,8 @@ var _ = ginkgo.Describe("BGP", func() {
 			framework.ExpectNoError(err)
 			speakerPods = make([]*corev1.Pod, 0)
 			for _, item := range speakers.Items {
-				speakerPods = append(speakerPods, &item)
+				i := item
+				speakerPods = append(speakerPods, &i)
 			}
 		})
 
diff --git a/e2etest/layer2_test.go b/e2etest/layer2_test.go
index bfaacb3a..a0aa95eb 100644
--- a/e2etest/layer2_test.go
+++ b/e2etest/layer2_test.go
@@ -22,6 +22,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"net"
 	"strconv"
 	"time"
@@ -32,6 +33,7 @@ import (
 	"github.com/onsi/gomega"
 	"go.universe.tf/metallb/e2etest/pkg/executor"
 	"go.universe.tf/metallb/e2etest/pkg/mac"
+	"go.universe.tf/metallb/e2etest/pkg/metrics"
 	internalconfig "go.universe.tf/metallb/internal/config"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -120,13 +122,7 @@ var _ = ginkgo.Describe("L2", func() {
 			err = wgetRetry(address, executor.Host)
 			framework.ExpectNoError(err)
 
-			err = mac.UpdateNodeCache(epNodes, executor.Host)
-			framework.ExpectNoError(err)
-
-			macAddr, err := mac.ForIP(ingressIP, executor.Host)
-			framework.ExpectNoError(err)
-
-			advNode, err := mac.MatchNode(epNodes, macAddr, executor.Host)
+			advNode, err := advertisingNodeFromMAC(epNodes, ingressIP, executor.Host)
 			framework.ExpectNoError(err)
 
 			for i := 0; i < 5; i++ {
@@ -289,6 +285,145 @@ var _ = ginkgo.Describe("L2", func() {
 	},
 		table.Entry("IPV4", &ipv4ServiceRange),
 		table.Entry("IPV6", &ipv6ServiceRange))
+
+	ginkgo.Context("metrics", func() {
+		// TODO: validate gratuitous_sent metric
+		var controllerPod *corev1.Pod
+		var speakerPods map[string]*corev1.Pod
+
+		ginkgo.BeforeEach(func() {
+			pods, err := cs.CoreV1().Pods("metallb-system").List(context.Background(), metav1.ListOptions{
+				LabelSelector: "component=controller",
+			})
+			framework.ExpectNoError(err)
+			framework.ExpectEqual(len(pods.Items), 1, "More than one controller found")
+			controllerPod = &pods.Items[0]
+
+			speakers, err := cs.CoreV1().Pods("metallb-system").List(context.Background(), metav1.ListOptions{
+				LabelSelector: "component=speaker",
+			})
+			framework.ExpectNoError(err)
+			speakerPods = map[string]*corev1.Pod{}
+			for _, item := range speakers.Items {
+				i := item
+				speakerPods[i.Spec.NodeName] = &i
+			}
+		})
+
+		table.DescribeTable("should be exposed by the controller", func(ipFamily string) {
+			poolName := "l2-metrics-test"
+
+			configData := configFile{
+				Pools: []addressPool{
+					{
+						Name:     poolName,
+						Protocol: Layer2,
+						Addresses: []string{
+							ipv4ServiceRange,
+							ipv6ServiceRange,
+						},
+					},
+				},
+			}
+			poolCount, err := poolCount(configData.Pools[0])
+			framework.ExpectNoError(err)
+
+			err = updateConfigMap(cs, configData)
+			framework.ExpectNoError(err)
+
+			ginkgo.By("checking the metrics when no service is added")
+			gomega.Eventually(func() error {
+				controllerMetrics, err := metrics.ForPod(controllerPod, controllerPod)
+				if err != nil {
+					return err
+				}
+				err = validateGaugeValue(0, "metallb_allocator_addresses_in_use_total", map[string]string{"pool": poolName}, controllerMetrics)
+				if err != nil {
+					return err
+				}
+				err = validateGaugeValue(int(poolCount), "metallb_allocator_addresses_total", map[string]string{"pool": poolName}, controllerMetrics)
+				if err != nil {
+					return err
+				}
+				return nil
+			}, 2*time.Minute, 1*time.Second).Should(gomega.BeNil())
+
+			ginkgo.By("creating a service")
+			svc, _ := createServiceWithBackend(cs, f.Namespace.Name, corev1.ServiceExternalTrafficPolicyTypeCluster)
+			defer func() {
+				err := cs.CoreV1().Services(svc.Namespace).Delete(context.TODO(), svc.Name, metav1.DeleteOptions{})
+				framework.ExpectNoError(err)
+			}()
+
+			ginkgo.By("checking the metrics when a service is added")
+			gomega.Eventually(func() error {
+				controllerMetrics, err := metrics.ForPod(controllerPod, controllerPod)
+				if err != nil {
+					return err
+				}
+				err = validateGaugeValue(1, "metallb_allocator_addresses_in_use_total", map[string]string{"pool": poolName}, controllerMetrics)
+				if err != nil {
+					return err
+				}
+				return nil
+			}, 2*time.Minute, 1*time.Second).Should(gomega.BeNil())
+
+			port := strconv.Itoa(int(svc.Spec.Ports[0].Port))
+			ingressIP := e2eservice.GetIngressPoint(
+				&svc.Status.LoadBalancer.Ingress[0])
+
+			ginkgo.By("checking connectivity to its external VIP")
+			hostport := net.JoinHostPort(ingressIP, port)
+			address := fmt.Sprintf("http://%s/", hostport)
+			err = wgetRetry(address, executor.Host)
+			framework.ExpectNoError(err)
+
+			allNodes, err := cs.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
+			framework.ExpectNoError(err)
+
+			advNode, err := advertisingNodeFromMAC(allNodes.Items, ingressIP, executor.Host)
+			framework.ExpectNoError(err)
+			advSpeaker, ok := speakerPods[advNode.Name]
+			framework.ExpectEqual(ok, true, fmt.Sprintf("could not find speaker pod on announcing node %s", advNode.Name))
+			delete(speakerPods, advSpeaker.Spec.NodeName)
+
+			gomega.Eventually(func() error {
+				speakerMetrics, err := metrics.ForPod(controllerPod, advSpeaker)
+				if err != nil {
+					return err
+				}
+
+				err = validateGaugeValue(1, "metallb_speaker_announced", map[string]string{"node": advSpeaker.Spec.NodeName, "protocol": "layer2", "service": fmt.Sprintf("%s/%s", f.Namespace.Name, svc.Name)}, speakerMetrics)
+				if err != nil {
+					return err
+				}
+
+				err = validateCounterValue(1, "metallb_layer2_requests_received", map[string]string{"ip": ingressIP}, speakerMetrics)
+				if err != nil {
+					return err
+				}
+
+				err = validateCounterValue(1, "metallb_layer2_responses_sent", map[string]string{"ip": ingressIP}, speakerMetrics)
+				if err != nil {
+					return err
+				}
+
+				return nil
+			}, 2*time.Minute, 1*time.Second).Should(gomega.BeNil())
+
+			// Negative - validate that the other speakers don't publish layer2 metrics
+			for _, p := range speakerPods {
+				speakerMetrics, err := metrics.ForPod(controllerPod, p)
+				framework.ExpectNoError(err)
+
+				err = validateGaugeValue(1, "metallb_speaker_announced", map[string]string{"node": p.Spec.NodeName, "protocol": "layer2", "service": fmt.Sprintf("%s/%s", f.Namespace.Name, svc.Name)}, speakerMetrics)
+				framework.ExpectError(err, fmt.Sprintf("metallb_speaker_announced present in node: %s", p.Spec.NodeName))
+			}
+
+		},
+			table.Entry("IPV4 - Checking service", "ipv4"),
+			table.Entry("IPV6 - Checking service", "ipv6"))
+	})
 })
 
 func firstIPFromRange(ipRange string) string {
@@ -307,3 +442,80 @@ func getEndpointHostName(ep string, exec executor.Executor) (string, error) {
 
 	return res, nil
 }
+
+// TODO: The tests find the announcing node in multiple ways (MAC/Events).
+// We should have a test that verifies that they all return the same node.
+func advertisingNodeFromMAC(nodes []corev1.Node, ip string, exc executor.Executor) (*corev1.Node, error) {
+	err := mac.UpdateNodeCache(nodes, exc)
+	if err != nil {
+		return nil, err
+	}
+
+	macAddr, err := mac.ForIP(ip, exc)
+	if err != nil {
+		return nil, err
+	}
+
+	advNode, err := mac.MatchNode(nodes, macAddr, exc)
+	if err != nil {
+		return nil, err
+	}
+
+	return advNode, err
+}
+
+// taken from internal: poolCount returns the number of addresses in the pool.
+// TODO: find a better place for this func.
+func poolCount(p addressPool) (int64, error) {
+	var total int64
+	for _, r := range p.Addresses {
+		cidrs, err := internalconfig.ParseCIDR(r)
+		if err != nil {
+			return 0, err
+		}
+		for _, cidr := range cidrs {
+			o, b := cidr.Mask.Size()
+			if b-o >= 62 {
+				// An enormous ipv6 range is allocated which will never run out.
+				// Just return max to avoid any math errors.
+				return math.MaxInt64, nil
+			}
+			sz := int64(math.Pow(2, float64(b-o)))
+
+			cur := ipaddr.NewCursor([]ipaddr.Prefix{*ipaddr.NewPrefix(cidr)})
+			firstIP := cur.First().IP
+			lastIP := cur.Last().IP
+
+			if p.AvoidBuggyIPs {
+				if o <= 24 {
+					// A pair of buggy IPs occur for each /24 present in the range.
+					buggies := int64(math.Pow(2, float64(24-o))) * 2
+					sz -= buggies
+				} else {
+					// Ranges smaller than /24 contain 1 buggy IP if they
+					// start/end on a /24 boundary, otherwise they contain
+					// none.
+					if ipConfusesBuggyFirmwares(firstIP) {
+						sz--
+					}
+					if ipConfusesBuggyFirmwares(lastIP) {
+						sz--
+					}
+				}
+			}
+
+			total += sz
+		}
+	}
+	return total, nil
+}
+
+// taken from internal: ipConfusesBuggyFirmwares returns true if ip is an IPv4 address ending in 0 or 255.
+// TODO: find a better place for this func.
+func ipConfusesBuggyFirmwares(ip net.IP) bool {
+	ip = ip.To4()
+	if ip == nil {
+		return false
+	}
+	return ip[3] == 0 || ip[3] == 255
+}
diff --git a/e2etest/pkg/frr/config/config.go b/e2etest/pkg/frr/config/config.go
index 3bd09d65..63b9f50a 100644
--- a/e2etest/pkg/frr/config/config.go
+++ b/e2etest/pkg/frr/config/config.go
@@ -45,13 +45,14 @@ log stdout debugging
 `
 
 type RouterConfig struct {
-	ASN       uint32
-	Neighbors []*NeighborConfig
-	BGPPort   uint16
-	RouterID  string
-	Password  string
-	HoldTime  string
-	IPFamily  string
+	ASN           uint32
+	Neighbors     []*NeighborConfig
+	BGPPort       uint16
+	RouterID      string
+	Password      string
+	HoldTime      string
+	KeepaliveTime string
+	IPFamily      string
 }
 
 type NeighborConfig struct {
diff --git a/internal/bgp/frr/frr_test.go b/internal/bgp/frr/frr_test.go
index 12a106ab..e245e36a 100644
--- a/internal/bgp/frr/frr_test.go
+++ b/internal/bgp/frr/frr_test.go
@@ -82,7 +82,7 @@ func TestSingleSession(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -96,7 +96,7 @@ func TestSingleSessionClose(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -109,12 +109,12 @@ func TestTwoSessions(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
 	defer session1.Close()
-	session2, err := sessionManager.NewSession(l, "10.4.4.254:179", net.ParseIP("10.3.3.254"), 300, net.ParseIP("10.3.3.254"), 400, time.Second, "password", "hostname")
+	session2, err := sessionManager.NewSession(l, "10.4.4.254:179", net.ParseIP("10.3.3.254"), 300, net.ParseIP("10.3.3.254"), 400, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -128,12 +128,12 @@ func TestTwoSessionsDuplicate(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
 	defer session1.Close()
-	session2, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session2, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -147,12 +147,12 @@ func TestTwoSessionsDuplicateRouter(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
 	defer session1.Close()
-	session2, err := sessionManager.NewSession(l, "10.4.4.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 400, time.Second, "password", "hostname")
+	session2, err := sessionManager.NewSession(l, "10.4.4.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 400, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -166,7 +166,7 @@ func TestSingleAdvertisement(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -195,7 +195,7 @@ func TestSingleAdvertisementNoRouterID(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, nil, 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, nil, 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -224,7 +224,7 @@ func TestSingleAdvertisementInvalidPrefix(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -250,7 +250,7 @@ func TestSingleAdvertisementInvalidNoPort(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err == nil {
 		session.Close()
 		t.Fatalf("Should not be able to create session")
@@ -265,7 +265,7 @@ func TestSingleAdvertisementInvalidNextHop(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -293,7 +293,7 @@ func TestSingleAdvertisementStop(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -327,7 +327,7 @@ func TestSingleAdvertisementChange(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
@@ -371,7 +371,7 @@ func TestTwoAdvertisements(t *testing.T) {
 
 	l := log.NewNopLogger()
 	sessionManager := NewSessionManager()
-	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, "password", "hostname")
+	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
 	}
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisement.golden b/internal/bgp/frr/testdata/TestSingleAdvertisement.golden
index a341486e..6e8b1907 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisement.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisement.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisementChange.golden b/internal/bgp/frr/testdata/TestSingleAdvertisementChange.golden
index b6488164..9b9156c4 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisementChange.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisementChange.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisementInvalid.golden b/internal/bgp/frr/testdata/TestSingleAdvertisementInvalid.golden
index 244ed543..c5ee2a62 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisementInvalid.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisementInvalid.golden
@@ -8,7 +8,7 @@ router bgp 100
   bgp router-id 10.1.1.254
 
   neighbor 10.2.2.254 remote-as 200
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisementInvalidPrefix.golden b/internal/bgp/frr/testdata/TestSingleAdvertisementInvalidPrefix.golden
index 21768dd8..120b24f6 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisementInvalidPrefix.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisementInvalidPrefix.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisementNoRouterID.golden b/internal/bgp/frr/testdata/TestSingleAdvertisementNoRouterID.golden
index 6099f2c8..d259cd3a 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisementNoRouterID.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisementNoRouterID.golden
@@ -11,7 +11,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleAdvertisementStop.golden b/internal/bgp/frr/testdata/TestSingleAdvertisementStop.golden
index 21768dd8..120b24f6 100644
--- a/internal/bgp/frr/testdata/TestSingleAdvertisementStop.golden
+++ b/internal/bgp/frr/testdata/TestSingleAdvertisementStop.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestSingleSession.golden b/internal/bgp/frr/testdata/TestSingleSession.golden
index 21768dd8..120b24f6 100644
--- a/internal/bgp/frr/testdata/TestSingleSession.golden
+++ b/internal/bgp/frr/testdata/TestSingleSession.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestTwoAdvertisements.golden b/internal/bgp/frr/testdata/TestTwoAdvertisements.golden
index ae9fc482..328eb680 100644
--- a/internal/bgp/frr/testdata/TestTwoAdvertisements.golden
+++ b/internal/bgp/frr/testdata/TestTwoAdvertisements.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestTwoSessions.golden b/internal/bgp/frr/testdata/TestTwoSessions.golden
index 5177eed2..3a4e341f 100644
--- a/internal/bgp/frr/testdata/TestTwoSessions.golden
+++ b/internal/bgp/frr/testdata/TestTwoSessions.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
@@ -32,7 +32,7 @@ router bgp 300
 
   neighbor 10.4.4.254 remote-as 400
   neighbor 10.4.4.254 port 179
-  neighbor 10.4.4.254 timers 0 1
+  neighbor 10.4.4.254 timers 1 1
   neighbor 10.4.4.254 password password
   neighbor 10.4.4.254 update-source 10.3.3.254
 
diff --git a/internal/bgp/frr/testdata/TestTwoSessionsDuplicate.golden b/internal/bgp/frr/testdata/TestTwoSessionsDuplicate.golden
index 21768dd8..120b24f6 100644
--- a/internal/bgp/frr/testdata/TestTwoSessionsDuplicate.golden
+++ b/internal/bgp/frr/testdata/TestTwoSessionsDuplicate.golden
@@ -12,7 +12,7 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
 
diff --git a/internal/bgp/frr/testdata/TestTwoSessionsDuplicateRouter.golden b/internal/bgp/frr/testdata/TestTwoSessionsDuplicateRouter.golden
index 006c180c..b0821edc 100644
--- a/internal/bgp/frr/testdata/TestTwoSessionsDuplicateRouter.golden
+++ b/internal/bgp/frr/testdata/TestTwoSessionsDuplicateRouter.golden
@@ -12,12 +12,12 @@ router bgp 100
 
   neighbor 10.2.2.254 remote-as 200
   neighbor 10.2.2.254 port 179
-  neighbor 10.2.2.254 timers 0 1
+  neighbor 10.2.2.254 timers 1 1
   neighbor 10.2.2.254 password password
   neighbor 10.2.2.254 update-source 10.1.1.254
   neighbor 10.4.4.254 remote-as 400
   neighbor 10.4.4.254 port 179
-  neighbor 10.4.4.254 timers 0 1
+  neighbor 10.4.4.254 timers 1 1
   neighbor 10.4.4.254 password password
   neighbor 10.4.4.254 update-source 10.1.1.254
 
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index c57f69c9..91a94551 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -106,6 +106,7 @@ address-pools:
 						SrcAddr:       net.ParseIP("10.20.30.40"),
 						Port:          1179,
 						HoldTime:      180 * time.Second,
+						KeepaliveTime: 60 * time.Second,
 						RouterID:      net.ParseIP("10.20.30.40"),
 						NodeSelectors: []labels.Selector{labels.Everything()},
 					},
@@ -115,6 +116,7 @@ address-pools:
 						Addr:          net.ParseIP("2.3.4.5"),
 						Port:          179,
 						HoldTime:      90 * time.Second,
+						KeepaliveTime: 30 * time.Second,
 						NodeSelectors: []labels.Selector{selector("bar in (quux),foo=bar")},
 					},
 				},
@@ -196,6 +198,7 @@ peers:
 						Addr:          net.ParseIP("1.2.3.4"),
 						Port:          179,
 						HoldTime:      90 * time.Second,
+						KeepaliveTime: 30 * time.Second,
 						NodeSelectors: []labels.Selector{labels.Everything()},
 					},
 				},
@@ -252,7 +255,6 @@ peers:
   hold-time: 1s
 `,
 		},
-
 		{
 			desc: "invalid router ID",
 			raw: `
@@ -280,6 +282,7 @@ peers:
 						Addr:          net.ParseIP("1.2.3.4"),
 						Port:          179,
 						HoldTime:      90 * time.Second,
+						KeepaliveTime: 30 * time.Second,
 						NodeSelectors: []labels.Selector{labels.Everything()},
 					},
 				},
diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go
index c818adfa..dea685ec 100644
--- a/speaker/bgp_controller_test.go
+++ b/speaker/bgp_controller_test.go
@@ -110,7 +110,7 @@ type fakeBGPSessionManager struct {
 	gotAds map[string][]*bgp.Advertisement
 }
 
-func (f *fakeBGPSessionManager) NewSession(_ log.Logger, addr string, _ net.IP, _ uint32, _ net.IP, _ uint32, _ time.Duration, _, _ string) (bgp.Session, error) {
+func (f *fakeBGPSessionManager) NewSession(_ log.Logger, addr string, _ net.IP, _ uint32, _ net.IP, _ uint32, _ time.Duration, _ time.Duration, _ string, _ string) (bgp.Session, error) {
 	f.Lock()
 	defer f.Unlock()