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

Refactor the frr configuration template #1714

Merged
merged 7 commits into from Nov 25, 2022
Merged

Refactor the frr configuration template #1714

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec3857c
Refactor the frr configuration template
fedepaol Nov 16, 2022
74c29ce
FRR: move the templates in separate files
fedepaol Nov 17, 2022
b7ed3c9
E2E metrics tests: remove checking metallb_bgp_notifications_sent
fedepaol Nov 17, 2022
86dcdab
FRR templates: make the order deterministic
fedepaol Nov 24, 2022
bec3183
Merge branch 'main' into refactorff
fedepaol Nov 24, 2022
0f925ec
Merge branch 'main' into refactorff
fedepaol Nov 25, 2022
6121bfb
Merge branch 'main' into refactorff
fedepaol Nov 25, 2022
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
244 changes: 153 additions & 91 deletions internal/bgp/frr/config.go
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/pkg/errors"
"go.universe.tf/metallb/internal/ipfamily"
)

Expand All @@ -28,7 +29,116 @@ var reloaderPidFileName = "/etc/frr_reloader/reloader.pid"
// It may be necessary to arrange this into multiple nested templates
// (https://pkg.go.dev/text/template#hdr-Nested_template_definitions), this
// should also be considered.

const configTemplate = `
{{- define "localpreffilter" -}}
{{frrIPFamily .advertisement.IPFamily}} prefix-list {{localPrefPrefixList .neighbor .advertisement.LocalPref}} permit {{.advertisement.Prefix}}
route-map {{.neighbor.Addr}}-out permit {{counter .neighbor.Addr}}
match {{frrIPFamily .advertisement.IPFamily}} address prefix-list {{localPrefPrefixList .neighbor .advertisement.LocalPref}}
set local-preference {{.advertisement.LocalPref}}
on-match next
{{- end -}}

{{- define "communityfilter" -}}
{{frrIPFamily .advertisement.IPFamily}} prefix-list {{communityPrefixList .neighbor .community}} permit {{.advertisement.Prefix}}
route-map {{.neighbor.Addr}}-out permit {{counter .neighbor.Addr}}
match {{frrIPFamily .advertisement.IPFamily}} address prefix-list {{communityPrefixList .neighbor .community}}
set community {{.community}} additive
on-match next
{{- end -}}

{{- /* The prefixes are per router in FRR, but MetalLB api allows to associate a given BGPAdvertisement to a service IP,
and a given advertisement contains both the properties of the announcement (i.e. community) and the list of peers
we may want to advertise to. Because of this, for each neighbor we must opt-in and allow the advertisement, and
deny all the others.*/ -}}
{{- define "neighborfilters" -}}

route-map {{.neighbor.Addr}}-in deny 20
{{- range $a := .neighbor.Advertisements }}
{{/* Advertisements for which we must enable set the local pref */}}
{{- if not (eq $a.LocalPref 0)}}
{{template "localpreffilter" dict "advertisement" $a "neighbor" $.neighbor}}
{{- end -}}

{{/* Advertisements for which we must enable the community property */}}
{{- range $c := $a.Communities }}
{{template "communityfilter" dict "advertisement" $a "neighbor" $.neighbor "community" $c}}
{{- end }}
{{/* this advertisement is allowed to the specific neighbor */}}
{{frrIPFamily $a.IPFamily}} prefix-list {{allowedPrefixList $.neighbor}} permit {{$a.Prefix}}
{{- end }}

route-map {{$.neighbor.Addr}}-out permit {{counter $.neighbor.Addr}}
match ip address prefix-list {{allowedPrefixList $.neighbor}}
route-map {{$.neighbor.Addr}}-out permit {{counter $.neighbor.Addr}}
match ipv6 address prefix-list {{allowedPrefixList $.neighbor}}

ip prefix-list {{allowedPrefixList $.neighbor }} deny any
ipv6 prefix-list {{allowedPrefixList $.neighbor}} deny any
{{- end -}}

{{- define "neighborsession"}}
neighbor {{.neighbor.Addr}} remote-as {{.neighbor.ASN}}
{{- if .neighbor.EBGPMultiHop }}
neighbor {{.neighbor.Addr}} ebgp-multihop
{{- end }}
{{ if .neighbor.Port -}}
neighbor {{.neighbor.Addr}} port {{.neighbor.Port}}
{{- end }}
neighbor {{.neighbor.Addr}} timers {{.neighbor.KeepaliveTime}} {{.neighbor.HoldTime}}
{{ if .neighbor.Password -}}
neighbor {{.neighbor.Addr}} password {{.neighbor.Password}}
{{- end }}
{{ if .neighbor.SrcAddr -}}
neighbor {{.neighbor.Addr}} update-source {{.neighbor.SrcAddr}}
{{- end }}
{{- if ne .neighbor.BFDProfile ""}}
neighbor {{.neighbor.Addr}} bfd profile {{.neighbor.BFDProfile}}
{{- end }}
{{- if mustDisableConnectedCheck .neighbor.IPFamily .routerASN .neighbor.ASN .neighbor.EBGPMultiHop }}
neighbor {{.neighbor.Addr}} disable-connected-check
{{- end }}
{{- end -}}

{{- define "neighborenableipfamily"}}
{{/* no bgp default ipv4-unicast prevents peering if no address families are defined. We declare an ipv4 one for the peer to make the pairing happen */}}
address-family ipv4 unicast
neighbor {{.Addr}} activate
neighbor {{.Addr}} route-map {{.Addr}}-in in
neighbor {{.Addr}} route-map {{.Addr}}-out out
exit-address-family
address-family ipv6 unicast
neighbor {{.Addr}} activate
neighbor {{.Addr}} route-map {{.Addr}}-in in
neighbor {{.Addr}} route-map {{.Addr}}-out out
exit-address-family
{{- end -}}

{{- define "bfdprofile" }}
profile {{.profile.Name}}
{{ if .profile.ReceiveInterval -}}
receive-interval {{.profile.ReceiveInterval}}
{{end -}}
{{ if .profile.TransmitInterval -}}
transmit-interval {{.profile.TransmitInterval}}
{{end -}}
{{ if .profile.DetectMultiplier -}}
detect-multiplier {{.profile.DetectMultiplier}}
{{end -}}
{{ if .profile.EchoMode -}}
echo-mode
{{end -}}
{{ if .profile.EchoInterval -}}
echo-interval {{.profile.EchoInterval}}
{{end -}}
{{ if .profile.PassiveMode -}}
passive-mode
{{end -}}
{{ if .profile.MinimumTTL -}}
minimum-ttl {{ .profile.MinimumTTL }}
{{end -}}
{{- end -}}

log file /etc/frr/frr.log {{.Loglevel}}
log timestamp precision 3
{{- if eq .Loglevel "debugging" }}
Expand All @@ -51,31 +161,8 @@ ip nht resolve-via-default
ipv6 nht resolve-via-default

{{- range .Routers }}
{{- range $n := .Neighbors }}
route-map {{$n.Addr}}-in deny 20
{{- range $a := .Advertisements }}
{{- if not (eq $a.LocalPref 0)}}
{{frrIPFamily $a.IPFamily}} prefix-list {{localPrefPrefixList $n $a.LocalPref}} permit {{$a.Prefix}}
route-map {{$n.Addr}}-out permit {{counter $n.Addr}}
match {{frrIPFamily $a.IPFamily}} address prefix-list {{localPrefPrefixList $n $a.LocalPref}}
set local-preference {{$a.LocalPref}}
on-match next
{{- end }}
{{- range $c := $a.Communities }}
{{frrIPFamily $a.IPFamily}} prefix-list {{communityPrefixList $n $c}} permit {{$a.Prefix}}
route-map {{$n.Addr}}-out permit {{counter $n.Addr}}
match {{frrIPFamily $a.IPFamily}} address prefix-list {{communityPrefixList $n $c}}
set community {{$c}} additive
on-match next
{{- end }}
{{frrIPFamily $a.IPFamily}} prefix-list {{allowedPrefixList $n}} permit {{$a.Prefix}}
{{- end }}
route-map {{$n.Addr}}-out permit {{counter $n.Addr}}
match ip address prefix-list {{allowedPrefixList $n}}
route-map {{$n.Addr}}-out permit {{counter $n.Addr}}
match ipv6 address prefix-list {{allowedPrefixList $n}}
ip prefix-list {{allowedPrefixList $n}} deny any
ipv6 prefix-list {{allowedPrefixList $n}} deny any
{{- range .Neighbors }}
{{template "neighborfilters" dict "neighbor" .}}
{{- end }}
{{- end }}

Expand All @@ -87,79 +174,37 @@ router bgp {{$r.MyASN}}
{{ if $r.RouterId }}
bgp router-id {{$r.RouterId}}
{{- end }}
{{range .Neighbors }}
neighbor {{.Addr}} remote-as {{.ASN}}
{{- if .EBGPMultiHop }}
neighbor {{.Addr}} ebgp-multihop
{{- end }}
{{ if .Port -}}
neighbor {{.Addr}} port {{.Port}}
{{- end }}
neighbor {{.Addr}} timers {{.KeepaliveTime}} {{.HoldTime}}
{{ if .Password -}}
neighbor {{.Addr}} password {{.Password}}
{{- end }}
{{ if .SrcAddr -}}
neighbor {{.Addr}} update-source {{.SrcAddr}}
{{- end }}
{{- if ne .BFDProfile ""}}
neighbor {{.Addr}} bfd profile {{.BFDProfile}}
{{- end }}
{{- if mustDisableConnectedCheck .IPFamily $r.MyASN .ASN .EBGPMultiHop }}
neighbor {{.Addr}} disable-connected-check
{{- end }}

{{- range .Neighbors }}
{{- template "neighborsession" dict "neighbor" . "routerASN" $r.MyASN -}}
{{- end }}
{{range $n := .Neighbors -}}
{{/* no bgp default ipv4-unicast prevents peering if no address families are defined. We declare an ipv4 one for the peer to make the pairing happen */}}
{{- if eq (len .Advertisements) 0}}

{{- range $n := .Neighbors -}}
{{- template "neighborenableipfamily" . -}}
{{end -}}

{{- if gt (len .IPV4Prefixes) 0}}
address-family ipv4 unicast
neighbor {{$n.Addr}} activate
neighbor {{$n.Addr}} route-map {{$n.Addr}}-in in
neighbor {{$n.Addr}} route-map {{$n.Addr}}-out out
{{- range .IPV4Prefixes }}
network {{.}}
{{- end}}
exit-address-family
{{end }}

{{- if gt (len .IPV6Prefixes) 0}}
address-family ipv6 unicast
neighbor {{$n.Addr}} activate
neighbor {{$n.Addr}} route-map {{$n.Addr}}-in in
neighbor {{$n.Addr}} route-map {{$n.Addr}}-out out
exit-address-family
{{- range .IPV6Prefixes }}
network {{.}}
{{- end}}
{{- range .Advertisements }}
address-family {{.IPFamily.String}} unicast
neighbor {{$n.Addr}} activate
neighbor {{$n.Addr}} route-map {{$n.Addr}}-in in
network {{.Prefix}}
neighbor {{$n.Addr}} route-map {{$n.Addr}}-out out
exit-address-family
{{- end}}
{{end -}}
{{end }}
{{end }}
{{- if gt (len .BFDProfiles) 0}}
bfd
{{- range .BFDProfiles }}
profile {{.Name}}
{{ if .ReceiveInterval -}}
receive-interval {{.ReceiveInterval}}
{{end -}}
{{ if .TransmitInterval -}}
transmit-interval {{.TransmitInterval}}
{{end -}}
{{ if .DetectMultiplier -}}
detect-multiplier {{.DetectMultiplier}}
{{end -}}
{{ if .EchoMode -}}
echo-mode
{{end -}}
{{ if .EchoInterval -}}
echo-interval {{.EchoInterval}}
{{end -}}
{{ if .PassiveMode -}}
passive-mode
{{end -}}
{{ if .MinimumTTL -}}
minimum-ttl {{ .MinimumTTL }}
{{end -}}
{{ end }}
{{ end }}`
{{- template "bfdprofile" dict "profile" . -}}
{{- end }}
{{- end }}`

type frrConfig struct {
Loglevel string
Expand All @@ -177,9 +222,12 @@ type reloadEvent struct {
// to all the neighbors. Once this constraint is changed, we may need prefix-lists per neighbor.

type routerConfig struct {
MyASN uint32
RouterId string
Neighbors map[string]*neighborConfig
MyASN uint32
RouterId string
Neighbors map[string]*neighborConfig
VRF string
IPV4Prefixes []string
IPV6Prefixes []string
}

type BFDProfile struct {
Expand Down Expand Up @@ -264,6 +312,20 @@ func templateConfig(data interface{}) (string, error) {
}
return false
},
"dict": func(values ...interface{}) (map[string]interface{}, error) {
if len(values)%2 != 0 {
return nil, errors.New("invalid dict call, expecting even number of args")
}
dict := make(map[string]interface{}, len(values)/2)
for i := 0; i < len(values); i += 2 {
key, ok := values[i].(string)
if !ok {
return nil, fmt.Errorf("dict keys must be strings, got %v %T", values[i], values[i])
}
dict[key] = values[i+1]
}
return dict, nil
},
}).Parse(configTemplate)
if err != nil {
return "", err
Expand Down
18 changes: 17 additions & 1 deletion internal/bgp/frr/frr.go
View file
Open in desktop
Expand Up @@ -216,6 +216,7 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
var neighbor *neighborConfig
var exist bool

routerAnnouncements := make(map[string]struct{}, 0)
routerName := routerName(s.routerID.String(), s.myASN)
if router, exist = config.Routers[routerName]; !exist {
router = &routerConfig{
Expand Down Expand Up @@ -277,15 +278,30 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
community := metallbconfig.CommunityToString(c)
communities = append(communities, community)
}

prefix := adv.Prefix.String()
advConfig := advertisementConfig{
IPFamily: family,
Prefix: adv.Prefix.String(),
Prefix: prefix,
Communities: communities,
LocalPref: adv.LocalPref,
}

neighbor.Advertisements = append(neighbor.Advertisements, &advConfig)
if _, ok := routerAnnouncements[prefix]; !ok {
switch family {
case ipfamily.IPv4:
router.IPV4Prefixes = append(router.IPV4Prefixes, prefix)
case ipfamily.IPv6:
router.IPV6Prefixes = append(router.IPV6Prefixes, prefix)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should check here if the prefix already exists in the slice, to prevent duplications.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's the if that was supposed to prevent dupes. Need to check why it didn't work

}
routerAnnouncements[prefix] = struct{}{}
}
}
// This is required to make the rendering stable, because it depends on the order the neighbor
// are iterated and that's a map.
sort.Strings(router.IPV4Prefixes)
sort.Strings(router.IPV6Prefixes)
}

return config, nil
Expand Down
3 changes: 1 addition & 2 deletions internal/bgp/frr/testdata/TestBFDProfileAllDefault.golden
View file
Open in desktop
@@ -1,4 +1,3 @@

log file /etc/frr/frr.log informational
log timestamp precision 3
hostname dummyhostname
Expand All @@ -15,4 +14,4 @@ bfd
detect-multiplier 5
echo-interval 90
minimum-ttl 60


3 changes: 1 addition & 2 deletions internal/bgp/frr/testdata/TestBFDProfileCornerCases.golden
View file
Open in desktop
@@ -1,4 +1,3 @@

log file /etc/frr/frr.log informational
log timestamp precision 3
hostname dummyhostname
Expand All @@ -15,4 +14,4 @@ bfd
echo-interval 90
passive-mode
minimum-ttl 60


3 changes: 1 addition & 2 deletions internal/bgp/frr/testdata/TestBFDProfileNoSessions.golden
View file
Open in desktop
@@ -1,4 +1,3 @@

log file /etc/frr/frr.log informational
log timestamp precision 3
hostname dummyhostname
Expand All @@ -20,4 +19,4 @@ bfd
detect-multiplier 5
echo-interval 90
minimum-ttl 60