packet/bgp: make serialization of ipv4/v6 nlri goroutine-safe

The serialization of ipv4/v6 nlri should be goroutine-safe but somehow not.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

3 files changed, 31 insertions, 24 deletions

diff --git a/packet/bgp/bgp.go b/packet/bgp/bgp.go
index 7c6fb582..514c4eb9 100644
--- a/packet/bgp/bgp.go
+++ b/packet/bgp/bgp.go
@@ -1077,6 +1077,14 @@ func (r *IPAddrPrefixDefault) decodePrefix(data []byte, bitlen uint8, addrlen ui
 	}
 	b := make([]byte, addrlen)
 	copy(b, data[:bytelen])
+	// clear trailing bits in the last byte. rfc doesn't require
+	// this but some bgp implementations need this...
+	rem := bitlen % 8
+	if rem != 0 {
+		mask := 0xff00 >> rem
+		lastByte := b[bytelen-1] & byte(mask)
+		b[bytelen-1] = lastByte
+	}
 	r.Prefix = b
 	return nil
 }
@@ -1085,16 +1093,6 @@ func (r *IPAddrPrefixDefault) serializePrefix(bitLen uint8) ([]byte, error) {
 	byteLen := (int(bitLen) + 7) / 8
 	buf := make([]byte, byteLen)
 	copy(buf, r.Prefix)
-	// clear trailing bits in the last byte. rfc doesn't require
-	// this though.
-	rem := bitLen % 8
-	if rem != 0 {
-		mask := 0xff00 >> rem
-		lastByte := buf[byteLen-1] & byte(mask)
-		buf[byteLen-1] = lastByte
-	}
-	r.Prefix = make([]byte, len(r.Prefix))
-	copy(r.Prefix, buf)
 	return buf, nil
 }
 
@@ -1173,13 +1171,14 @@ func (r *IPAddrPrefix) SAFI() uint8 {
 }
 
 func NewIPAddrPrefix(length uint8, prefix string) *IPAddrPrefix {
-	return &IPAddrPrefix{
+	p := &IPAddrPrefix{
 		IPAddrPrefixDefault{
 			Length: length,
-			Prefix: net.ParseIP(prefix).To4(),
 		},
 		4,
 	}
+	p.IPAddrPrefixDefault.decodePrefix(net.ParseIP(prefix).To4(), length, 4)
+	return p
 }
 
 func isIPv4MappedIPv6(ip net.IP) bool {
@@ -1203,15 +1202,16 @@ func (r *IPv6AddrPrefix) String() string {
 }
 
 func NewIPv6AddrPrefix(length uint8, prefix string) *IPv6AddrPrefix {
-	return &IPv6AddrPrefix{
+	p := &IPv6AddrPrefix{
 		IPAddrPrefix{
 			IPAddrPrefixDefault{
 				Length: length,
-				Prefix: net.ParseIP(prefix),
 			},
 			16,
 		},
 	}
+	p.IPAddrPrefixDefault.decodePrefix(net.ParseIP(prefix), length, 16)
+	return p
 }
 
 const (
diff --git a/packet/bgp/bgp_test.go b/packet/bgp/bgp_test.go
index 1b9df371..248fb346 100644
--- a/packet/bgp/bgp_test.go
+++ b/packet/bgp/bgp_test.go
@@ -63,10 +63,17 @@ func Test_Message(t *testing.T) {
 func Test_IPAddrPrefixString(t *testing.T) {
 	ipv4 := NewIPAddrPrefix(24, "129.6.10.0")
 	assert.Equal(t, "129.6.10.0/24", ipv4.String())
-	ipv6 := NewIPv6AddrPrefix(18, "3343:faba:3903::1")
-	assert.Equal(t, "3343:faba:3903::1/18", ipv6.String())
-	ipv6 = NewIPv6AddrPrefix(18, "3343:faba:3903::0")
-	assert.Equal(t, "3343:faba:3903::/18", ipv6.String())
+	ipv4 = NewIPAddrPrefix(24, "129.6.10.1")
+	assert.Equal(t, "129.6.10.0/24", ipv4.String())
+	ipv4 = NewIPAddrPrefix(22, "129.6.129.0")
+	assert.Equal(t, "129.6.128.0/22", ipv4.String())
+
+	ipv6 := NewIPv6AddrPrefix(64, "3343:faba:3903::0")
+	assert.Equal(t, "3343:faba:3903::/64", ipv6.String())
+	ipv6 = NewIPv6AddrPrefix(64, "3343:faba:3903::1")
+	assert.Equal(t, "3343:faba:3903::/64", ipv6.String())
+	ipv6 = NewIPv6AddrPrefix(63, "3343:faba:3903:129::0")
+	assert.Equal(t, "3343:faba:3903:128::/63", ipv6.String())
 }
 
 func Test_RouteTargetMembershipNLRIString(t *testing.T) {
@@ -787,7 +794,7 @@ func Test_CompareFlowSpecNLRI(t *testing.T) {
 
 func Test_MpReachNLRIWithIPv4MappedIPv6Prefix(t *testing.T) {
 	assert := assert.New(t)
-	n1 := NewIPv6AddrPrefix(120, "::ffff:10.0.0.1")
+	n1 := NewIPv6AddrPrefix(120, "::ffff:10.0.0.0")
 	buf1, err := n1.Serialize()
 	assert.Nil(err)
 	n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_IPv6_UC))
@@ -806,7 +813,7 @@ func Test_MpReachNLRIWithIPv4MappedIPv6Prefix(t *testing.T) {
 
 	label := NewMPLSLabelStack(2)
 
-	n3 := NewLabeledIPv6AddrPrefix(120, "::ffff:10.0.0.1", *label)
+	n3 := NewLabeledIPv6AddrPrefix(120, "::ffff:10.0.0.0", *label)
 	buf1, err = n3.Serialize()
 	assert.Nil(err)
 	n4, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_IPv6_MPLS))
diff --git a/packet/bgp/helper.go b/packet/bgp/helper.go
index 3b152628..ecae6bdd 100644
--- a/packet/bgp/helper.go
+++ b/packet/bgp/helper.go
@@ -74,16 +74,16 @@ func NewTestBGPUpdateMessage() *BGPMessage {
 	}
 
 	prefixes1 := []AddrPrefixInterface{
-		NewLabeledVPNIPAddrPrefix(20, "192.0.9.0", *NewMPLSLabelStack(1, 2, 3),
+		NewLabeledVPNIPAddrPrefix(24, "192.0.9.0", *NewMPLSLabelStack(1, 2, 3),
 			NewRouteDistinguisherTwoOctetAS(256, 10000)),
-		NewLabeledVPNIPAddrPrefix(26, "192.10.8.192", *NewMPLSLabelStack(5, 6, 7, 8),
+		NewLabeledVPNIPAddrPrefix(24, "192.10.8.0", *NewMPLSLabelStack(5, 6, 7, 8),
 			NewRouteDistinguisherIPAddressAS("10.0.1.1", 10001)),
 	}
 
-	prefixes2 := []AddrPrefixInterface{NewIPv6AddrPrefix(100,
+	prefixes2 := []AddrPrefixInterface{NewIPv6AddrPrefix(128,
 		"fe80:1234:1234:5667:8967:af12:8912:1023")}
 
-	prefixes3 := []AddrPrefixInterface{NewLabeledVPNIPv6AddrPrefix(100,
+	prefixes3 := []AddrPrefixInterface{NewLabeledVPNIPv6AddrPrefix(128,
 		"fe80:1234:1234:5667:8967:af12:1203:33a1", *NewMPLSLabelStack(5, 6),
 		NewRouteDistinguisherFourOctetAS(5, 6))}