zebra: Introducing MIN_ZAPIVER and MAX_ZAPIVER. And avoiding double close channel when sequential retry to connect zebra.

- Introducing MIN_ZAPIVER and MAX_ZAPIVER for avoiding editing files except for zapi.go when new ZAPI version will be supported in future.
- Fix a bug avoiding panic by double close for a channel.
- Changing algorithm for sequential retry to connect zebra.

2 files changed, 38 insertions, 11 deletions

diff --git a/internal/pkg/config/default.go b/internal/pkg/config/default.go
index 08f9f865..13e1267c 100644
--- a/internal/pkg/config/default.go
+++ b/internal/pkg/config/default.go
@@ -8,6 +8,7 @@ import (
 	"reflect"
 	"strconv"
 
+	"github.com/osrg/gobgp/internal/pkg/zebra"
 	"github.com/osrg/gobgp/pkg/packet/bgp"
 	"github.com/osrg/gobgp/pkg/packet/bmp"
 	"github.com/osrg/gobgp/pkg/packet/rtr"
@@ -400,10 +401,10 @@ func setDefaultConfigValuesWithViper(v *viper.Viper, b *BgpConfigSet) error {
 	if b.Zebra.Config.Url == "" {
 		b.Zebra.Config.Url = "unix:/var/run/quagga/zserv.api"
 	}
-	if b.Zebra.Config.Version < 2 {
-		b.Zebra.Config.Version = 2
-	} else if b.Zebra.Config.Version > 6 {
-		b.Zebra.Config.Version = 6
+	if b.Zebra.Config.Version < zebra.MinZapiVer {
+		b.Zebra.Config.Version = zebra.MinZapiVer
+	} else if b.Zebra.Config.Version > zebra.MaxZapiVer {
+		b.Zebra.Config.Version = zebra.MaxZapiVer
 	}
 	if !v.IsSet("zebra.config.nexthop-trigger-enable") && !b.Zebra.Config.NexthopTriggerEnable && b.Zebra.Config.Version > 2 {
 		b.Zebra.Config.NexthopTriggerEnable = true
diff --git a/internal/pkg/zebra/zapi.go b/internal/pkg/zebra/zapi.go
index b0f8cc3a..e4391ca6 100644
--- a/internal/pkg/zebra/zapi.go
+++ b/internal/pkg/zebra/zapi.go
@@ -17,6 +17,7 @@ package zebra
 
 import (
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -35,6 +36,11 @@ const (
 	INTERFACE_NAMSIZ  = 20
 )
 
+const (
+	MinZapiVer uint8 = 2
+	MaxZapiVer uint8 = 6
+)
+
 // Internal Interface Status.
 type INTERFACE_STATUS uint8
 
@@ -970,10 +976,10 @@ func NewClient(network, address string, typ ROUTE_TYPE, version uint8) (*Client,
 	}
 	outgoing := make(chan *Message)
 	incoming := make(chan *Message, 64)
-	if version < 2 {
-		version = 2
-	} else if version > 6 {
-		version = 6
+	if version < MinZapiVer {
+		version = MinZapiVer
+	} else if version > MaxZapiVer {
+		version = MaxZapiVer
 	}
 
 	c := &Client{
@@ -1001,7 +1007,8 @@ func NewClient(network, address string, typ ROUTE_TYPE, version uint8) (*Client,
 					log.WithFields(log.Fields{
 						"Topic": "Zebra",
 					}).Errorf("failed to write: %s", err)
-					close(outgoing)
+					ChannelClose(outgoing)
+					return
 				}
 			} else {
 				log.Debug("finish outgoing loop")
@@ -1026,6 +1033,12 @@ func NewClient(network, address string, typ ROUTE_TYPE, version uint8) (*Client,
 
 		hd := &Header{}
 		err = hd.DecodeFromBytes(headerBuf)
+		if c.Version != hd.Version {
+			log.WithFields(log.Fields{
+				"Topic": "Zebra",
+			}).Warnf("ZAPI version mismatch. configured version: %d, version of received message:%d", c.Version, hd.Version)
+			return nil, errors.New("ZAPI version mismatch")
+		}
 		if err != nil {
 			log.WithFields(log.Fields{
 				"Topic": "Zebra",
@@ -1077,7 +1090,7 @@ func NewClient(network, address string, typ ROUTE_TYPE, version uint8) (*Client,
 
 	// Start receive loop only when the first message successfully received.
 	go func() {
-		defer close(incoming)
+		defer ChannelClose(incoming)
 		for {
 			if m, err := receiveSingleMsg(); err != nil {
 				return
@@ -1298,8 +1311,21 @@ func (c *Client) SendNexthopRegister(vrfId uint32, body *NexthopRegisterBody, is
 	return c.SendCommand(command, vrfId, body)
 }
 
+// for avoiding double close
+func ChannelClose(ch chan *Message) bool {
+	select {
+	case _, ok := <-ch:
+		if ok {
+			close(ch)
+			return true
+		}
+	default:
+	}
+	return false
+}
+
 func (c *Client) Close() error {
-	close(c.outgoing)
+	ChannelClose(c.outgoing)
 	return c.conn.Close()
 }