packet/bgp: Sort FlowSpec rules when decoding/creating

Currently, we sort the FlowSpec rules when creating a new path
containing the FlowSpec NLRI and when parsing CLI arguments for
the FlowSpec rules.

This patch moves sorting the rules into the inside of the "packet"
module and sorts them when decoding binary and creating new NLRI.

Signed-off-by: IWASE Yusuke <iwase.yusuke0@gmail.com>

3 files changed, 55 insertions, 26 deletions

diff --git a/packet/bgp/bgp.go b/packet/bgp/bgp.go
index ef31817c..987bf328 100644
--- a/packet/bgp/bgp.go
+++ b/packet/bgp/bgp.go
@@ -4027,6 +4027,10 @@ func (n *FlowSpecNLRI) decodeFromBytes(rf RouteFamily, data []byte, options ...*
 		n.Value = append(n.Value, i)
 	}
 
+	// Sort Traffic Filtering Rules in types order to avoid the unordered rules
+	// are determined different.
+	sort.SliceStable(n.Value, func(i, j int) bool { return n.Value[i].Type() < n.Value[j].Type() })
+
 	return nil
 }
 
@@ -4246,7 +4250,13 @@ func (n *FlowSpecIPv4Unicast) DecodeFromBytes(data []byte, options ...*Marshalli
 }
 
 func NewFlowSpecIPv4Unicast(value []FlowSpecComponentInterface) *FlowSpecIPv4Unicast {
-	return &FlowSpecIPv4Unicast{FlowSpecNLRI{Value: value, rf: RF_FS_IPv4_UC}}
+	sort.SliceStable(value, func(i, j int) bool { return value[i].Type() < value[j].Type() })
+	return &FlowSpecIPv4Unicast{
+		FlowSpecNLRI: FlowSpecNLRI{
+			Value: value,
+			rf:    RF_FS_IPv4_UC,
+		},
+	}
 }
 
 type FlowSpecIPv4VPN struct {
@@ -4258,7 +4268,14 @@ func (n *FlowSpecIPv4VPN) DecodeFromBytes(data []byte, options ...*MarshallingOp
 }
 
 func NewFlowSpecIPv4VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecIPv4VPN {
-	return &FlowSpecIPv4VPN{FlowSpecNLRI{Value: value, rf: RF_FS_IPv4_VPN, rd: rd}}
+	sort.SliceStable(value, func(i, j int) bool { return value[i].Type() < value[j].Type() })
+	return &FlowSpecIPv4VPN{
+		FlowSpecNLRI: FlowSpecNLRI{
+			Value: value,
+			rf:    RF_FS_IPv4_VPN,
+			rd:    rd,
+		},
+	}
 }
 
 type FlowSpecIPv6Unicast struct {
@@ -4270,10 +4287,13 @@ func (n *FlowSpecIPv6Unicast) DecodeFromBytes(data []byte, options ...*Marshalli
 }
 
 func NewFlowSpecIPv6Unicast(value []FlowSpecComponentInterface) *FlowSpecIPv6Unicast {
-	return &FlowSpecIPv6Unicast{FlowSpecNLRI{
-		Value: value,
-		rf:    RF_FS_IPv6_UC,
-	}}
+	sort.SliceStable(value, func(i, j int) bool { return value[i].Type() < value[j].Type() })
+	return &FlowSpecIPv6Unicast{
+		FlowSpecNLRI: FlowSpecNLRI{
+			Value: value,
+			rf:    RF_FS_IPv6_UC,
+		},
+	}
 }
 
 type FlowSpecIPv6VPN struct {
@@ -4285,11 +4305,14 @@ func (n *FlowSpecIPv6VPN) DecodeFromBytes(data []byte, options ...*MarshallingOp
 }
 
 func NewFlowSpecIPv6VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecIPv6VPN {
-	return &FlowSpecIPv6VPN{FlowSpecNLRI{
-		Value: value,
-		rf:    RF_FS_IPv6_VPN,
-		rd:    rd,
-	}}
+	sort.SliceStable(value, func(i, j int) bool { return value[i].Type() < value[j].Type() })
+	return &FlowSpecIPv6VPN{
+		FlowSpecNLRI: FlowSpecNLRI{
+			Value: value,
+			rf:    RF_FS_IPv6_VPN,
+			rd:    rd,
+		},
+	}
 }
 
 type FlowSpecL2VPN struct {
@@ -4301,11 +4324,14 @@ func (n *FlowSpecL2VPN) DecodeFromBytes(data []byte, options ...*MarshallingOpti
 }
 
 func NewFlowSpecL2VPN(rd RouteDistinguisherInterface, value []FlowSpecComponentInterface) *FlowSpecL2VPN {
-	return &FlowSpecL2VPN{FlowSpecNLRI{
-		Value: value,
-		rf:    RF_FS_L2_VPN,
-		rd:    rd,
-	}}
+	sort.SliceStable(value, func(i, j int) bool { return value[i].Type() < value[j].Type() })
+	return &FlowSpecL2VPN{
+		FlowSpecNLRI: FlowSpecNLRI{
+			Value: value,
+			rf:    RF_FS_L2_VPN,
+			rd:    rd,
+		},
+	}
 }
 
 type OpaqueNLRI struct {
diff --git a/packet/bgp/bgp_test.go b/packet/bgp/bgp_test.go
index c3833fa8..d3e2719f 100644
--- a/packet/bgp/bgp_test.go
+++ b/packet/bgp/bgp_test.go
@@ -787,20 +787,21 @@ func Test_CompareFlowSpecNLRI(t *testing.T) {
 	assert := assert.New(t)
 	cmp, err := ParseFlowSpecComponents(RF_FS_IPv4_UC, "destination 10.0.0.2/32 source 10.0.0.1/32 destination-port ==3128 protocol tcp")
 	assert.Nil(err)
-	n1 := &FlowSpecNLRI{Value: cmp, rf: RF_FS_IPv4_UC}
+	// Note: Use NewFlowSpecIPv4Unicast() for the consistent ordered rules.
+	n1 := NewFlowSpecIPv4Unicast(cmp).FlowSpecNLRI
 	cmp, err = ParseFlowSpecComponents(RF_FS_IPv4_UC, "source 10.0.0.0/24 destination-port ==3128 protocol tcp")
 	assert.Nil(err)
-	n2 := &FlowSpecNLRI{Value: cmp, rf: RF_FS_IPv4_UC}
+	n2 := NewFlowSpecIPv4Unicast(cmp).FlowSpecNLRI
+	r, err := CompareFlowSpecNLRI(&n1, &n2)
+	assert.Nil(err)
+	assert.True(r > 0)
 	cmp, err = ParseFlowSpecComponents(RF_FS_IPv4_UC, "source 10.0.0.9/32 port ==80 ==8080 destination-port >8080&<8080 ==3128 source-port >1024 protocol ==udp ==tcp")
-	n3 := &FlowSpecNLRI{Value: cmp, rf: RF_FS_IPv4_UC}
+	n3 := NewFlowSpecIPv4Unicast(cmp).FlowSpecNLRI
 	assert.Nil(err)
 	cmp, err = ParseFlowSpecComponents(RF_FS_IPv4_UC, "destination 192.168.0.2/32")
-	n4 := &FlowSpecNLRI{Value: cmp, rf: RF_FS_IPv4_UC}
+	n4 := NewFlowSpecIPv4Unicast(cmp).FlowSpecNLRI
 	assert.Nil(err)
-	r, err := CompareFlowSpecNLRI(n1, n2)
-	assert.Nil(err)
-	assert.True(r > 0)
-	r, err = CompareFlowSpecNLRI(n3, n4)
+	r, err = CompareFlowSpecNLRI(&n3, &n4)
 	assert.Nil(err)
 	assert.True(r < 0)
 }
diff --git a/packet/bgp/validate_test.go b/packet/bgp/validate_test.go
index 3bb60639..12f81e03 100644
--- a/packet/bgp/validate_test.go
+++ b/packet/bgp/validate_test.go
@@ -388,8 +388,8 @@ func Test_Validate_flowspec(t *testing.T) {
 	cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_TCP_FLAG, []*FlowSpecComponentItem{item5, item6}))
 	cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_PKT_LEN, []*FlowSpecComponentItem{item2, item3, item4}))
 	cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_DSCP, []*FlowSpecComponentItem{item2, item3, item4}))
-	isFlagment := 0x02
-	item7 := NewFlowSpecComponentItem(isFlagment, 0)
+	isFragment := 0x02
+	item7 := NewFlowSpecComponentItem(isFragment, 0)
 	cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_FRAGMENT, []*FlowSpecComponentItem{item7}))
 	n1 := NewFlowSpecIPv4Unicast(cmp)
 	a := NewPathAttributeMpReachNLRI("", []AddrPrefixInterface{n1})
@@ -402,6 +402,8 @@ func Test_Validate_flowspec(t *testing.T) {
 	cmp = append(cmp, NewFlowSpecDestinationPrefix(NewIPAddrPrefix(24, "10.0.0.0")))
 	n1 = NewFlowSpecIPv4Unicast(cmp)
 	a = NewPathAttributeMpReachNLRI("", []AddrPrefixInterface{n1})
+	// Swaps components order to reproduce the rules order violation.
+	n1.Value[0], n1.Value[1] = n1.Value[1], n1.Value[0]
 	_, err = ValidateAttribute(a, m, false, false)
 	assert.NotNil(err)
 }