config: Properly set config of PeerGroup member

Currently, the config of PeerGroup members are not set properly.

For example, if LocalAs of the neighbor is not set in the config file,
LocalAs will be set to Global.As as a default value.
After this, however, if the neighbor is a member of a peer group,
LocalAs will be unexpectedly overwritten by the peer group config.

This commit fixes this by setting default values after
applying the peer group config.

Adding to this, this commit avoids unnecessary applying
the peer group config.

Signed-off-by: Satoshi Fujimoto <[email protected]>
This commit is contained in:
Satoshi Fujimoto
2017-11-08 10:14:21 +09:00
parent 5c16d53c7f
commit ab95321191
5 changed files with 76 additions and 65 deletions
+36 -58
View File
@@ -101,11 +101,17 @@ func isLocalLinkLocalAddress(ifindex int, addr net.IP) (bool, error) {
return false, nil
}
func SetDefaultNeighborConfigValues(n *Neighbor, g *Global) error {
return setDefaultNeighborConfigValuesWithViper(nil, n, g)
func SetDefaultNeighborConfigValues(n *Neighbor, pg *PeerGroup, g *Global) error {
// Determines this function is called against the same Neighbor struct,
// and if already called, returns immediately.
if n.State.LocalAs != 0 {
return nil
}
return setDefaultNeighborConfigValuesWithViper(nil, n, g, pg)
}
func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, g *Global) error {
func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, g *Global, pg *PeerGroup) error {
if n == nil {
return fmt.Errorf("neighbor config is nil")
}
@@ -114,14 +120,19 @@ func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, g *Glo
}
if v == nil {
// Determines this function is called against the same Neighbor struct,
// and if already called, returns immediately.
if n.State.LocalAs != 0 {
return nil
}
v = viper.New()
}
if pg != nil {
if err := OverwriteNeighborConfigWithPeerGroup(n, pg); err != nil {
return err
}
}
if n.Config.PeerAs == 0 {
return fmt.Errorf("peer-as is not set for %s", n.Config.NeighborAddress)
}
if n.Config.LocalAs == 0 {
n.Config.LocalAs = g.Config.As
if !g.Confederation.Config.Enabled || n.IsConfederation(g) {
@@ -328,45 +339,6 @@ func setDefaultPolicyConfigValuesWithViper(v *viper.Viper, p *PolicyDefinition)
return nil
}
func validatePeerGroupConfig(n *Neighbor, b *BgpConfigSet) error {
name := n.Config.PeerGroup
if name == "" {
return nil
}
pg, err := getPeerGroup(name, b)
if err != nil {
return err
}
if pg.Config.PeerAs != 0 && n.Config.PeerAs != 0 {
return fmt.Errorf("Cannot configure remote-as for members. PeerGroup AS %d.", pg.Config.PeerAs)
}
return nil
}
func getPeerGroup(n string, b *BgpConfigSet) (*PeerGroup, error) {
if n == "" {
return nil, fmt.Errorf("peer-group name is not configured")
}
for _, pg := range b.PeerGroups {
if n == pg.Config.PeerGroupName {
return &pg, nil
}
}
return nil, fmt.Errorf("No such peer-group: %s", n)
}
func validateDynamicNeighborConfig(d *DynamicNeighborConfig, b *BgpConfigSet) error {
if _, err := getPeerGroup(d.PeerGroup, b); err != nil {
return err
}
if _, _, err := net.ParseCIDR(d.Prefix); err != nil {
return fmt.Errorf("Invalid Dynamic Neighbor prefix %s", d.Prefix)
}
return nil
}
func setDefaultConfigValuesWithViper(v *viper.Viper, b *BgpConfigSet) error {
if v == nil {
v = viper.New()
@@ -411,26 +383,32 @@ func setDefaultConfigValuesWithViper(v *viper.Viper, b *BgpConfigSet) error {
}
for idx, n := range b.Neighbors {
if err := validatePeerGroupConfig(&n, b); err != nil {
return err
}
vv := viper.New()
if len(list) > idx {
vv.Set("neighbor", list[idx])
}
if err := setDefaultNeighborConfigValuesWithViper(vv, &n, &b.Global); err != nil {
pg, err := b.getPeerGroup(n.Config.PeerGroup)
if err != nil {
return nil
}
if pg != nil {
identifier := vv.Get("neighbor.config.neighbor-address")
if identifier == nil {
identifier = vv.Get("neighbor.config.neighbor-interface")
}
RegisterConfiguredFields(identifier.(string), list[idx])
}
if err := setDefaultNeighborConfigValuesWithViper(vv, &n, &b.Global, pg); err != nil {
return err
}
b.Neighbors[idx] = n
if n.Config.PeerGroup != "" {
RegisterConfiguredFields(vv.Get("neighbor.config.neighbor-address").(string), list[idx])
}
}
for _, d := range b.DynamicNeighbors {
if err := validateDynamicNeighborConfig(&d.Config, b); err != nil {
if err := d.validate(b); err != nil {
return err
}
}
@@ -463,7 +441,7 @@ func setDefaultConfigValuesWithViper(v *viper.Viper, b *BgpConfigSet) error {
func OverwriteNeighborConfigWithPeerGroup(c *Neighbor, pg *PeerGroup) error {
v := viper.New()
val, ok := configuredFields[c.State.NeighborAddress]
val, ok := configuredFields[c.Config.NeighborAddress]
if ok {
v.Set("neighbor", val)
} else {
+26
View File
@@ -40,6 +40,32 @@ func detectConfigFileType(path, def string) string {
}
}
func (b *BgpConfigSet) getPeerGroup(n string) (*PeerGroup, error) {
if n == "" {
return nil, nil
}
for _, pg := range b.PeerGroups {
if n == pg.Config.PeerGroupName {
return &pg, nil
}
}
return nil, fmt.Errorf("no such peer-group: %s", n)
}
func (d *DynamicNeighbor) validate(b *BgpConfigSet) error {
if d.Config.PeerGroup == "" {
return fmt.Errorf("dynamic neighbor requires the peer group config")
}
if _, err := b.getPeerGroup(d.Config.PeerGroup); err != nil {
return err
}
if _, _, err := net.ParseCIDR(d.Config.Prefix); err != nil {
return fmt.Errorf("invalid dynamic neighbor prefix %s", d.Config.Prefix)
}
return nil
}
func (n *Neighbor) IsConfederationMember(g *Global) bool {
for _, member := range g.Confederation.Config.MemberAsList {
if member == n.Config.PeerAs {
+1 -1
View File
@@ -79,7 +79,7 @@ func newDynamicPeer(g *config.Global, neighborAddress string, pg *config.PeerGro
}).Debugf("Can't overwrite neighbor config: %s", err)
return nil
}
if err := config.SetDefaultNeighborConfigValues(&conf, g); err != nil {
if err := config.SetDefaultNeighborConfigValues(&conf, pg, g); err != nil {
log.WithFields(log.Fields{
"Topic": "Peer",
"Key": neighborAddress,
+12 -5
View File
@@ -1762,13 +1762,16 @@ func (server *BgpServer) addNeighbor(c *config.Neighbor) error {
return fmt.Errorf("Can't overwrite the existing peer: %s", addr)
}
var pgConf *config.PeerGroup
if c.Config.PeerGroup != "" {
if err := config.OverwriteNeighborConfigWithPeerGroup(c, server.peerGroupMap[c.Config.PeerGroup].Conf); err != nil {
return err
pg, ok := server.peerGroupMap[c.Config.PeerGroup]
if !ok {
return fmt.Errorf("no such peer-group: %s", c.Config.PeerGroup)
}
pgConf = pg.Conf
}
if err := config.SetDefaultNeighborConfigValues(c, &server.bgpConfig.Global); err != nil {
if err := config.SetDefaultNeighborConfigValues(c, pgConf, &server.bgpConfig.Global); err != nil {
return err
}
@@ -1965,8 +1968,12 @@ func (s *BgpServer) UpdatePeerGroup(pg *config.PeerGroup) (needsSoftResetIn bool
func (s *BgpServer) updateNeighbor(c *config.Neighbor) (needsSoftResetIn bool, err error) {
if c.Config.PeerGroup != "" {
if err := config.OverwriteNeighborConfigWithPeerGroup(c, s.peerGroupMap[c.Config.PeerGroup].Conf); err != nil {
return needsSoftResetIn, err
if pg, ok := s.peerGroupMap[c.Config.PeerGroup]; ok {
if err := config.SetDefaultNeighborConfigValues(c, pg.Conf, &s.bgpConfig.Global); err != nil {
return needsSoftResetIn, err
}
} else {
return needsSoftResetIn, fmt.Errorf("no such peer-group: %s", c.Config.PeerGroup)
}
}
+1 -1
View File
@@ -201,7 +201,7 @@ func TestNumGoroutineWithAddDeleteNeighbor(t *testing.T) {
func newPeerandInfo(myAs, as uint32, address string, rib *table.TableManager) (*Peer, *table.PeerInfo) {
nConf := &config.Neighbor{Config: config.NeighborConfig{PeerAs: as, NeighborAddress: address}}
gConf := &config.Global{Config: config.GlobalConfig{As: myAs}}
config.SetDefaultNeighborConfigValues(nConf, gConf)
config.SetDefaultNeighborConfigValues(nConf, nil, gConf)
policy := table.NewRoutingPolicy()
policy.Reset(&config.RoutingPolicy{}, nil)
p := NewPeer(