From 5e3933b1fee6aa352c42ae475e434867f36361a6 Mon Sep 17 00:00:00 2001
From: Daniel Czerwonk <daniel@dan-nrw.de>
Date: Mon, 25 Jun 2018 23:44:18 +0200
Subject: [PATCH] all tests are passing again

---
 protocols/bgp/packet/large_community.go       |   2 +-
 protocols/bgp/packet/path_attributes.go       |  37 ----
 protocols/bgp/packet/path_attributes_test.go  | 203 ------------------
 protocols/bgp/server/fsm_established.go       |   2 +-
 protocols/bgp/server/update_helper.go         |   7 +-
 route/bgp_path.go                             |  35 ++-
 route/bgp_path_test.go                        |  66 ++++++
 route/bgp_test.go                             |  40 ++--
 .../actions/add_community_action_test.go      |  18 +-
 .../actions/add_large_community_action.go     |  12 +-
 .../add_large_community_action_test.go        |  40 ++--
 .../actions/as_path_prepend_action_test.go    |  20 +-
 routingtable/filter/large_community_filter.go |  14 +-
 routingtable/filter/term_condition_test.go    |  23 +-
 routingtable/update_helper_test.go            |  22 +-
 15 files changed, 230 insertions(+), 311 deletions(-)
 create mode 100644 route/bgp_path_test.go

diff --git a/protocols/bgp/packet/large_community.go b/protocols/bgp/packet/large_community.go
index 5b3f7788..4d93eb60 100644
--- a/protocols/bgp/packet/large_community.go
+++ b/protocols/bgp/packet/large_community.go
@@ -12,7 +12,7 @@ type LargeCommunity struct {
 	DataPart2           uint32
 }
 
-func (c LargeCommunity) String() string {
+func (c *LargeCommunity) String() string {
 	return fmt.Sprintf("(%d,%d,%d)", c.GlobalAdministrator, c.DataPart1, c.DataPart2)
 }
 
diff --git a/protocols/bgp/packet/path_attributes.go b/protocols/bgp/packet/path_attributes.go
index fa5e9f0e..8be48478 100644
--- a/protocols/bgp/packet/path_attributes.go
+++ b/protocols/bgp/packet/path_attributes.go
@@ -3,7 +3,6 @@ package packet
 import (
 	"bytes"
 	"fmt"
-	"strings"
 
 	"github.com/taktv6/tflow2/convert"
 )
@@ -343,24 +342,6 @@ func (pa *PathAttribute) setLength(buf *bytes.Buffer) (int, error) {
 	return bytesRead, nil
 }
 
-func (a *PathAttribute) CommunityString() string {
-	s := ""
-	for _, com := range a.Value.([]uint32) {
-		s += CommunityStringForUint32(com) + " "
-	}
-
-	return strings.TrimRight(s, " ")
-}
-
-func (a *PathAttribute) LargeCommunityString() string {
-	s := ""
-	for _, com := range a.Value.([]LargeCommunity) {
-		s += com.String() + " "
-	}
-
-	return strings.TrimRight(s, " ")
-}
-
 // dumpNBytes is used to dump n bytes of buf. This is useful in case an path attributes
 // length doesn't match a fixed length's attributes length (e.g. ORIGIN is always an octet)
 func dumpNBytes(buf *bytes.Buffer, n uint16) error {
@@ -553,24 +534,6 @@ func (pa *PathAttribute) serializeLargeCommunities(buf *bytes.Buffer) uint8 {
 	return length
 }
 
-func LargeCommunityAttributeForString(s string) (*PathAttribute, error) {
-	strs := strings.Split(s, " ")
-	coms := make([]LargeCommunity, len(strs))
-
-	var err error
-	for i, str := range strs {
-		coms[i], err = ParseLargeCommunityString(str)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return &PathAttribute{
-		TypeCode: LargeCommunitiesAttr,
-		Value:    coms,
-	}, nil
-}
-
 func fourBytesToUint32(address [4]byte) uint32 {
 	return uint32(address[0])<<24 + uint32(address[1])<<16 + uint32(address[2])<<8 + uint32(address[3])
 }
diff --git a/protocols/bgp/packet/path_attributes_test.go b/protocols/bgp/packet/path_attributes_test.go
index c3d5ecda..96200f4d 100644
--- a/protocols/bgp/packet/path_attributes_test.go
+++ b/protocols/bgp/packet/path_attributes_test.go
@@ -872,107 +872,6 @@ func TestDecodeUint32(t *testing.T) {
 	}
 }
 
-func TestASPathString(t *testing.T) {
-	tests := []struct {
-		name     string
-		pa       *PathAttribute
-		expected string
-	}{
-		{
-			name: "Test #1",
-			pa: &PathAttribute{
-				Value: ASPath{
-					{
-						Type: ASSequence,
-						ASNs: []uint32{10, 20, 30},
-					},
-				},
-			},
-			expected: "10 20 30",
-		},
-		{
-			name: "Test #2",
-			pa: &PathAttribute{
-				Value: ASPath{
-					{
-						Type: ASSequence,
-						ASNs: []uint32{10, 20, 30},
-					},
-					{
-						Type: ASSet,
-						ASNs: []uint32{200, 300},
-					},
-				},
-			},
-			expected: "10 20 30 (200 300)",
-		},
-	}
-
-	for _, test := range tests {
-		res := test.pa.ASPathString()
-		assert.Equal(t, test.expected, res)
-	}
-}
-
-func TestLargeCommunityString(t *testing.T) {
-	tests := []struct {
-		name     string
-		pa       *PathAttribute
-		expected string
-	}{
-		{
-			name: "two attributes",
-			pa: &PathAttribute{
-				Value: []LargeCommunity{
-					{
-						GlobalAdministrator: 1,
-						DataPart1:           2,
-						DataPart2:           3,
-					},
-					{
-						GlobalAdministrator: 4,
-						DataPart1:           5,
-						DataPart2:           6,
-					},
-				},
-			},
-			expected: "(1,2,3) (4,5,6)",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(te *testing.T) {
-			res := test.pa.LargeCommunityString()
-			assert.Equal(te, test.expected, res)
-		})
-	}
-}
-
-func TestCommunityString(t *testing.T) {
-	tests := []struct {
-		name     string
-		pa       *PathAttribute
-		expected string
-	}{
-		{
-			name: "two attributes",
-			pa: &PathAttribute{
-				Value: []uint32{
-					131080, 16778241,
-				},
-			},
-			expected: "(2,8) (256,1025)",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(te *testing.T) {
-			res := test.pa.CommunityString()
-			assert.Equal(te, test.expected, res)
-		})
-	}
-}
-
 func TestSetOptional(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -1852,108 +1751,6 @@ func TestSerializeAddPath(t *testing.T) {
 	}
 }
 
-func TestParseASPathStr(t *testing.T) {
-	tests := []struct {
-		name     string
-		input    string
-		wantFail bool
-		expected *PathAttribute
-	}{
-		{
-			name:     "Empty AS Path",
-			input:    "",
-			wantFail: false,
-			expected: &PathAttribute{
-				TypeCode: ASPathAttr,
-				Value:    ASPath{},
-			},
-		},
-		{
-			name:     "Simple AS_SEQUENCE",
-			input:    "3320 15169",
-			wantFail: false,
-			expected: &PathAttribute{
-				TypeCode: ASPathAttr,
-				Value: ASPath{
-					ASPathSegment{
-						Type: ASSequence,
-						ASNs: []uint32{3320, 15169},
-					},
-				},
-			},
-		},
-		{
-			name:     "AS_SEQUENCE with more than 255 elements",
-			input:    "123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123 123",
-			wantFail: false,
-			expected: &PathAttribute{
-				TypeCode: ASPathAttr,
-				Value: ASPath{
-					ASPathSegment{
-						Type: ASSequence,
-						ASNs: []uint32{123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123},
-					},
-					ASPathSegment{
-						Type: ASSequence,
-						ASNs: []uint32{123, 123, 123, 123, 123},
-					},
-				},
-			},
-		},
-		{
-			name:     "AS_SET only",
-			input:    "(3320 201701 15169)",
-			wantFail: false,
-			expected: &PathAttribute{
-				TypeCode: ASPathAttr,
-				Value: ASPath{
-					ASPathSegment{
-						Type: ASSet,
-						ASNs: []uint32{3320, 201701, 15169},
-					},
-				},
-			},
-		},
-		{
-			name:     "Mixed AS Path",
-			input:    "199714 51324 (3320 201701 15169)",
-			wantFail: false,
-			expected: &PathAttribute{
-				TypeCode: ASPathAttr,
-				Value: ASPath{
-					ASPathSegment{
-						Type: ASSequence,
-						ASNs: []uint32{199714, 51324},
-					},
-					ASPathSegment{
-						Type: ASSet,
-						ASNs: []uint32{3320, 201701, 15169},
-					},
-				},
-			},
-		},
-	}
-
-	for _, test := range tests {
-		res, err := ParseASPathStr(test.input)
-		if err != nil {
-			if test.wantFail {
-				continue
-			}
-
-			t.Errorf("Unexpected failure for test %q: %v", test.name, err)
-			continue
-		}
-
-		if test.wantFail {
-			t.Errorf("Unexpected success for test %q", test.name)
-			continue
-		}
-
-		assert.Equal(t, test.expected, res)
-	}
-}
-
 func TestFourBytesToUint32(t *testing.T) {
 	tests := []struct {
 		name     string
diff --git a/protocols/bgp/server/fsm_established.go b/protocols/bgp/server/fsm_established.go
index 9ce739ec..d4464a7b 100644
--- a/protocols/bgp/server/fsm_established.go
+++ b/protocols/bgp/server/fsm_established.go
@@ -235,7 +235,7 @@ func (s *establishedState) updates(u *packet.BGPUpdate) {
 			case packet.CommunitiesAttr:
 				path.BGPPath.Communities = pa.Value.([]uint32)
 			case packet.LargeCommunitiesAttr:
-				path.BGPPath.LargeCommunities = pa.LargeCommunityString()
+				path.BGPPath.LargeCommunities = pa.Value.([]packet.LargeCommunity)
 			}
 		}
 		s.fsm.adjRIBIn.AddPath(pfx, path)
diff --git a/protocols/bgp/server/update_helper.go b/protocols/bgp/server/update_helper.go
index 2f46e811..b472127c 100644
--- a/protocols/bgp/server/update_helper.go
+++ b/protocols/bgp/server/update_helper.go
@@ -57,11 +57,10 @@ func addOptionalPathAttribues(p *route.Path, parent *packet.PathAttribute) error
 	}
 
 	if len(p.BGPPath.LargeCommunities) > 0 {
-		largeCommunities, err := packet.LargeCommunityAttributeForString(p.BGPPath.LargeCommunities)
-		if err != nil {
-			return fmt.Errorf("Could not create large communities attribute: %v", err)
+		largeCommunities := &packet.PathAttribute{
+			TypeCode: packet.LargeCommunitiesAttr,
+			Value:    p.BGPPath.LargeCommunities,
 		}
-
 		current.Next = largeCommunities
 		current = largeCommunities
 	}
diff --git a/route/bgp_path.go b/route/bgp_path.go
index 1222487b..bd42ac3b 100644
--- a/route/bgp_path.go
+++ b/route/bgp_path.go
@@ -22,7 +22,7 @@ type BGPPath struct {
 	BGPIdentifier    uint32
 	Source           uint32
 	Communities      []uint32
-	LargeCommunities string
+	LargeCommunities []packet.LargeCommunity
 }
 
 // ECMP determines if routes b and c are euqal in terms of ECMP
@@ -155,14 +155,14 @@ func (b *BGPPath) Print() string {
 	}
 	ret := fmt.Sprintf("\t\tLocal Pref: %d\n", b.LocalPref)
 	ret += fmt.Sprintf("\t\tOrigin: %s\n", origin)
-	ret += fmt.Sprintf("\t\tAS Path: %s\n")
+	ret += fmt.Sprintf("\t\tAS Path: %v\n", b.ASPath)
 	nh := uint32To4Byte(b.NextHop)
 	ret += fmt.Sprintf("\t\tNEXT HOP: %d.%d.%d.%d\n", nh[0], nh[1], nh[2], nh[3])
 	ret += fmt.Sprintf("\t\tMED: %d\n", b.MED)
 	ret += fmt.Sprintf("\t\tPath ID: %d\n", b.PathIdentifier)
 	ret += fmt.Sprintf("\t\tSource: %d\n", b.Source)
-	ret += fmt.Sprintf("\t\tCommunities: %s\n", b.Communities)
-	ret += fmt.Sprintf("\t\tLargeCommunities: %s\n", b.LargeCommunities)
+	ret += fmt.Sprintf("\t\tCommunities: %v\n", b.Communities)
+	ret += fmt.Sprintf("\t\tLargeCommunities: %v\n", b.LargeCommunities)
 
 	return ret
 }
@@ -182,6 +182,11 @@ func (b *BGPPath) Prepend(asn uint32, times uint16) {
 			b.insertNewASSequence()
 		}
 
+		old := b.ASPath[0].ASNs
+		asns := make([]uint32, len(old)+1)
+		copy(asns[1:], old)
+		asns[0] = asn
+		b.ASPath[0].ASNs = asns
 	}
 
 	b.ASPathLen = b.ASPath.Length()
@@ -210,7 +215,7 @@ func (p *BGPPath) Copy() *BGPPath {
 
 // ComputeHash computes an hash over all attributes of the path
 func (b *BGPPath) ComputeHash() string {
-	s := fmt.Sprintf("%d\t%d\t%s\t%d\t%d\t%v\t%d\t%d\t%s\t%s\t%d",
+	s := fmt.Sprintf("%d\t%d\t%v\t%d\t%d\t%v\t%d\t%d\t%v\t%v\t%d",
 		b.NextHop,
 		b.LocalPref,
 		b.ASPath,
@@ -230,6 +235,26 @@ func (b *BGPPath) ComputeHash() string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
+// CommunitiesString returns the formated communities
+func (b *BGPPath) CommunitiesString() string {
+	str := ""
+	for _, com := range b.Communities {
+		str += packet.CommunityStringForUint32(com) + " "
+	}
+
+	return strings.TrimRight(str, " ")
+}
+
+// LargeCommunitiesString returns the formated communities
+func (b *BGPPath) LargeCommunitiesString() string {
+	str := ""
+	for _, com := range b.LargeCommunities {
+		str += com.String() + " "
+	}
+
+	return strings.TrimRight(str, " ")
+}
+
 func uint32To4Byte(addr uint32) [4]byte {
 	slice := convert.Uint32Byte(addr)
 	ret := [4]byte{
diff --git a/route/bgp_path_test.go b/route/bgp_path_test.go
new file mode 100644
index 00000000..d0f0a993
--- /dev/null
+++ b/route/bgp_path_test.go
@@ -0,0 +1,66 @@
+package route
+
+import (
+	"testing"
+
+	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCommunitiesString(t *testing.T) {
+	tests := []struct {
+		name     string
+		comms    []uint32
+		expected string
+	}{
+		{
+			name:     "two attributes",
+			comms:    []uint32{131080, 16778241},
+			expected: "(2,8) (256,1025)",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(te *testing.T) {
+			p := &BGPPath{
+				Communities: test.comms,
+			}
+
+			assert.Equal(te, test.expected, p.CommunitiesString())
+		})
+	}
+}
+
+func TestLargeCommunitiesString(t *testing.T) {
+	tests := []struct {
+		name     string
+		comms    []packet.LargeCommunity
+		expected string
+	}{
+		{
+			name: "two attributes",
+			comms: []packet.LargeCommunity{
+				{
+					GlobalAdministrator: 1,
+					DataPart1:           2,
+					DataPart2:           3,
+				},
+				{
+					GlobalAdministrator: 4,
+					DataPart1:           5,
+					DataPart2:           6,
+				},
+			},
+			expected: "(1,2,3) (4,5,6)",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(te *testing.T) {
+			p := &BGPPath{
+				LargeCommunities: test.comms,
+			}
+			assert.Equal(te, test.expected, p.LargeCommunitiesString())
+		})
+	}
+}
diff --git a/route/bgp_test.go b/route/bgp_test.go
index 263f3471..2eb017c2 100644
--- a/route/bgp_test.go
+++ b/route/bgp_test.go
@@ -3,26 +3,42 @@ package route
 import (
 	"testing"
 
+	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
+
 	"github.com/stretchr/testify/assert"
 )
 
 func TestComputeHash(t *testing.T) {
 	p := &BGPPath{
-		ASPath:           "123 456",
-		BGPIdentifier:    1,
-		Communities:      "(123, 456)",
-		EBGP:             false,
-		LargeCommunities: "(1, 2, 3)",
-		LocalPref:        100,
-		MED:              1,
-		NextHop:          100,
-		PathIdentifier:   5,
-		Source:           4,
+		ASPath: packet.ASPath{
+			packet.ASPathSegment{
+				ASNs:  []uint32{123, 456},
+				Count: 2,
+				Type:  packet.ASSequence,
+			},
+		},
+		BGPIdentifier: 1,
+		Communities: []uint32{
+			123, 456,
+		},
+		EBGP: false,
+		LargeCommunities: []packet.LargeCommunity{
+			packet.LargeCommunity{
+				DataPart1:           1,
+				DataPart2:           2,
+				GlobalAdministrator: 3,
+			},
+		},
+		LocalPref:      100,
+		MED:            1,
+		NextHop:        100,
+		PathIdentifier: 5,
+		Source:         4,
 	}
 
-	assert.Equal(t, "24d5b7681ab221b464a2c772e828628482cbfa4d5c6aac7a8285d33ef99b868a", p.ComputeHash())
+	assert.Equal(t, "45e238420552b88043edb8cb402034466b08d53b49f8e0fedc680747014ddeff", p.ComputeHash())
 
 	p.LocalPref = 150
 
-	assert.NotEqual(t, "24d5b7681ab221b464a2c772e828628482cbfa4d5c6aac7a8285d33ef99b868a", p.ComputeHash())
+	assert.NotEqual(t, "45e238420552b88043edb8cb402034466b08d53b49f8e0fedc680747014ddeff", p.ComputeHash())
 }
diff --git a/routingtable/filter/actions/add_community_action_test.go b/routingtable/filter/actions/add_community_action_test.go
index 0687e86c..9ec3d140 100644
--- a/routingtable/filter/actions/add_community_action_test.go
+++ b/routingtable/filter/actions/add_community_action_test.go
@@ -11,7 +11,7 @@ import (
 func TestAddingCommunities(t *testing.T) {
 	tests := []struct {
 		name        string
-		current     string
+		current     []uint32
 		communities []uint32
 		expected    string
 	}{
@@ -23,16 +23,20 @@ func TestAddingCommunities(t *testing.T) {
 			expected: "(1,2)",
 		},
 		{
-			name:    "add one to existing",
-			current: "(1,2)",
+			name: "add one to existing",
+			current: []uint32{
+				65538,
+			},
 			communities: []uint32{
 				196612,
 			},
 			expected: "(1,2) (3,4)",
 		},
 		{
-			name:    "add two to existing",
-			current: "(1,2)",
+			name: "add two to existing",
+			current: []uint32{
+				65538,
+			},
 			communities: []uint32{
 				196612, 327686,
 			},
@@ -41,7 +45,7 @@ func TestAddingCommunities(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Run(test.name, func(te *testing.T) {
+		t.Run(test.name, func(t *testing.T) {
 			p := &route.Path{
 				BGPPath: &route.BGPPath{
 					Communities: test.current,
@@ -51,7 +55,7 @@ func TestAddingCommunities(t *testing.T) {
 			a := NewAddCommunityAction(test.communities)
 			modPath, _ := a.Do(net.Prefix{}, p)
 
-			assert.Equal(te, test.expected, modPath.BGPPath.Communities)
+			assert.Equal(t, test.expected, modPath.BGPPath.CommunitiesString())
 		})
 	}
 }
diff --git a/routingtable/filter/actions/add_large_community_action.go b/routingtable/filter/actions/add_large_community_action.go
index 17169c31..534ad30b 100644
--- a/routingtable/filter/actions/add_large_community_action.go
+++ b/routingtable/filter/actions/add_large_community_action.go
@@ -1,18 +1,16 @@
 package actions
 
 import (
-	"strings"
-
 	"github.com/bio-routing/bio-rd/net"
 	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
 	"github.com/bio-routing/bio-rd/route"
 )
 
 type AddLargeCommunityAction struct {
-	communities []*packet.LargeCommunity
+	communities []packet.LargeCommunity
 }
 
-func NewAddLargeCommunityAction(coms []*packet.LargeCommunity) *AddLargeCommunityAction {
+func NewAddLargeCommunityAction(coms []packet.LargeCommunity) *AddLargeCommunityAction {
 	return &AddLargeCommunityAction{
 		communities: coms,
 	}
@@ -24,11 +22,7 @@ func (a *AddLargeCommunityAction) Do(p net.Prefix, pa *route.Path) (modPath *rou
 	}
 
 	modified := pa.Copy()
-
-	for _, com := range a.communities {
-		modified.BGPPath.LargeCommunities = modified.BGPPath.LargeCommunities + " " + com.String()
-	}
-	modified.BGPPath.LargeCommunities = strings.TrimLeft(modified.BGPPath.LargeCommunities, " ")
+	modified.BGPPath.LargeCommunities = append(modified.BGPPath.LargeCommunities, a.communities...)
 
 	return modified, false
 }
diff --git a/routingtable/filter/actions/add_large_community_action_test.go b/routingtable/filter/actions/add_large_community_action_test.go
index d2c597fd..f7af9139 100644
--- a/routingtable/filter/actions/add_large_community_action_test.go
+++ b/routingtable/filter/actions/add_large_community_action_test.go
@@ -12,14 +12,14 @@ import (
 func TestAddingLargeCommunities(t *testing.T) {
 	tests := []struct {
 		name        string
-		current     string
-		communities []*packet.LargeCommunity
+		current     []packet.LargeCommunity
+		communities []packet.LargeCommunity
 		expected    string
 	}{
 		{
 			name: "add one to empty",
-			communities: []*packet.LargeCommunity{
-				&packet.LargeCommunity{
+			communities: []packet.LargeCommunity{
+				packet.LargeCommunity{
 					GlobalAdministrator: 1,
 					DataPart1:           2,
 					DataPart2:           3,
@@ -28,10 +28,16 @@ func TestAddingLargeCommunities(t *testing.T) {
 			expected: "(1,2,3)",
 		},
 		{
-			name:    "add one to existing",
-			current: "(5,6,7)",
-			communities: []*packet.LargeCommunity{
-				&packet.LargeCommunity{
+			name: "add one to existing",
+			current: []packet.LargeCommunity{
+				packet.LargeCommunity{
+					GlobalAdministrator: 5,
+					DataPart1:           6,
+					DataPart2:           7,
+				},
+			},
+			communities: []packet.LargeCommunity{
+				packet.LargeCommunity{
 					GlobalAdministrator: 1,
 					DataPart1:           2,
 					DataPart2:           3,
@@ -40,15 +46,21 @@ func TestAddingLargeCommunities(t *testing.T) {
 			expected: "(5,6,7) (1,2,3)",
 		},
 		{
-			name:    "add two to existing",
-			current: "(5,6,7)",
-			communities: []*packet.LargeCommunity{
-				&packet.LargeCommunity{
+			name: "add two to existing",
+			current: []packet.LargeCommunity{
+				packet.LargeCommunity{
+					GlobalAdministrator: 5,
+					DataPart1:           6,
+					DataPart2:           7,
+				},
+			},
+			communities: []packet.LargeCommunity{
+				packet.LargeCommunity{
 					GlobalAdministrator: 1,
 					DataPart1:           2,
 					DataPart2:           3,
 				},
-				&packet.LargeCommunity{
+				packet.LargeCommunity{
 					GlobalAdministrator: 7,
 					DataPart1:           8,
 					DataPart2:           9,
@@ -69,7 +81,7 @@ func TestAddingLargeCommunities(t *testing.T) {
 			a := NewAddLargeCommunityAction(test.communities)
 			modPath, _ := a.Do(net.Prefix{}, p)
 
-			assert.Equal(te, test.expected, modPath.BGPPath.LargeCommunities)
+			assert.Equal(te, test.expected, modPath.BGPPath.LargeCommunitiesString())
 		})
 	}
 }
diff --git a/routingtable/filter/actions/as_path_prepend_action_test.go b/routingtable/filter/actions/as_path_prepend_action_test.go
index 348116cc..253c119f 100644
--- a/routingtable/filter/actions/as_path_prepend_action_test.go
+++ b/routingtable/filter/actions/as_path_prepend_action_test.go
@@ -3,6 +3,8 @@ package actions
 import (
 	"testing"
 
+	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
+
 	"github.com/bio-routing/bio-rd/net"
 	"github.com/bio-routing/bio-rd/route"
 	"github.com/stretchr/testify/assert"
@@ -23,7 +25,13 @@ func TestAppendPath(t *testing.T) {
 			name:  "append 0",
 			times: 0,
 			bgpPath: &route.BGPPath{
-				ASPath:    "12345 12345",
+				ASPath: packet.ASPath{
+					packet.ASPathSegment{
+						Count: 2,
+						Type:  packet.ASSequence,
+						ASNs:  []uint32{12345, 12345},
+					},
+				},
 				ASPathLen: 2,
 			},
 			expectedPath:   "12345 12345",
@@ -33,7 +41,13 @@ func TestAppendPath(t *testing.T) {
 			name:  "append 3",
 			times: 3,
 			bgpPath: &route.BGPPath{
-				ASPath:    "12345 15169",
+				ASPath: packet.ASPath{
+					packet.ASPathSegment{
+						Count: 2,
+						Type:  packet.ASSequence,
+						ASNs:  []uint32{12345, 15169},
+					},
+				},
 				ASPathLen: 2,
 			},
 			expectedPath:   "12345 12345 12345 12345 15169",
@@ -52,7 +66,7 @@ func TestAppendPath(t *testing.T) {
 				return
 			}
 
-			assert.Equal(te, test.expectedPath, p.BGPPath.ASPath, "ASPath")
+			assert.Equal(te, test.expectedPath, p.BGPPath.ASPath.String(), "ASPath")
 			assert.Equal(te, test.expectedLength, p.BGPPath.ASPathLen, "ASPathLen")
 		})
 	}
diff --git a/routingtable/filter/large_community_filter.go b/routingtable/filter/large_community_filter.go
index 0f9d474e..cb4722bd 100644
--- a/routingtable/filter/large_community_filter.go
+++ b/routingtable/filter/large_community_filter.go
@@ -1,15 +1,19 @@
 package filter
 
 import (
-	"strings"
-
 	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
 )
 
 type LargeCommunityFilter struct {
-	community *packet.LargeCommunity
+	community packet.LargeCommunity
 }
 
-func (f *LargeCommunityFilter) Matches(communityString string) bool {
-	return strings.Contains(communityString, f.community.String())
+func (f *LargeCommunityFilter) Matches(coms []packet.LargeCommunity) bool {
+	for _, com := range coms {
+		if com == f.community {
+			return true
+		}
+	}
+
+	return false
 }
diff --git a/routingtable/filter/term_condition_test.go b/routingtable/filter/term_condition_test.go
index cca90950..e1b3d4bd 100644
--- a/routingtable/filter/term_condition_test.go
+++ b/routingtable/filter/term_condition_test.go
@@ -107,7 +107,7 @@ func TestMatches(t *testing.T) {
 			name:   "community matches",
 			prefix: net.NewPfx(strAddr("10.0.0.0"), 24),
 			bgpPath: &route.BGPPath{
-				Communities: "(1,2) (3,4) (5,6)",
+				Communities: []uint32{65538, 196612, 327686}, // (1,2) (3,4) (5,6)
 			},
 			communityFilters: []*CommunityFilter{
 				&CommunityFilter{196612}, // (3,4)
@@ -118,7 +118,7 @@ func TestMatches(t *testing.T) {
 			name:   "community does not match",
 			prefix: net.NewPfx(strAddr("10.0.0.0"), 24),
 			bgpPath: &route.BGPPath{
-				Communities: "(1,2) (3,4) (5,6)",
+				Communities: []uint32{65538, 196612, 327686}, // (1,2) (3,4) (5,6)
 			},
 			communityFilters: []*CommunityFilter{
 				&CommunityFilter{196608}, // (3,0)
@@ -137,11 +137,22 @@ func TestMatches(t *testing.T) {
 			name:   "large community matches",
 			prefix: net.NewPfx(strAddr("10.0.0.0"), 24),
 			bgpPath: &route.BGPPath{
-				LargeCommunities: "(1,2,0) (1,2,3)",
+				LargeCommunities: []packet.LargeCommunity{
+					packet.LargeCommunity{
+						GlobalAdministrator: 1,
+						DataPart1:           2,
+						DataPart2:           3,
+					},
+					packet.LargeCommunity{
+						GlobalAdministrator: 1,
+						DataPart1:           2,
+						DataPart2:           0,
+					},
+				},
 			},
 			largeCommunityFilters: []*LargeCommunityFilter{
 				{
-					&packet.LargeCommunity{
+					packet.LargeCommunity{
 						GlobalAdministrator: 1,
 						DataPart1:           2,
 						DataPart2:           3,
@@ -156,7 +167,7 @@ func TestMatches(t *testing.T) {
 			bgpPath: &route.BGPPath{},
 			largeCommunityFilters: []*LargeCommunityFilter{
 				{
-					&packet.LargeCommunity{
+					packet.LargeCommunity{
 						GlobalAdministrator: 1,
 						DataPart1:           2,
 						DataPart2:           3,
@@ -170,7 +181,7 @@ func TestMatches(t *testing.T) {
 			prefix: net.NewPfx(strAddr("10.0.0.0"), 24),
 			largeCommunityFilters: []*LargeCommunityFilter{
 				{
-					&packet.LargeCommunity{
+					packet.LargeCommunity{
 						GlobalAdministrator: 1,
 						DataPart1:           2,
 						DataPart2:           3,
diff --git a/routingtable/update_helper_test.go b/routingtable/update_helper_test.go
index 29342f37..452a7e4a 100644
--- a/routingtable/update_helper_test.go
+++ b/routingtable/update_helper_test.go
@@ -2,9 +2,11 @@ package routingtable
 
 import (
 	"net"
+	"strings"
 	"testing"
 
 	bnet "github.com/bio-routing/bio-rd/net"
+	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
 	"github.com/bio-routing/bio-rd/route"
 	"github.com/stretchr/testify/assert"
 )
@@ -58,19 +60,31 @@ func TestShouldPropagateUpdate(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Run(test.name, func(te *testing.T) {
-			pfx := bnet.NewPfx(0, 32)
+		t.Run(test.name, func(t *testing.T) {
+			comms := make([]uint32, 0)
+			for _, s := range strings.Split(test.communities, " ") {
+				if s == "" {
+					continue
+				}
+
+				com, err := packet.ParseCommunityString(s)
+				if err != nil {
+					t.Fatalf("test failed: %s", err)
+				}
+				comms = append(comms, com)
+			}
 
+			pfx := bnet.NewPfx(0, 32)
 			pa := &route.Path{
 				Type: route.BGPPathType,
 				BGPPath: &route.BGPPath{
-					Communities: test.communities,
+					Communities: comms,
 					Source:      bnet.IPv4ToUint32(net.ParseIP("192.168.1.1")),
 				},
 			}
 
 			res := ShouldPropagateUpdate(pfx, pa, &test.neighbor)
-			assert.Equal(te, test.expected, res)
+			assert.Equal(t, test.expected, res)
 		})
 	}
 }
-- 
GitLab