packet: store path attr flag as type BGPAttrFlag

also add more meaningful debug message to ValidateFlags()

Signed-off-by: ISHIDA Wataru <ishida.wataru@lab.ntt.co.jp>

3 files changed, 37 insertions, 18 deletions

diff --git a/packet/bgp.go b/packet/bgp.go
index 79a03aac..0797465b 100644
--- a/packet/bgp.go
+++ b/packet/bgp.go
@@ -1715,13 +1715,32 @@ func routeFamilyPrefix(afi uint16, safi uint8) (prefix AddrPrefixInterface, err
 	return prefix, nil
 }
 
+type BGPAttrFlag uint8
+
 const (
-	BGP_ATTR_FLAG_EXTENDED_LENGTH = 1 << 4
-	BGP_ATTR_FLAG_PARTIAL         = 1 << 5
-	BGP_ATTR_FLAG_TRANSITIVE      = 1 << 6
-	BGP_ATTR_FLAG_OPTIONAL        = 1 << 7
+	BGP_ATTR_FLAG_EXTENDED_LENGTH BGPAttrFlag = 1 << 4
+	BGP_ATTR_FLAG_PARTIAL         BGPAttrFlag = 1 << 5
+	BGP_ATTR_FLAG_TRANSITIVE      BGPAttrFlag = 1 << 6
+	BGP_ATTR_FLAG_OPTIONAL        BGPAttrFlag = 1 << 7
 )
 
+func (f BGPAttrFlag) String() string {
+	var strs []string = make([]string, 0, 4)
+	if f&BGP_ATTR_FLAG_EXTENDED_LENGTH > 0 {
+		strs = append(strs, "EXTENDED_LENGTH")
+	}
+	if f&BGP_ATTR_FLAG_PARTIAL > 0 {
+		strs = append(strs, "PARTIAL")
+	}
+	if f&BGP_ATTR_FLAG_TRANSITIVE > 0 {
+		strs = append(strs, "TRANSITIVE")
+	}
+	if f&BGP_ATTR_FLAG_OPTIONAL > 0 {
+		strs = append(strs, "OPTIONAL")
+	}
+	return strings.Join(strs, "|")
+}
+
 type BGPAttrType uint8
 
 const (
@@ -1822,7 +1841,7 @@ const (
 	BGP_ERROR_SUB_OUT_OF_RESOURCES
 )
 
-var pathAttrFlags map[BGPAttrType]uint8 = map[BGPAttrType]uint8{
+var pathAttrFlags map[BGPAttrType]BGPAttrFlag = map[BGPAttrType]BGPAttrFlag{
 	BGP_ATTR_TYPE_ORIGIN:               BGP_ATTR_FLAG_TRANSITIVE,
 	BGP_ATTR_TYPE_AS_PATH:              BGP_ATTR_FLAG_TRANSITIVE,
 	BGP_ATTR_TYPE_NEXT_HOP:             BGP_ATTR_FLAG_TRANSITIVE,
@@ -1845,13 +1864,13 @@ type PathAttributeInterface interface {
 	DecodeFromBytes([]byte) error
 	Serialize() ([]byte, error)
 	Len() int
-	getFlags() uint8
+	getFlags() BGPAttrFlag
 	getType() BGPAttrType
 	ToApiStruct() *api.PathAttr
 }
 
 type PathAttribute struct {
-	Flags  uint8
+	Flags  BGPAttrFlag
 	Type   BGPAttrType
 	Length uint16
 	Value  []byte
@@ -1867,7 +1886,7 @@ func (p *PathAttribute) Len() int {
 	return int(l)
 }
 
-func (p *PathAttribute) getFlags() uint8 {
+func (p *PathAttribute) getFlags() BGPAttrFlag {
 	return p.Flags
 }
 
@@ -1882,7 +1901,7 @@ func (p *PathAttribute) DecodeFromBytes(data []byte) error {
 	if len(data) < 2 {
 		return NewMessageError(eCode, eSubCode, data, "attribute header length is short")
 	}
-	p.Flags = data[0]
+	p.Flags = BGPAttrFlag(data[0])
 	p.Type = BGPAttrType(data[1])
 
 	if p.Flags&BGP_ATTR_FLAG_EXTENDED_LENGTH != 0 {
@@ -1918,7 +1937,7 @@ func (p *PathAttribute) Serialize() ([]byte, error) {
 		p.Flags &^= BGP_ATTR_FLAG_EXTENDED_LENGTH
 	}
 	buf := make([]byte, p.Len())
-	buf[0] = p.Flags
+	buf[0] = uint8(p.Flags)
 	buf[1] = uint8(p.Type)
 	if p.Flags&BGP_ATTR_FLAG_EXTENDED_LENGTH != 0 {
 		binary.BigEndian.PutUint16(buf[2:4], p.Length)
diff --git a/packet/validate.go b/packet/validate.go
index b05574b2..b79012fc 100644
--- a/packet/validate.go
+++ b/packet/validate.go
@@ -132,7 +132,7 @@ func ValidateAttribute(a PathAttributeInterface, rfs map[RouteFamily]bool) (bool
 }
 
 // validator for PathAttribute
-func ValidateFlags(t BGPAttrType, flags uint8) (bool, string) {
+func ValidateFlags(t BGPAttrType, flags BGPAttrFlag) (bool, string) {
 
 	/*
 	 * RFC 4271 P.17 For well-known attributes, the Transitive bit MUST be set to 1.
@@ -156,8 +156,8 @@ func ValidateFlags(t BGPAttrType, flags uint8) (bool, string) {
 
 	// check flags are correct
 	if f, ok := pathAttrFlags[t]; ok {
-		if f != (flags & ^uint8(BGP_ATTR_FLAG_EXTENDED_LENGTH)) {
-			eMsg := "flags are invalid. attribtue type : " + strconv.Itoa(int(t))
+		if f != BGPAttrFlag(uint8(flags) & ^uint8(BGP_ATTR_FLAG_EXTENDED_LENGTH)) {
+			eMsg := fmt.Sprintf("flags are invalid. attribute type: %d, expect: %s, actual: %s", t, f, flags)
 			return false, eMsg
 		}
 	}
diff --git a/packet/validate_test.go b/packet/validate_test.go
index d7a22465..311ae031 100644
--- a/packet/validate_test.go
+++ b/packet/validate_test.go
@@ -146,7 +146,7 @@ func Test_Validate_duplicate_attribute(t *testing.T) {
 	assert := assert.New(t)
 	message := bgpupdate().Body.(*BGPUpdate)
 	// duplicate origin path attribute
-	originBytes := []byte{pathAttrFlags[BGP_ATTR_TYPE_ORIGIN], 1, 1, 1}
+	originBytes := []byte{byte(pathAttrFlags[BGP_ATTR_TYPE_ORIGIN]), 1, 1, 1}
 	origin := &PathAttributeOrigin{}
 	origin.DecodeFromBytes(originBytes)
 	message.PathAttributes = append(message.PathAttributes, origin)
@@ -189,7 +189,7 @@ func Test_Validate_invalid_origin(t *testing.T) {
 	assert := assert.New(t)
 	message := bgpupdate().Body.(*BGPUpdate)
 	// origin needs to be well-known
-	originBytes := []byte{pathAttrFlags[BGP_ATTR_TYPE_ORIGIN], 1, 1, 5}
+	originBytes := []byte{byte(pathAttrFlags[BGP_ATTR_TYPE_ORIGIN]), 1, 1, 5}
 	origin := &PathAttributeOrigin{}
 	origin.DecodeFromBytes(originBytes)
 	message.PathAttributes[0] = origin
@@ -209,7 +209,7 @@ func Test_Validate_invalid_nexthop_zero(t *testing.T) {
 
 	// invalid nexthop
 	addr := net.ParseIP("0.0.0.1").To4()
-	nexthopBytes := []byte{pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP], 3, 4}
+	nexthopBytes := []byte{byte(pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP]), 3, 4}
 	nexthopBytes = append(nexthopBytes, addr...)
 	nexthop := &PathAttributeNextHop{}
 	nexthop.DecodeFromBytes(nexthopBytes)
@@ -230,7 +230,7 @@ func Test_Validate_invalid_nexthop_lo(t *testing.T) {
 
 	// invalid nexthop
 	addr := net.ParseIP("127.0.0.1").To4()
-	nexthopBytes := []byte{pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP], 3, 4}
+	nexthopBytes := []byte{byte(pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP]), 3, 4}
 	nexthopBytes = append(nexthopBytes, addr...)
 	nexthop := &PathAttributeNextHop{}
 	nexthop.DecodeFromBytes(nexthopBytes)
@@ -251,7 +251,7 @@ func Test_Validate_invalid_nexthop_de(t *testing.T) {
 
 	// invalid nexthop
 	addr := net.ParseIP("224.0.0.1").To4()
-	nexthopBytes := []byte{pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP], 3, 4}
+	nexthopBytes := []byte{byte(pathAttrFlags[BGP_ATTR_TYPE_NEXT_HOP]), 3, 4}
 	nexthopBytes = append(nexthopBytes, addr...)
 	nexthop := &PathAttributeNextHop{}
 	nexthop.DecodeFromBytes(nexthopBytes)