bgp: fix bug of flowspec vpn parser/serializer

close #997

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

3 files changed, 126 insertions, 77 deletions

diff --git a/gobgp/cmd/global.go b/gobgp/cmd/global.go
index 9b9bd4af..1563c847 100644
--- a/gobgp/cmd/global.go
+++ b/gobgp/cmd/global.go
@@ -276,7 +276,7 @@ func ParseExtendedCommunities(input string) ([]bgp.ExtendedCommunityInterface, e
 	return exts, nil
 }
 
-func ParseFlowSpecArgs(rf bgp.RouteFamily, args []string) (bgp.AddrPrefixInterface, []string, error) {
+func ParseFlowSpecArgs(rf bgp.RouteFamily, args []string, rd bgp.RouteDistinguisherInterface) (bgp.AddrPrefixInterface, []string, error) {
 	thenPos := len(args)
 	for idx, v := range args {
 		if v == "then" {
@@ -302,7 +302,7 @@ func ParseFlowSpecArgs(rf bgp.RouteFamily, args []string) (bgp.AddrPrefixInterfa
 		nlri = bgp.NewFlowSpecIPv6Unicast(cmp)
 		fnlri = &nlri.(*bgp.FlowSpecIPv6Unicast).FlowSpecNLRI
 	case bgp.RF_FS_L2_VPN:
-		nlri = bgp.NewFlowSpecL2VPN(cmp)
+		nlri = bgp.NewFlowSpecL2VPN(rd, cmp)
 		fnlri = &nlri.(*bgp.FlowSpecL2VPN).FlowSpecNLRI
 	default:
 		return nil, nil, fmt.Errorf("invalid route family")
@@ -557,8 +557,25 @@ func extractAggregator(args []string) ([]string, bgp.PathAttributeInterface, err
 	return args, nil, nil
 }
 
+func extractRouteDistinguisher(args []string) ([]string, bgp.RouteDistinguisherInterface, error) {
+	for idx, arg := range args {
+		if arg == "rd" {
+			if len(args) < (idx + 1) {
+				return nil, nil, fmt.Errorf("invalid rd format")
+			}
+			rd, err := bgp.ParseRouteDistinguisher(args[idx+1])
+			if err != nil {
+				return nil, nil, err
+			}
+			return append(args[:idx], args[idx+2:]...), rd, nil
+		}
+	}
+	return args, nil, nil
+}
+
 func ParsePath(rf bgp.RouteFamily, args []string) (*api.Path, error) {
 	var nlri bgp.AddrPrefixInterface
+	var rd bgp.RouteDistinguisherInterface
 	var extcomms []string
 	var err error
 	attrs := table.PathAttrs(make([]bgp.PathAttributeInterface, 0, 1))
@@ -623,7 +640,7 @@ func ParsePath(rf bgp.RouteFamily, args []string) (*api.Path, error) {
 		ip, net, _ := net.ParseCIDR(args[0])
 		ones, _ := net.Mask.Size()
 
-		rd, err := bgp.ParseRouteDistinguisher(args[2])
+		rd, err = bgp.ParseRouteDistinguisher(args[2])
 		if err != nil {
 			return nil, err
 		}
@@ -671,8 +688,14 @@ func ParsePath(rf bgp.RouteFamily, args []string) (*api.Path, error) {
 		}
 	case bgp.RF_EVPN:
 		nlri, extcomms, err = ParseEvpnArgs(args)
-	case bgp.RF_FS_IPv4_UC, bgp.RF_FS_IPv6_UC, bgp.RF_FS_L2_VPN:
-		nlri, extcomms, err = ParseFlowSpecArgs(rf, args)
+	case bgp.RF_FS_L2_VPN:
+		args, rd, err = extractRouteDistinguisher(args)
+		if err != nil {
+			return nil, err
+		}
+		fallthrough
+	case bgp.RF_FS_IPv4_UC, bgp.RF_FS_IPv6_UC:
+		nlri, extcomms, err = ParseFlowSpecArgs(rf, args, rd)
 	case bgp.RF_OPAQUE:
 		m := extractReserved(args, []string{"key", "value"})
 		if len(m["key"]) != 1 || len(m["value"]) != 1 {
diff --git a/packet/bgp/bgp.go b/packet/bgp/bgp.go
index 9d6678d3..597307d8 100644
--- a/packet/bgp/bgp.go
+++ b/packet/bgp/bgp.go
@@ -2975,6 +2975,7 @@ func (p *FlowSpecUnknown) MarshalJSON() ([]byte, error) {
 type FlowSpecNLRI struct {
 	Value []FlowSpecComponentInterface
 	rf    RouteFamily
+	rd    RouteDistinguisherInterface
 }
 
 func (n *FlowSpecNLRI) AFI() uint16 {
@@ -2987,6 +2988,10 @@ func (n *FlowSpecNLRI) SAFI() uint8 {
 	return safi
 }
 
+func (n *FlowSpecNLRI) RD() RouteDistinguisherInterface {
+	return n.rd
+}
+
 func (n *FlowSpecNLRI) decodeFromBytes(rf RouteFamily, data []byte) error {
 	var length int
 	if (data[0]>>4) == 0xf && len(data) > 2 {
@@ -3001,6 +3006,15 @@ func (n *FlowSpecNLRI) decodeFromBytes(rf RouteFamily, data []byte) error {
 
 	n.rf = rf
 
+	if n.SAFI() == SAFI_FLOW_SPEC_VPN {
+		if length < 8 {
+			return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, "not all flowspec component bytes available")
+		}
+		n.rd = GetRouteDistinguisher(data[:8])
+		data = data[8:]
+		length -= 8
+	}
+
 	for l := length; l > 0; {
 		if len(data) == 0 {
 			return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, "not all flowspec component bytes available")
@@ -3009,28 +3023,20 @@ func (n *FlowSpecNLRI) decodeFromBytes(rf RouteFamily, data []byte) error {
 		var i FlowSpecComponentInterface
 		switch t {
 		case FLOW_SPEC_TYPE_DST_PREFIX:
-			switch rf {
-			case RF_FS_IPv4_UC:
+			switch {
+			case rf>>16 == AFI_IP:
 				i = NewFlowSpecDestinationPrefix(NewIPAddrPrefix(0, ""))
-			case RF_FS_IPv4_VPN:
-				i = NewFlowSpecDestinationPrefix(NewLabeledVPNIPAddrPrefix(0, "", *NewMPLSLabelStack(), nil))
-			case RF_FS_IPv6_UC:
+			case rf>>16 == AFI_IP6:
 				i = NewFlowSpecDestinationPrefix6(NewIPv6AddrPrefix(0, ""), 0)
-			case RF_FS_IPv6_VPN:
-				i = NewFlowSpecDestinationPrefix6(NewLabeledVPNIPv6AddrPrefix(0, "", *NewMPLSLabelStack(), nil), 0)
 			default:
 				return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Invalid address family: %v", rf))
 			}
 		case FLOW_SPEC_TYPE_SRC_PREFIX:
-			switch rf {
-			case RF_FS_IPv4_UC:
+			switch {
+			case rf>>16 == AFI_IP:
 				i = NewFlowSpecSourcePrefix(NewIPAddrPrefix(0, ""))
-			case RF_FS_IPv4_VPN:
-				i = NewFlowSpecSourcePrefix(NewLabeledVPNIPAddrPrefix(0, "", *NewMPLSLabelStack(), nil))
-			case RF_FS_IPv6_UC:
+			case rf>>16 == AFI_IP6:
 				i = NewFlowSpecSourcePrefix6(NewIPv6AddrPrefix(0, ""), 0)
-			case RF_FS_IPv6_VPN:
-				i = NewFlowSpecSourcePrefix6(NewLabeledVPNIPv6AddrPrefix(0, "", *NewMPLSLabelStack(), nil), 0)
 			default:
 				return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Invalid address family: %v", rf))
 			}
@@ -3072,6 +3078,16 @@ func (n *FlowSpecNLRI) decodeFromBytes(rf RouteFamily, data []byte) error {
 
 func (n *FlowSpecNLRI) Serialize() ([]byte, error) {
 	buf := make([]byte, 0, 32)
+	if n.SAFI() == SAFI_FLOW_SPEC_VPN {
+		if n.rd == nil {
+			return nil, fmt.Errorf("RD is nil")
+		}
+		b, err := n.rd.Serialize()
+		if err != nil {
+			return nil, err
+		}
+		buf = append(buf, b...)
+	}
 	for _, v := range n.Value {
 		b, err := v.Serialize()
 		if err != nil {
@@ -3097,6 +3113,9 @@ func (n *FlowSpecNLRI) Serialize() ([]byte, error) {
 
 func (n *FlowSpecNLRI) Len() int {
 	l := 0
+	if n.SAFI() == SAFI_FLOW_SPEC_VPN {
+		l += n.RD().Len()
+	}
 	for _, v := range n.Value {
 		l += v.Len()
 	}
@@ -3109,6 +3128,9 @@ func (n *FlowSpecNLRI) Len() int {
 
 func (n *FlowSpecNLRI) String() string {
 	buf := bytes.NewBuffer(make([]byte, 0, 32))
+	if n.SAFI() == SAFI_FLOW_SPEC_VPN {
+		buf.WriteString(fmt.Sprintf("[rd: %s]", n.rd))
+	}
 	for _, v := range n.Value {
 		buf.WriteString(v.String())
 	}
@@ -3116,11 +3138,21 @@ func (n *FlowSpecNLRI) String() string {
 }
 
 func (n *FlowSpecNLRI) MarshalJSON() ([]byte, error) {
+	if n.rd != nil {
+		return json.Marshal(struct {
+			RD    RouteDistinguisherInterface  `json:"rd"`
+			Value []FlowSpecComponentInterface `json:"value"`
+		}{
+			RD:    n.rd,
+			Value: n.Value,
+		})
+	}
 	return json.Marshal(struct {
 		Value []FlowSpecComponentInterface `json:"value"`
 	}{
 		Value: n.Value,
 	})
+
 }
 
 //
@@ -3137,6 +3169,13 @@ func CompareFlowSpecNLRI(n, m *FlowSpecNLRI) (int, error) {
 	longer := n.Value
 	shorter := m.Value
 	invert := 1
+	if n.SAFI() == SAFI_FLOW_SPEC_VPN {
+		k, _ := n.Serialize()
+		l, _ := m.Serialize()
+		if result := bytes.Compare(k, l); result != 0 {
+			return result, nil
+		}
+	}
 	if len(n.Value) < len(m.Value) {
 		longer = m.Value
 		shorter = n.Value
@@ -3160,33 +3199,13 @@ func CompareFlowSpecNLRI(n, m *FlowSpecNLRI) (int, error) {
 			// common prefix is equal, then the most specific prefix has precedence.
 			var p, q *IPAddrPrefixDefault
 			var pCommon, qCommon uint64
-			if family == RF_FS_IPv4_UC || family == RF_FS_IPv4_VPN {
-				if family == RF_FS_IPv4_VPN {
-					var s, t *LabeledVPNIPAddrPrefix
-					if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
-						s = v.(*FlowSpecDestinationPrefix).Prefix.(*LabeledVPNIPAddrPrefix)
-						t = w.(*FlowSpecDestinationPrefix).Prefix.(*LabeledVPNIPAddrPrefix)
-					} else {
-						s = v.(*FlowSpecSourcePrefix).Prefix.(*LabeledVPNIPAddrPrefix)
-						t = w.(*FlowSpecSourcePrefix).Prefix.(*LabeledVPNIPAddrPrefix)
-					}
-					k, _ := s.RD.Serialize()
-					l, _ := t.RD.Serialize()
-					if result := bytes.Compare(k, l); result < 0 {
-						return invert, nil
-					} else if result > 0 {
-						return invert * -1, nil
-					}
-					p = &s.IPAddrPrefixDefault
-					q = &t.IPAddrPrefixDefault
+			if n.AFI() == AFI_IP {
+				if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
+					p = &v.(*FlowSpecDestinationPrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
+					q = &w.(*FlowSpecDestinationPrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
 				} else {
-					if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
-						p = &v.(*FlowSpecDestinationPrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
-						q = &w.(*FlowSpecDestinationPrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
-					} else {
-						p = &v.(*FlowSpecSourcePrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
-						q = &w.(*FlowSpecSourcePrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
-					}
+					p = &v.(*FlowSpecSourcePrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
+					q = &w.(*FlowSpecSourcePrefix).Prefix.(*IPAddrPrefix).IPAddrPrefixDefault
 				}
 				min := p.Length
 				if q.Length < p.Length {
@@ -3194,33 +3213,13 @@ func CompareFlowSpecNLRI(n, m *FlowSpecNLRI) (int, error) {
 				}
 				pCommon = uint64(binary.BigEndian.Uint32([]byte(p.Prefix.To4())) >> (32 - min))
 				qCommon = uint64(binary.BigEndian.Uint32([]byte(q.Prefix.To4())) >> (32 - min))
-			} else if family == RF_FS_IPv6_UC || family == RF_FS_IPv6_VPN {
-				if family == RF_FS_IPv6_VPN {
-					var s, t *LabeledVPNIPv6AddrPrefix
-					if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
-						s = v.(*FlowSpecDestinationPrefix6).Prefix.(*LabeledVPNIPv6AddrPrefix)
-						t = w.(*FlowSpecDestinationPrefix6).Prefix.(*LabeledVPNIPv6AddrPrefix)
-					} else {
-						s = v.(*FlowSpecSourcePrefix6).Prefix.(*LabeledVPNIPv6AddrPrefix)
-						t = w.(*FlowSpecSourcePrefix6).Prefix.(*LabeledVPNIPv6AddrPrefix)
-					}
-					k, _ := s.RD.Serialize()
-					l, _ := t.RD.Serialize()
-					if result := bytes.Compare(k, l); result < 0 {
-						return invert, nil
-					} else if result > 0 {
-						return invert * -1, nil
-					}
-					p = &s.IPAddrPrefixDefault
-					q = &t.IPAddrPrefixDefault
+			} else if n.AFI() == AFI_IP6 {
+				if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
+					p = &v.(*FlowSpecDestinationPrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
+					q = &w.(*FlowSpecDestinationPrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
 				} else {
-					if v.Type() == FLOW_SPEC_TYPE_DST_PREFIX {
-						p = &v.(*FlowSpecDestinationPrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
-						q = &w.(*FlowSpecDestinationPrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
-					} else {
-						p = &v.(*FlowSpecSourcePrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
-						q = &w.(*FlowSpecSourcePrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
-					}
+					p = &v.(*FlowSpecSourcePrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
+					q = &w.(*FlowSpecSourcePrefix6).Prefix.(*IPv6AddrPrefix).IPAddrPrefixDefault
 				}
 				min := uint(p.Length)
 				if q.Length < p.Length {
@@ -3287,7 +3286,7 @@ func (n *FlowSpecIPv4Unicast) DecodeFromBytes(data []byte) error {
 }
 
 func NewFlowSpecIPv4Unicast(value []FlowSpecComponentInterface) *FlowSpecIPv4Unicast {
-	return &FlowSpecIPv4Unicast{FlowSpecNLRI{value, RF_FS_IPv4_UC}}
+	return &FlowSpecIPv4Unicast{FlowSpecNLRI{value, RF_FS_IPv4_UC, nil}}
 }
 
 type FlowSpecIPv4VPN struct {
@@ -3298,8 +3297,8 @@ func (n *FlowSpecIPv4VPN) DecodeFromBytes(data []byte) error {
 	return n.decodeFromBytes(AfiSafiToRouteFamily(n.AFI(), n.SAFI()), data)
 }
 
-func NewFlowSpecIPv4VPN(value []FlowSpecComponentInterface) *FlowSpecIPv4VPN {
-	return &FlowSpecIPv4VPN{FlowSpecNLRI{value, RF_FS_IPv4_VPN}}
+func NewFlowSpecIPv4VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecIPv4VPN {
+	return &FlowSpecIPv4VPN{FlowSpecNLRI{value, RF_FS_IPv4_VPN, rd}}
 }
 
 type FlowSpecIPv6Unicast struct {
@@ -3325,10 +3324,11 @@ func (n *FlowSpecIPv6VPN) DecodeFromBytes(data []byte) error {
 	return n.decodeFromBytes(AfiSafiToRouteFamily(n.AFI(), n.SAFI()), data)
 }
 
-func NewFlowSpecIPv6VPN(value []FlowSpecComponentInterface) *FlowSpecIPv6VPN {
+func NewFlowSpecIPv6VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecIPv6VPN {
 	return &FlowSpecIPv6VPN{FlowSpecNLRI{
 		Value: value,
 		rf:    RF_FS_IPv6_VPN,
+		rd:    rd,
 	}}
 }
 
@@ -3340,10 +3340,11 @@ func (n *FlowSpecL2VPN) DecodeFromBytes(data []byte) error {
 	return n.decodeFromBytes(AfiSafiToRouteFamily(n.AFI(), n.SAFI()), data)
 }
 
-func NewFlowSpecL2VPN(value []FlowSpecComponentInterface) *FlowSpecL2VPN {
+func NewFlowSpecL2VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecL2VPN {
 	return &FlowSpecL2VPN{FlowSpecNLRI{
 		Value: value,
 		rf:    RF_FS_L2_VPN,
+		rd:    rd,
 	}}
 }
 
diff --git a/packet/bgp/bgp_test.go b/packet/bgp/bgp_test.go
index 48366b61..80dcc5af 100644
--- a/packet/bgp/bgp_test.go
+++ b/packet/bgp/bgp_test.go
@@ -429,7 +429,8 @@ func Test_FlowSpecNlriL2(t *testing.T) {
 	eq := 0x1
 	item1 := NewFlowSpecComponentItem(eq, int(IPv4))
 	cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_ETHERNET_TYPE, []*FlowSpecComponentItem{item1}))
-	n1 := NewFlowSpecL2VPN(cmp)
+	rd, _ := ParseRouteDistinguisher("100:100")
+	n1 := NewFlowSpecL2VPN(rd, cmp)
 	buf1, err := n1.Serialize()
 	assert.Nil(err)
 	n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_FS_L2_VPN))
@@ -455,3 +456,27 @@ func Test_NotificationErrorCode(t *testing.T) {
 	NewNotificationErrorCode(0, BGP_ERROR_SUB_BAD_MESSAGE_TYPE).String()
 	NewNotificationErrorCode(BGP_ERROR_ROUTE_REFRESH_MESSAGE_ERROR+1, 0).String()
 }
+
+func Test_FlowSpecNlriVPN(t *testing.T) {
+	assert := assert.New(t)
+	cmp := make([]FlowSpecComponentInterface, 0)
+	cmp = append(cmp, NewFlowSpecDestinationPrefix(NewIPAddrPrefix(24, "10.0.0.0")))
+	cmp = append(cmp, NewFlowSpecSourcePrefix(NewIPAddrPrefix(24, "10.0.0.0")))
+	rd, _ := ParseRouteDistinguisher("100:100")
+	n1 := NewFlowSpecIPv4VPN(rd, cmp)
+	buf1, err := n1.Serialize()
+	assert.Nil(err)
+	n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_FS_IPv4_VPN))
+	assert.Nil(err)
+	err = n2.DecodeFromBytes(buf1)
+	assert.Nil(err)
+	buf2, _ := n2.Serialize()
+	if reflect.DeepEqual(n1, n2) == true {
+		t.Log("OK")
+	} else {
+		t.Error("Something wrong")
+		t.Error(len(buf1), n1, buf1)
+		t.Error(len(buf2), n2, buf2)
+		t.Log(bytes.Equal(buf1, buf2))
+	}
+}