diff --git a/bgp/config_test.go b/bgp/config_test.go index 561d7498..a8b84d9b 100644 --- a/bgp/config_test.go +++ b/bgp/config_test.go @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/bgp/gobgp.go b/bgp/gobgp.go index ebc380a8..4aab85bb 100644 --- a/bgp/gobgp.go +++ b/bgp/gobgp.go @@ -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 diff --git a/bgp/ocgobgp.go b/bgp/ocgobgp.go index 84f2da64..515ed6c6 100644 --- a/bgp/ocgobgp.go +++ b/bgp/ocgobgp.go @@ -17,6 +17,7 @@ package bgp import ( "fmt" "strconv" + "strings" "golang.org/x/exp/maps" "golang.org/x/exp/slices" @@ -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), diff --git a/bgp/tests/local_tests/community_set_test.go b/bgp/tests/local_tests/community_set_test.go index 2f4150cd..71bf91c2 100644 --- a/bgp/tests/local_tests/community_set_test.go +++ b/bgp/tests/local_tests/community_set_test.go @@ -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() @@ -50,11 +57,22 @@ 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) @@ -62,9 +80,27 @@ func TestCommunitySet(t *testing.T) { 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}) @@ -72,12 +108,17 @@ func TestCommunitySet(t *testing.T) { } 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) { @@ -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) } @@ -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) {