Merge pull request #266 from sebageek/fix-rfc7911-support-multiple-af…

…isafis-in-addpath-capability

Allow multiple AFISAFIs in AddPath capability

protocols/bgp/packet/decoder.go

@@ -10,6 +10,10 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	addPathTupleSize = 4
+)
+
 // Decode decodes a BGP message
 func Decode(buf *bytes.Buffer, opt *DecodeOptions) (*BGPMessage, error) {
 	hdr, err := decodeHeader(buf)
@@ -242,7 +246,7 @@ func decodeCapability(buf *bytes.Buffer) (Capability, error) {
 		}
 		cap.Value = mpCap
 	case AddPathCapabilityCode:
-		addPathCap, err := decodeAddPathCapability(buf)
+		addPathCap, err := decodeAddPathCapability(buf, cap.Length)
 		if err != nil {
 			return cap, errors.Wrap(err, "Unable to decode add path capability")
 		}
@@ -280,20 +284,29 @@ func decodeMultiProtocolCapability(buf *bytes.Buffer) (MultiProtocolCapability,
 	return mpCap, nil
 }
 
-func decodeAddPathCapability(buf *bytes.Buffer) (AddPathCapability, error) {
-	addPathCap := AddPathCapability{}
-	fields := []interface{}{
-		&addPathCap.AFI,
-		&addPathCap.SAFI,
-		&addPathCap.SendReceive,
+func decodeAddPathCapability(buf *bytes.Buffer, capLength uint8) (AddPathCapability, error) {
+	addPathCaps := make(AddPathCapability, 0)
+
+	if capLength%addPathTupleSize != 0 {
+		return nil, fmt.Errorf("Invalid caplength %d, must be multiple of %d", capLength, addPathTupleSize)
 	}
 
-	err := decode.Decode(buf, fields)
-	if err != nil {
-		return addPathCap, err
+	for ; capLength >= addPathTupleSize; capLength -= addPathTupleSize {
+		addPathCap := AddPathCapabilityTuple{}
+		fields := []interface{}{
+			&addPathCap.AFI,
+			&addPathCap.SAFI,
+			&addPathCap.SendReceive,
+		}
+		err := decode.Decode(buf, fields)
+		if err != nil {
+			return nil, err
+		}
+
+		addPathCaps = append(addPathCaps, addPathCap)
 	}
 
-	return addPathCap, nil
+	return addPathCaps, nil
 }
 
 func decodeASN4Capability(buf *bytes.Buffer) (ASN4Capability, error) {

protocols/bgp/packet/decoder_test.go

@@ -1522,15 +1522,73 @@ func TestDecodeOptParams(t *testing.T) {
 							Code:   69,
 							Length: 4,
 							Value: AddPathCapability{
-								AFI:         1,
-								SAFI:        1,
-								SendReceive: 3,
+								AddPathCapabilityTuple{
+									AFI:         1,
+									SAFI:        1,
+									SendReceive: 3,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Add path capability with multiple entries",
+			input: []byte{
+				2,    // Type
+				6,    // Length
+				69,   // Code
+				8,    // Length
+				0, 1, // AFI
+				1, // SAFI
+				3, // Send/Receive
+
+				0, 2, // AFI
+				1, // SAFI
+				3, // Send/Receive
+			},
+			wantFail: false,
+			expected: []OptParam{
+				{
+					Type:   2,
+					Length: 6,
+					Value: Capabilities{
+						{
+							Code:   69,
+							Length: 8,
+							Value: AddPathCapability{
+								AddPathCapabilityTuple{
+									AFI:         1,
+									SAFI:        1,
+									SendReceive: 3,
+								},
+								AddPathCapabilityTuple{
+									AFI:         2,
+									SAFI:        1,
+									SendReceive: 3,
+								},
 							},
 						},
 					},
 				},
 			},
 		},
+		{
+			name: "Add path capability with broken capability length",
+			input: []byte{
+				2,    // Type
+				6,    // Length
+				69,   // Code
+				5,    // Length
+				0, 1, // AFI
+				1, // SAFI
+				3, // Send/Receive
+				1, // broken value
+			},
+			wantFail: true,
+			expected: nil,
+		},
 	}
 
 	for _, test := range tests {
@@ -1568,9 +1626,11 @@ func TestDecodeCapability(t *testing.T) {
 				Code:   69,
 				Length: 4,
 				Value: AddPathCapability{
-					AFI:         1,
-					SAFI:        1,
-					SendReceive: 3,
+					AddPathCapabilityTuple{
+						AFI:         1,
+						SAFI:        1,
+						SendReceive: 3,
+					},
 				},
 			},
 			wantFail: false,
@@ -1627,9 +1687,11 @@ func TestDecodeAddPathCapability(t *testing.T) {
 			input:    []byte{0, 1, 1, 3},
 			wantFail: false,
 			expected: AddPathCapability{
-				AFI:         1,
-				SAFI:        1,
-				SendReceive: 3,
+				AddPathCapabilityTuple{
+					AFI:         1,
+					SAFI:        1,
+					SendReceive: 3,
+				},
 			},
 		},
 		{
@@ -1641,7 +1703,7 @@ func TestDecodeAddPathCapability(t *testing.T) {
 
 	for _, test := range tests {
 		buf := bytes.NewBuffer(test.input)
-		cap, err := decodeAddPathCapability(buf)
+		cap, err := decodeAddPathCapability(buf, uint8(len(test.input)))
 		if err != nil {
 			if test.wantFail {
 				continue

protocols/bgp/packet/encoder_test.go

@@ -116,9 +116,11 @@ func TestSerializeOptParams(t *testing.T) {
 							Code:   69,
 							Length: 4,
 							Value: AddPathCapability{
-								AFI:         1,
-								SAFI:        1,
-								SendReceive: 3,
+								AddPathCapabilityTuple{
+									AFI:         1,
+									SAFI:        1,
+									SendReceive: 3,
+								},
 							},
 						},
 					},

protocols/bgp/packet/parameters.go

@@ -43,18 +43,26 @@ func (c Capability) serialize(buf *bytes.Buffer) {
 	buf.Write(payload)
 }
 
-type AddPathCapability struct {
+type AddPathCapabilityTuple struct {
 	AFI         uint16
 	SAFI        uint8
 	SendReceive uint8
 }
 
-func (a AddPathCapability) serialize(buf *bytes.Buffer) {
+func (a AddPathCapabilityTuple) serialize(buf *bytes.Buffer) {
 	buf.Write(convert.Uint16Byte(a.AFI))
 	buf.WriteByte(a.SAFI)
 	buf.WriteByte(a.SendReceive)
 }
 
+type AddPathCapability []AddPathCapabilityTuple
+
+func (a AddPathCapability) serialize(buf *bytes.Buffer) {
+	for _, cap := range a {
+		cap.serialize(buf)
+	}
+}
+
 type ASN4Capability struct {
 	ASN4 uint32
 }

protocols/bgp/server/bmp_router.go

@@ -375,21 +375,23 @@ func (p *peer) configureBySentOpen(msg *packet.BGPOpen) {
 			switch cap.Code {
 			case packet.AddPathCapabilityCode:
 				addPathCap := cap.Value.(packet.AddPathCapability)
-				peerFamily := p.addressFamily(addPathCap.AFI, addPathCap.SAFI)
-				if peerFamily == nil {
-					continue
-				}
-				switch addPathCap.SendReceive {
-				case packet.AddPathSend:
-					peerFamily.addPathSend = routingtable.ClientOptions{
-						MaxPaths: 10,
+				for _, addPathCapTuple := range addPathCap {
+					peerFamily := p.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI)
+					if peerFamily == nil {
+						continue
 					}
-				case packet.AddPathReceive:
-					peerFamily.addPathReceive = true
-				case packet.AddPathSendReceive:
-					peerFamily.addPathReceive = true
-					peerFamily.addPathSend = routingtable.ClientOptions{
-						MaxPaths: 10,
+					switch addPathCapTuple.SendReceive {
+					case packet.AddPathSend:
+						peerFamily.addPathSend = routingtable.ClientOptions{
+							MaxPaths: 10,
+						}
+					case packet.AddPathReceive:
+						peerFamily.addPathReceive = true
+					case packet.AddPathSendReceive:
+						peerFamily.addPathReceive = true
+						peerFamily.addPathSend = routingtable.ClientOptions{
+							MaxPaths: 10,
+						}
 					}
 				}
 			}

protocols/bgp/server/fsm_open_sent.go

@@ -203,33 +203,35 @@ func (s *openSentState) processMultiProtocolCapability(cap packet.MultiProtocolC
 }
 
 func (s *openSentState) processAddPathCapability(addPathCap packet.AddPathCapability) {
-	if addPathCap.SAFI != packet.UnicastSAFI {
-		return
-	}
+	for _, addPathCapTuple := range addPathCap {
+		if addPathCapTuple.SAFI != packet.UnicastSAFI {
+			continue
+		}
 
-	f := s.fsm.addressFamily(addPathCap.AFI, addPathCap.SAFI)
-	if f == nil {
-		return
-	}
+		f := s.fsm.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI)
+		if f == nil {
+			continue
+		}
 
-	peerAddressFamily := s.fsm.peer.addressFamily(addPathCap.AFI, addPathCap.SAFI)
+		peerAddressFamily := s.fsm.peer.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI)
 
-	switch addPathCap.SendReceive {
-	case packet.AddPathReceive:
-		if !peerAddressFamily.addPathSend.BestOnly {
-			f.addPathTX = peerAddressFamily.addPathSend
-		}
-	case packet.AddPathSend:
-		if peerAddressFamily.addPathReceive {
-			f.addPathRX = true
-		}
-	case packet.AddPathSendReceive:
-		if !peerAddressFamily.addPathSend.BestOnly {
-			f.addPathTX = peerAddressFamily.addPathSend
-		}
+		switch addPathCapTuple.SendReceive {
+		case packet.AddPathReceive:
+			if !peerAddressFamily.addPathSend.BestOnly {
+				f.addPathTX = peerAddressFamily.addPathSend
+			}
+		case packet.AddPathSend:
+			if peerAddressFamily.addPathReceive {
+				f.addPathRX = true
+			}
+		case packet.AddPathSendReceive:
+			if !peerAddressFamily.addPathSend.BestOnly {
+				f.addPathTX = peerAddressFamily.addPathSend
+			}
 
-		if peerAddressFamily.addPathReceive {
-			f.addPathRX = true
+			if peerAddressFamily.addPathReceive {
+				f.addPathRX = true
+			}
 		}
 	}
 }

protocols/bgp/server/fsm_open_sent_test.go

@@ -229,9 +229,11 @@ func TestProcessAddPathCapabilityTX(t *testing.T) {
 			},
 			caps: []packet.AddPathCapability{
 				{
-					AFI:         packet.IPv4AFI,
-					SAFI:        packet.UnicastSAFI,
-					SendReceive: packet.AddPathReceive,
+					packet.AddPathCapabilityTuple{
+						AFI:         packet.IPv4AFI,
+						SAFI:        packet.UnicastSAFI,
+						SendReceive: packet.AddPathReceive,
+					},
 				},
 			},
 			expected: routingtable.ClientOptions{MaxPaths: 3},
@@ -255,9 +257,11 @@ func TestProcessAddPathCapabilityTX(t *testing.T) {
 			},
 			caps: []packet.AddPathCapability{
 				{
-					AFI:         packet.IPv4AFI,
-					SAFI:        packet.UnicastSAFI,
-					SendReceive: packet.AddPathReceive,
+					packet.AddPathCapabilityTuple{
+						AFI:         packet.IPv4AFI,
+						SAFI:        packet.UnicastSAFI,
+						SendReceive: packet.AddPathReceive,
+					},
 				},
 			},
 			expected: routingtable.ClientOptions{BestOnly: true},

protocols/bgp/server/peer.go

@@ -377,9 +377,11 @@ func addPathCapabilityForFamily(f *AddressFamilyConfig, afi uint16, safi uint8)
 	return true, packet.Capability{
 		Code: packet.AddPathCapabilityCode,
 		Value: packet.AddPathCapability{
-			AFI:         afi,
-			SAFI:        safi,
-			SendReceive: addPath,
+			packet.AddPathCapabilityTuple{
+				AFI:         afi,
+				SAFI:        safi,
+				SendReceive: addPath,
+			},
 		},
 	}
 }