Skip to content

Commit

Permalink
Fix remove community set
Browse files Browse the repository at this point in the history
  • Loading branch information
@wenovus
wenovus committed Oct 4, 2023
1 parent b9d012c commit 9da19af
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 11 deletions.
8 changes: 4 additions & 4 deletions bgp/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func TestIntendedToGoBGPPolicies(t *testing.T) {
RouteDisposition: "none",
IgpActions: gobgp.IgpActions{SetTag: ""},
BgpActions: gobgp.BgpActions{SetAsPathPrepend: gobgp.SetAsPathPrepend{RepeatN: 0x0, As: "0"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string{"12345:54321"}, CommunitySetRef: ""}, Options: "ADD"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string{"12345:54321"}, CommunitySetRef: ""}, Options: "add"},
SetExtCommunity: gobgp.SetExtCommunity{SetExtCommunityMethod: gobgp.SetExtCommunityMethod{CommunitiesList: []string(nil), ExtCommunitySetRef: ""}, Options: ""},
SetRouteOrigin: "",
SetLocalPref: 0x0,
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestIntendedToGoBGPPolicies(t *testing.T) {
RouteDisposition: "accept-route",
IgpActions: gobgp.IgpActions{SetTag: ""},
BgpActions: gobgp.BgpActions{SetAsPathPrepend: gobgp.SetAsPathPrepend{RepeatN: 0x0, As: "0"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string(nil), CommunitySetRef: ""}, Options: "REPLACE"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string(nil), CommunitySetRef: ""}, Options: "replace"},
SetExtCommunity: gobgp.SetExtCommunity{SetExtCommunityMethod: gobgp.SetExtCommunityMethod{CommunitiesList: []string(nil), ExtCommunitySetRef: ""}, Options: ""},
SetRouteOrigin: "",
SetLocalPref: 0x0,
Expand Down Expand Up @@ -292,7 +292,7 @@ func TestIntendedToGoBGPPolicies(t *testing.T) {
RouteDisposition: "none",
IgpActions: gobgp.IgpActions{SetTag: ""},
BgpActions: gobgp.BgpActions{SetAsPathPrepend: gobgp.SetAsPathPrepend{RepeatN: 0x0, As: "0"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string{"12345:54321"}, CommunitySetRef: ""}, Options: "ADD"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string{"12345:54321"}, CommunitySetRef: ""}, Options: "add"},
SetExtCommunity: gobgp.SetExtCommunity{SetExtCommunityMethod: gobgp.SetExtCommunityMethod{CommunitiesList: []string(nil), ExtCommunitySetRef: ""}, Options: ""},
SetRouteOrigin: "",
SetLocalPref: 0x0,
Expand Down Expand Up @@ -338,7 +338,7 @@ func TestIntendedToGoBGPPolicies(t *testing.T) {
RouteDisposition: "accept-route",
IgpActions: gobgp.IgpActions{SetTag: ""},
BgpActions: gobgp.BgpActions{SetAsPathPrepend: gobgp.SetAsPathPrepend{RepeatN: 0x0, As: "0"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string(nil), CommunitySetRef: ""}, Options: "REPLACE"},
SetCommunity: gobgp.SetCommunity{SetCommunityMethod: gobgp.SetCommunityMethod{CommunitiesList: []string(nil), CommunitySetRef: ""}, Options: "replace"},
SetExtCommunity: gobgp.SetExtCommunity{SetExtCommunityMethod: gobgp.SetExtCommunityMethod{CommunitiesList: []string(nil), ExtCommunitySetRef: ""}, Options: ""},
SetRouteOrigin: "",
SetLocalPref: 0x0,
Expand Down
3 changes: 3 additions & 0 deletions bgp/gobgp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,12 @@ func (t *bgpTask) start(_ context.Context, yclient *ygnmi.Client) error {
BGPPath.NeighborAny().ApplyPolicy().ExportPolicy().Config().PathStruct(),
// BGP defined sets
// -- prefix sets
RoutingPolicyPath.DefinedSets().PrefixSetAny().Name().Config().PathStruct(),
RoutingPolicyPath.DefinedSets().PrefixSetAny().Mode().Config().PathStruct(),
RoutingPolicyPath.DefinedSets().PrefixSetAny().PrefixAny().IpPrefix().Config().PathStruct(),
RoutingPolicyPath.DefinedSets().PrefixSetAny().PrefixAny().MasklengthRange().Config().PathStruct(),
// -- community sets
ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySetAny().CommunitySetName().Config().PathStruct(),
ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySetAny().CommunityMember().Config().PathStruct(),
ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySetAny().MatchSetOptions().Config().PathStruct(),
// -- AS path sets
Expand Down
3 changes: 2 additions & 1 deletion bgp/ocgobgp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package bgp
import (
"fmt"
"strconv"
"strings"

"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -98,7 +99,7 @@ func convertPolicyDefinition(policy *oc.RoutingPolicy_PolicyDefinition, neighAdd
SetCommunityMethod: gobgp.SetCommunityMethod{
CommunitiesList: setCommunitiesList,
},
Options: statement.GetActions().GetBgpActions().GetSetCommunity().GetOptions().String(),
Options: strings.ToLower(statement.GetActions().GetBgpActions().GetSetCommunity().GetOptions().String()),
},
SetLocalPref: statement.GetActions().GetBgpActions().GetSetLocalPref(),
SetMed: gobgp.BgpSetMedType(setmed),
Expand Down
144 changes: 138 additions & 6 deletions bgp/tests/local_tests/community_set_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ func TestCommunitySet(t *testing.T) {
policyName := "community-sets"
policy := &oc.RoutingPolicy_PolicyDefinition_Statement_OrderedMap{}

// Create prefix set for ANY/ALL policy
prefixSetName := "tenRoutes"
tenRoutes := "10.0.0.0/8"
prefixSetPath := ocpath.Root().RoutingPolicy().DefinedSets().PrefixSet(prefixSetName)
Replace(t, dut2, prefixSetPath.Mode().Config(), oc.PrefixSet_Mode_IPV4)
Replace(t, dut2, prefixSetPath.Prefix(tenRoutes, "8..32").IpPrefix().Config(), tenRoutes)

// Create ANY community set
anyCommSetName := "ANY-community-set"
anyCommMemberPath := ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(anyCommSetName).CommunityMember()
Expand All @@ -50,34 +57,68 @@ func TestCommunitySet(t *testing.T) {
})
Replace(t, dut2, ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(allCommSetName).MatchSetOptions().Config(), oc.RoutingPolicy_MatchSetOptionsType_ALL)

// Create INVERT community set
invertCommSetName := "INVERT-community-set"
invertCommMemberPath := ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(invertCommSetName).CommunityMember()
Replace(t, dut2, invertCommMemberPath.Config(), []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{
oc.UnionString("11111:11111"),
oc.UnionString("22222:22222"),
})
Replace(t, dut2, ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(invertCommSetName).MatchSetOptions().Config(), oc.RoutingPolicy_MatchSetOptionsType_INVERT)

// Match on given list of community set members and reject them.
stmt, err := policy.AppendNew("reject-any-community-sets")
if err != nil {
t.Fatalf("Cannot append new BGP policy statement: %v", err)
}
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetPrefixSet(prefixSetName)
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetMatchSetOptions(oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY)
stmt.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(anyCommSetName)
stmt.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_REJECT_ROUTE)

stmt, err = policy.AppendNew("reject-all-community-sets")
if err != nil {
t.Fatalf("Cannot append new BGP policy statement: %v", err)
}
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetPrefixSet(prefixSetName)
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetMatchSetOptions(oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY)
stmt.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(allCommSetName)
stmt.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_REJECT_ROUTE)

// Create prefix set for INVERT policy
prefixSetName = "twentyRoutes"
twentyRoutes := "20.0.0.0/8"
prefixSetPath = ocpath.Root().RoutingPolicy().DefinedSets().PrefixSet(prefixSetName)
Replace(t, dut2, prefixSetPath.Mode().Config(), oc.PrefixSet_Mode_IPV4)
Replace(t, dut2, prefixSetPath.Prefix(twentyRoutes, "8..32").IpPrefix().Config(), twentyRoutes)

stmt, err = policy.AppendNew("reject-invert-community-sets")
if err != nil {
t.Fatalf("Cannot append new BGP policy statement: %v", err)
}
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetPrefixSet(prefixSetName)
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetMatchSetOptions(oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY)
stmt.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(invertCommSetName)
stmt.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_REJECT_ROUTE)

// Install policy
Replace(t, dut2, ocpath.Root().RoutingPolicy().PolicyDefinition(policyName).Config(), &oc.RoutingPolicy_PolicyDefinition{Statement: policy})
Replace(t, dut2, bgp.BGPPath.Neighbor(dut1.RouterID).ApplyPolicy().ImportPolicy().Config(), []string{policyName})
Await(t, dut2, bgp.BGPPath.Neighbor(dut1.RouterID).ApplyPolicy().ImportPolicy().State(), []string{policyName})
}

routeUnderTestList := []string{
"10.0.0.0/10",
"10.0.0.0/11",
"10.0.0.0/12",
"10.0.0.0/13",
"10.0.0.0/14",
"10.0.0.0/15",
"10.0.0.0/16",
"10.1.0.0/16",
"10.2.0.0/16",
"10.3.0.0/16",
"10.4.0.0/16",
"10.5.0.0/16",
"10.0.0.0/17",
"20.0.0.0/18",
"20.0.0.0/19",
"20.0.0.0/20",
}

installSetPolicy := func(t *testing.T, dut1, dut2, _, _, _ *Device, testRef bool) {
Expand Down Expand Up @@ -162,7 +203,68 @@ func TestCommunitySet(t *testing.T) {
},
)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetMethod(oc.SetCommunity_Method_INLINE)
// TODO(wenbli): Test REMOVE, it's possible GoBGP does not support it.
case 6:
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetOptions(oc.BgpPolicy_BgpSetCommunityOptionType_ADD)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().GetOrCreateInline().SetCommunities(
[]oc.RoutingPolicy_PolicyDefinition_Statement_Actions_BgpActions_SetCommunity_Inline_Communities_Union{
oc.UnionString("11111:11111"),
oc.UnionString("22222:22222"),
},
)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetMethod(oc.SetCommunity_Method_INLINE)

stmt, err = policy.AppendNew(fmt.Sprintf("stmt%d-2", i))
if err != nil {
t.Fatalf("Cannot append new BGP policy statement: %v", err)
}
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetPrefixSet(prefixSetName)
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetMatchSetOptions(oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY)

stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetOptions(oc.BgpPolicy_BgpSetCommunityOptionType_REMOVE)
commSetName := fmt.Sprintf("ref-set-%d", i)
commPath := ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(commSetName)
commUnions := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{
oc.UnionString("11111:11111"),
oc.UnionString("22222:22222"),
oc.UnionString("33333:33333"),
}
Replace(t, dut1, commPath.CommunitySetName().Config(), commSetName)
Replace(t, dut1, commPath.CommunityMember().Config(), commUnions)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().GetOrCreateReference().SetCommunitySetRef(commSetName)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetMethod(oc.SetCommunity_Method_REFERENCE)
case 7:
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetOptions(oc.BgpPolicy_BgpSetCommunityOptionType_ADD)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().GetOrCreateInline().SetCommunities(
[]oc.RoutingPolicy_PolicyDefinition_Statement_Actions_BgpActions_SetCommunity_Inline_Communities_Union{
oc.UnionString("11111:11111"),
oc.UnionString("22222:22222"),
},
)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetMethod(oc.SetCommunity_Method_INLINE)

stmt, err = policy.AppendNew(fmt.Sprintf("stmt%d-2", i))
if err != nil {
t.Fatalf("Cannot append new BGP policy statement: %v", err)
}
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetPrefixSet(prefixSetName)
stmt.GetOrCreateConditions().GetOrCreateMatchPrefixSet().SetMatchSetOptions(oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY)

stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetOptions(oc.BgpPolicy_BgpSetCommunityOptionType_REMOVE)
commSetName := fmt.Sprintf("ref-set-%d", i)
commPath := ocpath.Root().RoutingPolicy().DefinedSets().BgpDefinedSets().CommunitySet(commSetName)
commUnions := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{
oc.UnionString("[0-9]+:[0-9]+"),
}
Replace(t, dut1, commPath.CommunitySetName().Config(), commSetName)
Replace(t, dut1, commPath.CommunityMember().Config(), commUnions)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().GetOrCreateReference().SetCommunitySetRef(commSetName)
stmt.GetOrCreateActions().GetOrCreateBgpActions().GetOrCreateSetCommunity().SetMethod(oc.SetCommunity_Method_REFERENCE)
case 8:
installSetCommunityPolicy(oc.UnionString("10000:10000"))
case 9:
installSetCommunityPolicy(oc.UnionString("11111:11111"))
case 10:
installSetCommunityPolicy(oc.UnionString("22222:22222"))
}
stmt.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE)
}
Expand Down Expand Up @@ -217,6 +319,36 @@ func TestCommunitySet(t *testing.T) {
ReachPrefix: routeUnderTestList[5],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_DISCARD,
}, {
Description: "REMOVE",
Input: &valpb.TestRoute{
ReachPrefix: routeUnderTestList[6],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_ACCEPT,
}, {
Description: "REMOVE-ALL",
Input: &valpb.TestRoute{
ReachPrefix: routeUnderTestList[7],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_ACCEPT,
}, {
Description: "matches-invert",
Input: &valpb.TestRoute{
ReachPrefix: routeUnderTestList[8],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_DISCARD,
}, {
Description: "no-match-invert",
Input: &valpb.TestRoute{
ReachPrefix: routeUnderTestList[9],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_ACCEPT,
}, {
Description: "no-match-invert-2",
Input: &valpb.TestRoute{
ReachPrefix: routeUnderTestList[10],
},
ExpectedResult: valpb.RouteTestResult_ROUTE_TEST_RESULT_ACCEPT,
}},
},
installPolicies: func(t *testing.T, dut1, dut2, dut3, dut4, dut5 *Device) {
Expand Down

0 comments on commit 9da19af

Please sign in to comment.