Skip to content

Commit

Permalink
Merge branch 'main' into prefix_set_INVERT
Browse files Browse the repository at this point in the history
  • Loading branch information
dplore authored Dec 27, 2024
2 parents 73e7337 + d63d627 commit 5614acd
Show file tree
Hide file tree
Showing 20 changed files with 299 additions and 869 deletions.
30 changes: 15 additions & 15 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,27 @@
* @openconfig/featureprofiles-approvers

# /feature folders each have owners who are auto requested for review and may merge PR's
/feature/acl/ @alokmtri-g
/feature/aft/ @sudhinj @yunjie-lu
/feature/bgp/ @dplore
/feature/acl/ @openconfig/featureprofiles-owner-acl
/feature/aft/ @openconfig/featureprofiles-owner-aft
/feature/bgp/ @openconfig/featureprofiles-owner-bgp
/feature/dhcp/ @alokmtri-g
/feature/ethernet/ @ram-mac
/feature/gribi/ @nflath @nachikethas @xw-g
/feature/interface/ @ram-mac
/feature/isis/ @rohit-rp
/feature/lldp/ @alokmtri-g
/feature/mpls/ @swetha-haridasula
/feature/mtu/ @swetha-haridasula
/feature/networkinstance/ @swetha-haridasula
/feature/platform/ @amrindrr
/feature/platform/transceiver @jianchen-g @yiwenhu-g @ahsaanyousaf @ejbrever @rezachit
/feature/qos @sezhang2
/feature/interface/ @openconfig/featureprofiles-owner-interface
/feature/isis/ @openconfig/featureprofiles-owner-isis
/feature/lldp/ @openconfig/featureprofiles-owner-lldp
/feature/mpls/ @openconfig/featureprofiles-owner-mpls
/feature/mtu/ @openconfig/featureprofiles-owner-mtu
/feature/networkinstance/ @openconfig/featureprofiles-owner-networkinstance
/feature/platform/ @openconfig/featureprofiles-owner-platform
/feature/platform/transceiver @openconfig/featureprofiles-owner-platform-transceiver
/feature/qos @openconfig/featureprofiles-owner-qos
/feature/routing_policy/ @swetha-haridasula
/feature/sampling/ @sudhinj
/feature/security @mihirpitale-googler @morrowc
/feature/staticroute/ @swetha-haridasula
/feature/security @openconfig/featureprofiles-owner-security
/feature/staticroute/ @openconfig/featureprofiles-owner-staticroute
/feature/stp/ @alokmtri-g
/feature/system @swetha-haridasula
/feature/system @openconfig/featureprofiles-owner-system
/feature/vrrp @amrindrr

# Common OTG utilities
Expand Down
96 changes: 66 additions & 30 deletions feature/acl/otg_tests/acl_update_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@

## Summary

Configure an IP ACL, then test changing the ACL configuration to ensure a make-before-break behavior is performed. Make before break for ACL is defined as
Test configuration of an IP ACL.
Test changing the ACL configuration to ensure no packets are dropped due to
the configuration change, when the rule added or removed is not intended to
affect the traffic (make before break).


## Testbed type

* [`featureprofiles/topologies/atedut_2.testbed`](https://github.com/openconfig/featureprofiles/blob/main/topologies/atedut_2.testbed)

## ACL-1 Layer 3 terms

Expand Down Expand Up @@ -47,36 +55,64 @@ Configure an IP ACL, then test changing the ACL configuration to ensure a make-b

* Repeat the same test by moving ACLs to the DUT egress interface.

## Config Parameter coverage

```
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/destination-address
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/protocol
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/source-address
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/destination-address
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/protocol
acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/source-address
acl/interfaces/interface/ingress-acl-sets/ingress-acl-set
acl/interfaces/interface/ingress-acl-sets/ingress-acl-set/acl-entries
acl/interfaces/interface/ingress-acl-sets/ingress-acl-set/acl-entries/acl-entry
acl/interfaces/interface/egress-acl-sets/egress-acl-set
acl/interfaces/interface/egress-acl-sets/egress-acl-set/acl-entries
acl/interfaces/interface/egress-acl-sets/egress-acl-set/acl-entries/acl-entry
```

## Telemetry Parameter coverage

### Sub Test 4

* Repeat sub tests 1 through 4 using a port where [/interfaces/interface/state/management](https://github.com/openconfig/public/blob/daf73c37e9062b458bb9eab645840e5d3835c74d/release/models/interfaces/openconfig-interfaces.yang#L719-L727)
is true and in the case of a modular form factor device (MFF), provided by a `CONTROLLER_CARD` component.

## OpenConfig Path and RPC Coverage

```yaml
paths:
# base acl paths
/acl/acl-sets/acl-set/config/name:
/acl/acl-sets/acl-set/config/type:
/acl/acl-sets/acl-set/acl-entries/acl-entry/config/sequence-id:
/acl/acl-sets/acl-set/acl-entries/acl-entry/config/description:

# ipv4 address match
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/destination-address:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/destination-address-prefix-set:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/protocol:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/source-address:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/config/source-address-prefix-set:

# icmpv4 match
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/icmpv4/config/type:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv4/icmpv4/config/code:

# ipv6 address match
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/destination-address:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/destination-address-prefix-set:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/protocol:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/source-address:
/acl/acl-sets/acl-set/acl-entries/acl-entry/ipv6/config/source-address-prefix-set:

# paths for tcp/udp port and port-range
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/source-port:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/source-port-set:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/destination-port:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/destination-port-set:

# paths needed to match IP fragments
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/detail-mode:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/explicit-detail-match-mode:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/explicit-tcp-flags:
/acl/acl-sets/acl-set/acl-entries/acl-entry/transport/config/builtin-detail:

# state paths for management port and ACL counters
/interfaces/interface/state/management:
/acl/interfaces/interface/ingress-acl-sets/ingress-acl-set/acl-entries/acl-entry/state/matched-packets:
/acl/interfaces/interface/egress-acl-sets/egress-acl-set/acl-entries/acl-entry/state/matched-packets:

rpcs:
gnmi:
gNMI.Set:
union_replace: true
replace: true
gNMI.Subscribe:
on_change: true
```
acl/interfaces/interface/ingress-acl-sets/ingress-acl-set/acl-entries/acl-entry/state/matched-packets
acl/interfaces/interface/egress-acl-sets/egress-acl-set/acl-entries/acl-entry/state/matched-packets
```

## Protocol/RPC Parameter coverage

None
## Minimum DUT platform requirement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package actions_med_localpref_prepend_flow_control_test

import (
"fmt"
"strconv"
"strings"
"testing"
Expand All @@ -25,6 +26,7 @@ import (
"github.com/openconfig/featureprofiles/internal/attrs"
"github.com/openconfig/featureprofiles/internal/deviations"
"github.com/openconfig/featureprofiles/internal/fptest"
"github.com/openconfig/featureprofiles/internal/helpers"
"github.com/openconfig/ondatra"
"github.com/openconfig/ondatra/gnmi"
"github.com/openconfig/ondatra/gnmi/oc"
Expand All @@ -40,12 +42,12 @@ func TestMain(m *testing.M) {
}

const (
advertisedRoutesv4Net1 = "192.168.10.0"
advertisedRoutesv6Net1 = "2024:db8:128:128::"
advertisedRoutesv4Net2 = "192.168.20.0"
advertisedRoutesv6Net2 = "2024:db8:64:64::"
advertisedRoutesv4PrefixLen = 24
advertisedRoutesv6PrefixLen = 64
advertisedRoutesv4Net1 = "192.168.10.1"
advertisedRoutesv6Net1 = "2024:db8:128:128::1"
advertisedRoutesv4Net2 = "192.168.20.1"
advertisedRoutesv6Net2 = "2024:db8:64:64::1"
advertisedRoutesv4PrefixLen = 32
advertisedRoutesv6PrefixLen = 128
dutAS = 64500
ateAS = 64501
plenIPv4 = 30
Expand Down Expand Up @@ -209,6 +211,24 @@ func VerifyBgpState(t *testing.T, dut *ondatra.DUTDevice) {
t.Log("BGP sessions Established")
}

// juniperBgpPolicyMEDAdd is used to configure set metric add via native cli as an alternative for below xpath.
// routing-policy/policy-definitions/policy-definition/statements/statement/actions/bgp-actions/config/set-med
func juniperBgpPolicyMEDAdd(polName string, metric int) string {
return fmt.Sprintf(`
policy-options {
policy-statement %s {
term 1 {
then {
metric add %d;
}
}
term 2 {
then accept;
}
}
}`, polName, metric)
}

// configureASLocalPrefMEDPolicy configures MED, Local Pref, AS prepend etc
func configureASLocalPrefMEDPolicy(t *testing.T, dut *ondatra.DUTDevice, policyType, policyValue, statement string, ASN uint32) {
t.Helper()
Expand All @@ -228,12 +248,25 @@ func configureASLocalPrefMEDPolicy(t *testing.T, dut *ondatra.DUTDevice, policyT
actions.PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE
case setMEDPolicy:
if strings.Contains(policyValue, "+") {
actions.GetOrCreateBgpActions().SetMed = oc.UnionString(policyValue)
if deviations.BgpSetMedV7Unsupported(dut) {
t.Logf("Push the CLI config:%s", dut.Vendor())
metric, _ := strconv.Atoi(policyValue)
switch dut.Vendor() {
case ondatra.JUNIPER:
config := juniperBgpPolicyMEDAdd(setMEDPolicy, metric)
helpers.GnmiCLIConfig(t, dut, config)
default:
t.Fatalf("BgpSetMedV7Unsupported deviation needs cli configuration for vendor %s which is not defined", dut.Vendor())
}
} else {
actions.GetOrCreateBgpActions().SetMed = oc.UnionString(policyValue)
actions.PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE
}
} else {
metric, _ := strconv.Atoi(policyValue)
actions.GetOrCreateBgpActions().SetMed = oc.UnionUint32(uint32(metric))
actions.PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE
}
actions.PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE
case setPrependPolicy:
metric, _ := strconv.Atoi(policyValue)
asPrepend := actions.GetOrCreateBgpActions().GetOrCreateSetAsPathPrepend()
Expand Down Expand Up @@ -298,15 +331,13 @@ func deleteBGPImportExportPolicy(t *testing.T, dut *ondatra.DUTDevice, ipv4, ipv
batchConfig := &gnmi.SetBatch{}
nbrPolPathv4 := bgpPath.Neighbor(ipv4).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).ApplyPolicy()
nbrPolPathv6 := bgpPath.Neighbor(ipv6).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).ApplyPolicy()
if deviations.DefaultRoutePolicyUnsupported(dut) {
// deleteBGPImportExportPolicy on port2 needed when default policy is not supported
nbrPolPathv4_2 := bgpPath.Neighbor(ipv4_2).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).ApplyPolicy()
nbrPolPathv6_2 := bgpPath.Neighbor(ipv6_2).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).ApplyPolicy()
gnmi.BatchDelete(batchConfig, nbrPolPathv4_2.ImportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv4_2.ExportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv6_2.ImportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv6_2.ExportPolicy().Config())
}
nbrPolPathv4_2 := bgpPath.Neighbor(ipv4_2).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).ApplyPolicy()
nbrPolPathv6_2 := bgpPath.Neighbor(ipv6_2).AfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).ApplyPolicy()
gnmi.BatchDelete(batchConfig, nbrPolPathv4_2.ImportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv4_2.ExportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv6_2.ImportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv6_2.ExportPolicy().Config())

gnmi.BatchDelete(batchConfig, nbrPolPathv4.ImportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv4.ExportPolicy().Config())
gnmi.BatchDelete(batchConfig, nbrPolPathv6.ImportPolicy().Config())
Expand Down Expand Up @@ -445,6 +476,7 @@ func configureOTG(t *testing.T, otg *otg.OTG) gosnappi.Config {
t.Logf("Pushing config to ATE and starting protocols...")
otg.PushConfig(t, config)
otg.StartProtocols(t)

return config
}

Expand All @@ -454,7 +486,7 @@ func validateOTGBgpPrefixV4AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
_, ok := gnmi.WatchAll(t,
otg,
gnmi.OTG().BgpPeer(peerName).UnicastIpv4PrefixAny().State(),
30*time.Second,
60*time.Second,
func(v *ygnmi.Value[*otgtelemetry.BgpPeer_UnicastIpv4Prefix]) bool {
_, present := v.Val()
return present
Expand All @@ -466,7 +498,7 @@ func validateOTGBgpPrefixV4AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
if bgpPrefix.Address != nil && bgpPrefix.GetAddress() == ipAddr &&
bgpPrefix.PrefixLength != nil && bgpPrefix.GetPrefixLength() == prefixLen {
foundPrefix = true
t.Logf("Prefix recevied on OTG is correct, got prefix %v, want prefix %v", bgpPrefix, ipAddr)
t.Logf("Prefix recevied on OTG is correct, got prefix %v, want prefix %v", bgpPrefix.GetAddress(), ipAddr)
switch pathAttr {
case setMEDPolicy:
if bgpPrefix.GetMultiExitDiscriminator() != metric {
Expand All @@ -475,12 +507,14 @@ func validateOTGBgpPrefixV4AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
t.Logf("For Prefix %v, got MED %d want MED %d", bgpPrefix.GetAddress(), bgpPrefix.GetMultiExitDiscriminator(), metric)
}
case setLocalPrefPolicy:
validateImportRoutingPolicy(t, dut, ipAddr, metric)
if !deviations.BGPRibOcPathUnsupported(dut) {
validateImportRoutingPolicy(t, dut, ipAddr, metric)
}
case setPrependPolicy:
if len(bgpPrefix.AsPath[0].GetAsNumbers()) != int(metric) {
t.Errorf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), int(metric))
} else {
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath), int(metric))
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), int(metric))
}
case setNxtPolicy:
if bgpPrefix.GetMultiExitDiscriminator() != metric {
Expand All @@ -491,7 +525,7 @@ func validateOTGBgpPrefixV4AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
if len(bgpPrefix.AsPath[0].GetAsNumbers()) != asnRepeatN+1 {
t.Errorf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), asnRepeatN+1)
} else {
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath), asnRepeatN+1)
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), asnRepeatN+1)
}
default:
t.Errorf("Incorrect BGP Path Attribute. Expected MED, Local Pref or AS Path Prepend!!!!")
Expand All @@ -511,7 +545,7 @@ func validateOTGBgpPrefixV6AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
_, ok := gnmi.WatchAll(t,
otg,
gnmi.OTG().BgpPeer(peerName).UnicastIpv6PrefixAny().State(),
30*time.Second,
60*time.Second,
func(v *ygnmi.Value[*otgtelemetry.BgpPeer_UnicastIpv6Prefix]) bool {
_, present := v.Val()
return present
Expand All @@ -523,7 +557,7 @@ func validateOTGBgpPrefixV6AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
if bgpPrefix.Address != nil && bgpPrefix.GetAddress() == ipAddr &&
bgpPrefix.PrefixLength != nil && bgpPrefix.GetPrefixLength() == prefixLen {
foundPrefix = true
t.Logf("Prefix recevied on OTG is correct, got prefix %v, want prefix %v", bgpPrefix, ipAddr)
t.Logf("Prefix recevied on OTG is correct, got prefix %v, want prefix %v", bgpPrefix.GetAddress(), ipAddr)
switch pathAttr {
case setMEDPolicy:
if bgpPrefix.GetMultiExitDiscriminator() != metric {
Expand All @@ -532,12 +566,14 @@ func validateOTGBgpPrefixV6AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
t.Logf("For Prefix %v, got MED %d want MED %d", bgpPrefix.GetAddress(), bgpPrefix.GetMultiExitDiscriminator(), metric)
}
case setLocalPrefPolicy:
validateImportRoutingPolicyV6(t, dut, ipAddr, metric)
if !deviations.BGPRibOcPathUnsupported(dut) {
validateImportRoutingPolicyV6(t, dut, ipAddr, metric)
}
case setPrependPolicy:
if len(bgpPrefix.AsPath[0].GetAsNumbers()) != int(metric) {
t.Errorf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), int(metric))
} else {
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath), int(metric))
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), int(metric))
}
case setNxtPolicy:
if bgpPrefix.GetMultiExitDiscriminator() != metric {
Expand All @@ -548,7 +584,7 @@ func validateOTGBgpPrefixV6AndASLocalPrefMED(t *testing.T, otg *otg.OTG, dut *on
if len(bgpPrefix.AsPath[0].GetAsNumbers()) != asnRepeatN+1 {
t.Errorf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), asnRepeatN+1)
} else {
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath), asnRepeatN+1)
t.Logf("For Prefix %v, got AS Path Prepend %d want AS Path Prepend %d", bgpPrefix.GetAddress(), len(bgpPrefix.AsPath[0].GetAsNumbers()), asnRepeatN+1)
}
default:
t.Errorf("Incorrect Routing Policy. Expected MED, Local Pref or AS Path Prepend!!!!")
Expand Down Expand Up @@ -735,6 +771,7 @@ func TestBGPPolicy(t *testing.T) {
if tc.isDeletePolicy {
deleteBGPImportExportPolicy(t, dut, tc.deleteNbrv4, tc.deleteNbrv6, atePort2.IPv4, atePort2.IPv6)
}

// Configure Routing Policy on the DUT.
configureASLocalPrefMEDPolicy(t, dut, tc.rpPolicy, tc.policyValue, tc.policyStatement, tc.asn)
if !deviations.DefaultRoutePolicyUnsupported(dut) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ platform_exceptions: {
skip_setting_statement_for_policy: true
}
}
platform_exceptions: {
platform: {
vendor: JUNIPER
}
deviations: {
bgp_rib_oc_path_unsupported: true
bgp_set_med_v7_unsupported: true
}
}
platform_exceptions: {
platform: {
vendor: NOKIA
Expand Down
Loading

0 comments on commit 5614acd

Please sign in to comment.