Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RT 7.4 & RT 7.11: Updated the script with deviation for ASpath set #3365

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestMain(m *testing.M) {
fptest.RunTests(m)
}

func configureImportBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4 string, ipv6 string, aspathMatch []string, communityMatch string, aspathSetName string, communitySetName string, commMatchSetOptions oc.E_BgpPolicy_MatchSetOptionsType, aspMatchSetOptions oc.E_RoutingPolicy_MatchSetOptionsType) {
func configureImportBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4, ipv6 string, aspathMatch []string, communityMatch, aspathSetName, communitySetName string, commMatchSetOptions, aspMatchSetOptions oc.E_RoutingPolicy_MatchSetOptionsType) {
root := &oc.Root{}
rp := root.GetOrCreateRoutingPolicy()
pdef1 := rp.GetOrCreatePolicyDefinition(ImpPolicy)
Expand All @@ -73,10 +73,13 @@ func configureImportBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4 string,
stmt1.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE)
}

aspathSet := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateAsPathSet(aspathSetName)
aspathSet.SetAsPathSetMember(aspathMatch)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetAsPathSet(aspathSetName)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetMatchSetOptions(aspMatchSetOptions)
if !deviations.BgpAspathsetUnsupported(dut) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to accept the BgpAspathsetUnsupported deviation, a workaround/alternative needs to be proposed to achieve similar functionality. This could be using CLI configuration applied via a gnmi.Set using union_replace.

For example, does juniper support implementing an OC call-policy which refers to a policy defined in native Juniper configuration that can match on as-paths?

aspathSet := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateAsPathSet(aspathSetName)
aspathSet.SetAsPathSetMember(aspathMatch)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetAsPathSet(aspathSetName)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetMatchSetOptions(aspMatchSetOptions)
}

pdAllow := rp.GetOrCreatePolicyDefinition(RPLPermitAll)
st, err := pdAllow.AppendNewStatement("id-1")
if err != nil {
Expand All @@ -100,7 +103,6 @@ func configureImportBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4 string,
cs = append(cs, oc.UnionString(communityMatch))
}
communitySet.SetCommunityMember(cs)
communitySet.SetMatchSetOptions(commMatchSetOptions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ok, I see it is moved into a separate, already accepted deviation in line 123.

}

var communitySetCLIConfig string
Expand All @@ -118,6 +120,7 @@ func configureImportBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4 string,
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(communitySetName)
} else {
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(communitySetName)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(commMatchSetOptions)
}

if deviations.CommunityMemberRegexUnsupported(dut) && communitySetName == "any_my_3_comms" {
Expand Down Expand Up @@ -273,6 +276,10 @@ func TestCommunitySet(t *testing.T) {
bs := cfgplugins.NewBGPSession(t, cfgplugins.PortCount2, nil)
bs.WithEBGP(t, []oc.E_BgpTypes_AFI_SAFI_TYPE{oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST, oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST}, []string{"port2"}, true, true)

if deviations.BgpAspathsetUnsupported(bs.DUT) {
testResults[4] = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above, we need some alternative configuration/policy to achieve the desired behavior of matching on as paths.

}

configureOTG(t, bs, prefixesV4, prefixesV6)
bs.PushAndStart(t)

Expand All @@ -287,7 +294,7 @@ func TestCommunitySet(t *testing.T) {
communityMatch := "(10[0-9]:1)"
communitySetName := "any_my_3_comms"
aspathSetName := "any_my_aspath"
commMatchSetOptions := oc.BgpPolicy_MatchSetOptionsType_ANY
commMatchSetOptions := oc.RoutingPolicy_MatchSetOptionsType_ANY
aspMatchSetOptions := oc.RoutingPolicy_MatchSetOptionsType_ANY
configureImportBGPPolicy(t, bs.DUT, ipv4, ipv6, aspathMatch, communityMatch, aspathSetName, communitySetName, commMatchSetOptions, aspMatchSetOptions)
sleepTime := time.Duration(totalPackets/trafficPps) + 5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,11 @@ platform_exceptions: {
}
}

platform_exceptions: {
platform: {
vendor: JUNIPER
}
deviations: {
bgp_aspathset_unsupported: true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ functions.
* conditions/bgp-conditions/match-community-set/config/
* community-set: "regex-community"
* match-set-options: "ANY"
* actions/config/policy-result = "NEXT_STATEMENT"
* actions/config/policy-result = "ACCEPT_ROUTE"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.


* Create policy-definitions/policy-definition/config/name = "multi_policy"
* statements/statement/config/name = "reject_route_community"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ const (
bgpActionMethod = oc.SetCommunity_Method_REFERENCE
bgpSetCommunityOptionType = oc.BgpPolicy_BgpSetCommunityOptionType_ADD
prefixSetNameSetOptions = oc.RoutingPolicy_MatchSetOptionsRestrictedType_ANY
matchAny = oc.BgpPolicy_MatchSetOptionsType_ANY
matchInvert = oc.BgpPolicy_MatchSetOptionsType_INVERT
matchAny = oc.RoutingPolicy_MatchSetOptionsType_ANY
matchInvert = oc.RoutingPolicy_MatchSetOptionsType_INVERT
rejectResult = oc.RoutingPolicy_PolicyResultType_REJECT_ROUTE
nextstatementResult = oc.RoutingPolicy_PolicyResultType_NEXT_STATEMENT
)
Expand Down Expand Up @@ -181,13 +181,29 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
addCommunitiesSetRefsAction := []string{"add-communities"}
setCommunitySetRefs := []string{"add_comm_60", "add_comm_70"}
myCommunitySets := []string{"50:1"}
addComm60Set := []string{"60:1"}
addComm70Set := []string{"70:1"}
if deviations.BgpCommunityMemberIsAString(dut) {
regexCommunities = []string{"(^|\\s)30:[0-9]+($|\\s)"}
}

root := &oc.Root{}
rp := root.GetOrCreateRoutingPolicy()

pdef := rp.GetOrCreatePolicyDefinition("PERMIT-ALL")
stmt, err := pdef.AppendNewStatement("20")
if err != nil {
t.Fatalf("AppendNewStatement(%s) failed: %v", "routePolicyStatement", err)
}
stmt.GetOrCreateActions().PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE

pdefrp := rp.GetOrCreatePolicyDefinition("routePolicy")
stmtrp, err := pdefrp.AppendNewStatement("routePolicyStatement")
if err != nil {
t.Fatalf("AppendNewStatement(%s) failed: %v", "routePolicyStatement", err)
}
stmtrp.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE)

// Configure the policy match_community_regex which will be called from multi_policy

pdef2 := rp.GetOrCreatePolicyDefinition(callPolicy)
Expand All @@ -210,7 +226,6 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
}
}
communitySetRegex.SetCommunityMember(pd2cs1)
communitySetRegex.SetMatchSetOptions(matchAny)
}

var communitySetCLIConfig string
Expand All @@ -228,14 +243,12 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
pd2stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(regexCommunitySet)
} else {
pd2stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(regexCommunitySet)
pd2stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(matchAny)
}

if !deviations.SkipSettingStatementForPolicy(dut) {
pd2stmt1.GetOrCreateActions().SetPolicyResult(nextstatementResult)
}
pd2stmt1.GetOrCreateActions().SetPolicyResult(oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE)

// Configure the parent policy multi_policy.

pdef1 := rp.GetOrCreatePolicyDefinition(parentPolicy)

// Configure multi_policy:STATEMENT1: reject_route_community
Expand All @@ -254,12 +267,12 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
}
}
communitySetReject.SetCommunityMember(cs1)
communitySetReject.SetMatchSetOptions(matchAny)

if deviations.BGPConditionsMatchCommunitySetUnsupported(dut) {
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(rejectCommunitySet)
} else {
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(rejectCommunitySet)
stmt1.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(matchAny)
}

stmt1.GetOrCreateActions().SetPolicyResult(rejectResult)
Expand All @@ -286,12 +299,12 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
}
}
communitySetNestedReject.SetCommunityMember(cs2)
communitySetNestedReject.SetMatchSetOptions(matchInvert)

if deviations.BGPConditionsMatchCommunitySetUnsupported(dut) {
stmt2.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(nestedRejectCommunitySet)
} else {
stmt2.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(nestedRejectCommunitySet)
stmt2.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(matchInvert)
}

stmt2.GetOrCreateActions().SetPolicyResult(rejectResult)
Expand All @@ -307,13 +320,12 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
communitySetRefsAddCommunities := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateCommunitySet(addCommunitiesSetRefs)

cs3 := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{}
for _, commMatch4 := range addCommunitiesRefs {
if commMatch4 != "" {
cs3 = append(cs3, oc.UnionString(commMatch4))
for _, commMatch3 := range addCommunitiesRefs {
if commMatch3 != "" {
cs3 = append(cs3, oc.UnionString(commMatch3))
}
}
communitySetRefsAddCommunities.SetCommunityMember(cs3)
communitySetRefsAddCommunities.SetMatchSetOptions(matchInvert)

if deviations.BGPConditionsMatchCommunitySetUnsupported(dut) {
stmt3.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(addCommunitiesSetRefs)
Expand All @@ -340,13 +352,33 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
communitySetMatchCommPrefixAddCommu := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateCommunitySet(myCommunitySet)

cs4 := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{}
for _, commMatch5 := range myCommunitySets {
if commMatch5 != "" {
cs4 = append(cs4, oc.UnionString(commMatch5))
for _, commMatch4 := range myCommunitySets {
if commMatch4 != "" {
cs4 = append(cs4, oc.UnionString(commMatch4))
}
}
communitySetMatchCommPrefixAddCommu.SetCommunityMember(cs4)
communitySetMatchCommPrefixAddCommu.SetMatchSetOptions(matchAny)
// communitySetMatchCommPrefixAddCommu.SetMatchSetOptions(oc.BgpPolicy_MatchSetOptionsType_ANY)

// Configure add_comm_60 [ "60:1"] to match_comm_and_prefix_add_2_community_sets statement
communitySetAdd60 := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateCommunitySet("add_comm_60")
cs5 := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{}
for _, commMatch5 := range addComm60Set {
if commMatch5 != "" {
cs5 = append(cs5, oc.UnionString(commMatch5))
}
}
communitySetAdd60.SetCommunityMember(cs5)

// Configure add_comm_70 [ "70:1"] to match_comm_and_prefix_add_2_community_sets statement
communitySetAdd70 := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateCommunitySet("add_comm_70")
cs6 := []oc.RoutingPolicy_DefinedSets_BgpDefinedSets_CommunitySet_CommunityMember_Union{}
for _, commMatch6 := range addComm70Set {
if commMatch6 != "" {
cs6 = append(cs6, oc.UnionString(commMatch6))
}
}
communitySetAdd70.SetCommunityMember(cs6)

// Configure multi_policy:STATEMENT4: match_comm_and_prefix_add_2_community_sets statement

Expand All @@ -364,7 +396,9 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
stmt6.GetOrCreateConditions().GetOrCreateBgpConditions().SetCommunitySet(myCommunitySet)
} else {
stmt4.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(myCommunitySet)
stmt4.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(matchAny)
stmt6.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetCommunitySet(myCommunitySet)
stmt6.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchCommunitySet().SetMatchSetOptions(matchAny)
}

// configure match-prefix-set: prefix-set-5 to match_comm_and_prefix_add_2_community_sets statement
Expand Down Expand Up @@ -415,7 +449,14 @@ func configureImportExportMultifacetMatchActionsBGPPolicy(t *testing.T, dut *ond
t.Fatalf("AppendNewStatement(%s) failed: %v", matchAspathSetMedStatement, err)
}

// TODO create as-path-set on the DUT, match-as-path-set not support.
// Configure my_aspath: [ "65512" ] to match_aspath_set_med statement
if !deviations.BgpAspathsetUnsupported(dut) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be some alternative to using aspath to achieve similar behavior. Without this, I can't accept this deviation.

myAspath := rp.GetOrCreateDefinedSets().GetOrCreateBgpDefinedSets().GetOrCreateAsPathSet(myAsPathName)
myAspath.SetAsPathSetMember([]string{strconv.Itoa(int(cfgplugins.AteAS2))})

stmt5.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetAsPathSet(myAsPathName)
stmt5.GetOrCreateConditions().GetOrCreateBgpConditions().GetOrCreateMatchAsPathSet().SetMatchSetOptions(matchAny)
}
// Configure set-med 100
stmt5.GetOrCreateActions().GetOrCreateBgpActions().SetMed = oc.UnionUint32(medValue)

Expand Down Expand Up @@ -603,14 +644,17 @@ func verifyTrafficV4AndV6(t *testing.T, bs *cfgplugins.BGPSession, testResults [
t.Errorf("FAIL- got %v%% packet loss for %s flow and prefixes: [%s, %s]; want < 0%% traffic loss", lossPct, "flow"+"ipv4"+strconv.Itoa(index), prefixPairV4[0], prefixPairV4[1])
} else if rxPackets != 0 && !testResults[index] {
t.Errorf("FAIL- got %v%% packet loss for %s flow and prefixes: [%s, %s]; want >100%% traffic loss", lossPct, "flow"+"ipv4"+strconv.Itoa(index), prefixPairV4[0], prefixPairV4[1])
} else if txPackets6 != rxPackets6 && testResults[index] {
} else {
t.Logf("Traffic validation successful for Prefixes: [%s, %s]. Result: [%t] PacketsTx: %d PacketsRx: %d", prefixPairV4[0], prefixPairV4[1], testResults[index], txPackets, rxPackets)
}

if txPackets6 != rxPackets6 && testResults[index] {
t.Errorf("FAIL- got %v%% packet loss for %s flow and prefixes: [%s, %s]; want < 0%% traffic loss", lossPct6, "flow"+"ipv6"+strconv.Itoa(index), prefixesV6[index][0], prefixesV6[index][1])
} else if rxPackets6 != 0 && !testResults[index] {
t.Errorf("FAIL- got %v%% packet loss for %s flow and prefixes: [%s, %s]; want >100%% traffic loss", lossPct6, "flow"+"ipv6"+strconv.Itoa(index), prefixesV6[index][0], prefixesV6[index][1])
} else {
t.Logf("Traffic validation successful for Prefixes: [%s, %s]. Result: [%t] PacketsTx: %d PacketsRx: %d", prefixesV6[index][0], prefixesV6[index][1], testResults[index], txPackets6, rxPackets6)
}

}
}

Expand Down Expand Up @@ -731,7 +775,9 @@ func validateOTGBgpPrefixV6AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
t.Logf("For Prefix %v, got AS Path %d want AS Path %d", bgpPrefix.GetAddress(), bgpPrefix.AsPath[0].GetAsNumbers(), metric)
}
case otglocalPref:
validateLocalPreferenceV6(t, dut, ipAddr, metric[0])
if !deviations.BGPRibOcPathUnsupported(dut) {
validateLocalPreferenceV6(t, dut, ipAddr, metric[0])
}
case otgCommunity:
t.Logf("For Prefix %v, Community received on OTG: %v", bgpPrefix.GetAddress(), bgpPrefix.Community)
for _, gotCommunity := range bgpPrefix.Community {
Expand Down Expand Up @@ -792,7 +838,9 @@ func validateOTGBgpPrefixV4AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
t.Logf("For Prefix %v, got AS Path %d want AS Path %d are equal", bgpPrefix.GetAddress(), bgpPrefix.AsPath[0].GetAsNumbers(), metric)
}
case otglocalPref:
validateLocalPreferenceV4(t, dut, ipAddr, metric[0])
if !deviations.BGPRibOcPathUnsupported(dut) {
validateLocalPreferenceV4(t, dut, ipAddr, metric[0])
}
case otgCommunity:
t.Logf("For Prefix %v, Community received on OTG: %v", bgpPrefix.GetAddress(), bgpPrefix.Community)
for _, gotCommunity := range bgpPrefix.Community {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,12 @@ platform_exceptions: {
skip_bgp_send_community_type: true
}
}
platform_exceptions: {
platform: {
vendor: JUNIPER
}
deviations: {
bgp_rib_oc_path_unsupported: true
bgp_aspathset_unsupported: true
}
}
5 changes: 5 additions & 0 deletions internal/deviations/deviations.go
Original file line number Diff line number Diff line change
Expand Up @@ -1135,3 +1135,8 @@ func DecapNHWithNextHopNIUnsupported(dut *ondatra.DUTDevice) bool {
func SflowSourceAddressUpdateUnsupported(dut *ondatra.DUTDevice) bool {
return lookupDUTDeviations(dut).GetSflowSourceAddressUpdateUnsupported()
}

// BgpAspathsetUnsupported returns true if as-path-set for bgp-defined-sets is unsupported
func BgpAspathsetUnsupported(dut *ondatra.DUTDevice) bool {
return lookupDUTDeviations(dut).GetBgpAspathsetUnsupported()
}
5 changes: 5 additions & 0 deletions proto/metadata.proto
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,11 @@ message Metadata {
// SFlow source address update is unsupported
// Arista: b/357914789
bool sflow_source_address_update_unsupported = 217;
// bgp_aspathset_unsupported is set to true for devices that do not support as-path-set for bgp-defined-sets.
// Juniper: b/330173167
bool bgp_aspathset_unsupported = 218;


// Reserved field numbers and identifiers.
reserved 84, 9, 28, 20, 90, 97, 55, 89, 19, 36, 35, 40, 173;
}
Expand Down
Loading
Loading