Skip to content

[Optimization] Improve traceroute_packetin_with_vrf_selection_test run time #5806

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.

Sign up for GitHub

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

Open
mansi0803 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Optimization] Improve traceroute_packetin_with_vrf_selection_test run time #5806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e2843b
traceroute_packetin_with_vrf_selection_test: Replace fixed sleep with…
agarwalmansii Jul 30, 2026
40b9d52
Replace hardcoded sleeps with otgutils.WaitForARP and gNMI packet awa…
agarwalmansii Jul 30, 2026
49eba6d
style: fix gofmt formatting issues in traceroute_packetin_with_vrf_se…
agarwalmansii Jul 31, 2026
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
24 changes: 20 additions & 4 deletions feature/p4rt/otg_tests/traceroute_packetin_with_vrf_selection_test/packetin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/google/gopacket"
"github.com/google/gopacket/layers"
"github.com/open-traffic-generator/snappi/gosnappi"
"github.com/openconfig/featureprofiles/internal/otgutils"
"github.com/openconfig/featureprofiles/internal/p4rtutils"
"github.com/openconfig/ondatra"
"github.com/openconfig/ondatra/gnmi"
Expand Down Expand Up @@ -102,7 +103,7 @@ func decodePacket6(t *testing.T, packetData []byte) uint8 {

// testTraffic sends traffic flow for duration seconds and returns the
// number of packets sent out.
func testTraffic(t *testing.T, top gosnappi.Config, ate *ondatra.ATEDevice, flows []gosnappi.Flow, srcEndPoint gosnappi.Port, duration int, cs gosnappi.ControlState) int {
func testTraffic(t *testing.T, top gosnappi.Config, ate *ondatra.ATEDevice, flows []gosnappi.Flow, srcEndPoint gosnappi.Port, targetPkts uint64, cs gosnappi.ControlState) int {
t.Helper()
initialOutPkts := make(map[string]uint64, len(flows))
top.Flows().Clear()
Expand All @@ -113,12 +114,27 @@ func testTraffic(t *testing.T, top gosnappi.Config, ate *ondatra.ATEDevice, flow
}
ate.OTG().PushConfig(t, top)
ate.OTG().StartProtocols(t)
time.Sleep(30 * time.Second)
otgutils.WaitForARP(t, ate.OTG(), top, "IPv4")
otgutils.WaitForARP(t, ate.OTG(), top, "IPv6")

// START THE PACKET CAPTURE AFTER CONFIG IS PUSHED
ate.OTG().SetControlState(t, cs)
Comment thread
mansi0803 marked this conversation as resolved.

for _, flow := range flows {
initialOutPkts[flow.Name()] = gnmi.Get(t, ate.OTG(), gnmi.OTG().Flow(flow.Name()).Counters().OutPkts().State())
}
ate.OTG().StartTraffic(t)
time.Sleep(time.Duration(duration) * time.Second)
// AWAIT LOGIC INSTEAD OF SLEEP
for _, flow := range flows {
_, ok := gnmi.Watch(t, ate.OTG(), gnmi.OTG().Flow(flow.Name()).Counters().OutPkts().State(), time.Minute, func(val *ygnmi.Value[uint64]) bool {
pkts, present := val.Val()
return present && (pkts >= initialOutPkts[flow.Name()]+targetPkts)
}).Await(t)

if !ok {
t.Errorf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts)
}
Comment on lines +134 to +136

@gemini-code-assist gemini-code-assist Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the traffic flow fails to reach the target number of packets, the subsequent packet-in validations will inevitably fail or produce misleading results due to missing packets. Consider using t.Fatalf instead of t.Errorf to terminate the test immediately and avoid cascading failures.

Suggested change
if !ok {
t.Errorf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts)
}
if !ok {
t.Fatalf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts)
}
References
  1. In tests, t.Fatalf is preferred over t.Errorf when a failure makes subsequent test steps meaningless, as this fails fast and reduces overall test execution time.

}
ate.OTG().StopTraffic(t)

cs.Port().Capture().SetState(gosnappi.StatePortCaptureState.STOP)
Expand Down Expand Up @@ -174,7 +190,7 @@ func testPacketIn(ctx context.Context, t *testing.T, args *testArgs, isIPv4 bool
for _, flowValue := range flowValues {
flow = append(flow, args.packetIO.GetTrafficFlow(args.ate, dstMac, isIPv4, 1, 300, 50, ipv4InnerDst, flowValue))
}
pktOut := testTraffic(t, args.top, args.ate, flow, srcEndPoint, 60, cs)
pktOut := testTraffic(t, args.top, args.ate, flow, srcEndPoint, 500, cs)

@AmrNJ AmrNJ Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why 500s? 60s seems fine.

var countPkts = map[string]int{"11": 0, "12": 0, "13": 0, "14": 0, "15": 0, "16": 0, "17": 0}

packetInTests := []struct {
Expand Down
5 changes: 0 additions & 5 deletions ...re/p4rt/otg_tests/traceroute_packetin_with_vrf_selection_test/traceroute_packetin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"testing"
"time"

"github.com/cisco-open/go-p4/p4rt_client"
"github.com/cisco-open/go-p4/utils"
Expand Down Expand Up @@ -133,10 +132,6 @@ func testPacket(t *testing.T, args *testArgs, cs gosnappi.ControlState, flowValu

configureDeviceID(ctx, t, dut)

ate.OTG().PushConfig(t, top)
ate.OTG().StartProtocols(t)
time.Sleep(30 * time.Second)

args = &testArgs{
ctx: ctx,
leader: args.leader,
Expand Down
5 changes: 0 additions & 5 deletions ...raceroute_packetin_with_vrf_selection_test/traceroute_packetin_with_vrf_selection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,13 +955,8 @@ func startCapture(t *testing.T, args *testArgs, capturePortList []string) gosnap
args.otgConfig.Captures().Add().SetName("packetCapture").
SetPortNames(capturePortList).
SetFormat(gosnappi.CaptureFormat.PCAP)
args.otg.PushConfig(t, args.otgConfig)
time.Sleep(30 * time.Second)
args.otg.StartProtocols(t)
time.Sleep(30 * time.Second)
cs := gosnappi.NewControlState()
cs.Port().Capture().SetState(gosnappi.StatePortCaptureState.START)
args.otg.SetControlState(t, cs)
return cs
}

Expand Down
Loading