policy: fix prefixset match about different families

currently wrongly try to match prefixset for v4 to v6 routes, and vice versa.

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

2 files changed, 96 insertions, 13 deletions

diff --git a/table/policy.go b/table/policy.go
index d1acac4c..fe1dcaf8 100644
--- a/table/policy.go
+++ b/table/policy.go
@@ -326,8 +326,9 @@ func NewPrefix(c config.Prefix) (*Prefix, error) {
 }
 
 type PrefixSet struct {
-	name string
-	tree *radix.Tree
+	name   string
+	tree   *radix.Tree
+	family bgp.RouteFamily
 }
 
 func (s *PrefixSet) Name() string {
@@ -343,6 +344,16 @@ func (lhs *PrefixSet) Append(arg DefinedSet) error {
 	if !ok {
 		return fmt.Errorf("type cast failed")
 	}
+	// if either is empty, family can be ignored.
+	if lhs.tree.Len() != 0 && rhs.tree.Len() != 0 {
+		_, w, _ := lhs.tree.Minimum()
+		l := w.([]*Prefix)
+		_, v, _ := rhs.tree.Minimum()
+		r := v.([]*Prefix)
+		if l[0].AddressFamily != r[0].AddressFamily {
+			return fmt.Errorf("can't append different family")
+		}
+	}
 	rhs.tree.Walk(func(key string, v interface{}) bool {
 		w, ok := lhs.tree.Get(key)
 		if ok {
@@ -354,6 +365,8 @@ func (lhs *PrefixSet) Append(arg DefinedSet) error {
 		}
 		return false
 	})
+	_, w, _ := lhs.tree.Minimum()
+	lhs.family = w.([]*Prefix)[0].AddressFamily
 	return nil
 }
 
@@ -398,6 +411,7 @@ func (lhs *PrefixSet) Replace(arg DefinedSet) error {
 		return fmt.Errorf("type cast failed")
 	}
 	lhs.tree = rhs.tree
+	lhs.family = rhs.family
 	return nil
 }
 
@@ -441,7 +455,13 @@ func NewPrefixSetFromApiStruct(name string, prefixes []*Prefix) (*PrefixSet, err
 		return nil, fmt.Errorf("empty prefix set name")
 	}
 	tree := radix.New()
-	for _, x := range prefixes {
+	var family bgp.RouteFamily
+	for i, x := range prefixes {
+		if i == 0 {
+			family = x.AddressFamily
+		} else if family != x.AddressFamily {
+			return nil, fmt.Errorf("multiple families")
+		}
 		key := CidrToRadixkey(x.Prefix.String())
 		d, ok := tree.Get(key)
 		if ok {
@@ -452,8 +472,9 @@ func NewPrefixSetFromApiStruct(name string, prefixes []*Prefix) (*PrefixSet, err
 		}
 	}
 	return &PrefixSet{
-		name: name,
-		tree: tree,
+		name:   name,
+		tree:   tree,
+		family: family,
 	}, nil
 }
 
@@ -466,11 +487,17 @@ func NewPrefixSet(c config.PrefixSet) (*PrefixSet, error) {
 		return nil, fmt.Errorf("empty prefix set name")
 	}
 	tree := radix.New()
-	for _, x := range c.PrefixList {
+	var family bgp.RouteFamily
+	for i, x := range c.PrefixList {
 		y, err := NewPrefix(x)
 		if err != nil {
 			return nil, err
 		}
+		if i == 0 {
+			family = y.AddressFamily
+		} else if family != y.AddressFamily {
+			return nil, fmt.Errorf("multiple families")
+		}
 		key := CidrToRadixkey(y.Prefix.String())
 		d, ok := tree.Get(key)
 		if ok {
@@ -481,8 +508,9 @@ func NewPrefixSet(c config.PrefixSet) (*PrefixSet, error) {
 		}
 	}
 	return &PrefixSet{
-		name: name,
-		tree: tree,
+		name:   name,
+		tree:   tree,
+		family: family,
 	}, nil
 }
 
@@ -1210,7 +1238,8 @@ func (c *PrefixCondition) Evaluate(path *Path, _ *PolicyOptions) bool {
 		}
 		return buffer.String()[:ones]
 	}
-	switch path.GetRouteFamily() {
+	family := path.GetRouteFamily()
+	switch family {
 	case bgp.RF_IPv4_UC:
 		masklen = path.GetNlri().(*bgp.IPAddrPrefix).Length
 		key = keyf(path.GetNlri().(*bgp.IPAddrPrefix).Prefix, int(masklen))
@@ -1220,6 +1249,9 @@ func (c *PrefixCondition) Evaluate(path *Path, _ *PolicyOptions) bool {
 	default:
 		return false
 	}
+	if family != c.set.family {
+		return false
+	}
 
 	result := false
 	_, ps, ok := c.set.tree.LongestPrefix(key)
diff --git a/table/policy_test.go b/table/policy_test.go
index 6897a227..a846dabe 100644
--- a/table/policy_test.go
+++ b/table/policy_test.go
@@ -17,16 +17,17 @@ package table
 
 import (
 	"fmt"
-	log "github.com/Sirupsen/logrus"
-	"github.com/osrg/gobgp/config"
-	"github.com/osrg/gobgp/packet/bgp"
-	"github.com/stretchr/testify/assert"
 	"math"
 	"net"
 	"strconv"
 	"strings"
 	"testing"
 	"time"
+
+	log "github.com/Sirupsen/logrus"
+	"github.com/osrg/gobgp/config"
+	"github.com/osrg/gobgp/packet/bgp"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestPrefixCalcurateNoRange(t *testing.T) {
@@ -2804,6 +2805,38 @@ func createAs4Value(s string) uint32 {
 	return uint32(upper)<<16 + uint32(lower)
 }
 
+func TestPrefixSetOperation(t *testing.T) {
+	// tryp to create prefixset with multiple families
+	p1 := config.Prefix{
+		IpPrefix:        "0.0.0.0/0",
+		MasklengthRange: "0..7",
+	}
+	p2 := config.Prefix{
+		IpPrefix:        "0::/25",
+		MasklengthRange: "25..128",
+	}
+	_, err := NewPrefixSet(config.PrefixSet{
+		PrefixSetName: "ps1",
+		PrefixList:    []config.Prefix{p1, p2},
+	})
+	assert.NotNil(t, err)
+	m1, _ := NewPrefixSet(config.PrefixSet{
+		PrefixSetName: "ps1",
+		PrefixList:    []config.Prefix{p1},
+	})
+	m2, err := NewPrefixSet(config.PrefixSet{PrefixSetName: "ps2"})
+	assert.Nil(t, err)
+	err = m1.Append(m2)
+	assert.Nil(t, err)
+	err = m2.Append(m1)
+	assert.Nil(t, err)
+	assert.Equal(t, bgp.RF_IPv4_UC, m2.family)
+	p3, _ := NewPrefix(config.Prefix{IpPrefix: "10.10.0.0/24", MasklengthRange: ""})
+	p4, _ := NewPrefix(config.Prefix{IpPrefix: "0::/25", MasklengthRange: ""})
+	_, err = NewPrefixSetFromApiStruct("ps3", []*Prefix{p3, p4})
+	assert.NotNil(t, err)
+}
+
 func TestPrefixSetMatch(t *testing.T) {
 	p1 := config.Prefix{
 		IpPrefix:        "0.0.0.0/0",
@@ -2861,6 +2894,24 @@ func TestPrefixSetMatch(t *testing.T) {
 	assert.False(t, m.Evaluate(path, nil))
 }
 
+func TestPrefixSetMatchV4withV6Prefix(t *testing.T) {
+	p1 := config.Prefix{
+		IpPrefix:        "c000::/3",
+		MasklengthRange: "3..128",
+	}
+	ps, err := NewPrefixSet(config.PrefixSet{
+		PrefixSetName: "ps1",
+		PrefixList:    []config.Prefix{p1},
+	})
+	assert.Nil(t, err)
+	m := &PrefixCondition{
+		set: ps,
+	}
+
+	path := NewPath(nil, bgp.NewIPAddrPrefix(6, "192.0.0.0"), false, []bgp.PathAttributeInterface{}, time.Now(), false)
+	assert.False(t, m.Evaluate(path, nil))
+}
+
 func TestLargeCommunityMatchAction(t *testing.T) {
 	coms := []*bgp.LargeCommunity{
 		&bgp.LargeCommunity{ASN: 100, LocalData1: 100, LocalData2: 100},