TE-16.3 fix by suredhar · Pull Request #5495 · openconfig/featureprofiles

suredhar · 2026-05-21T08:43:39Z

Fixes ISIS interface reference configuration and BGP configuration.

gemini-code-assist · 2026-05-21T08:43:47Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request updates the network configuration logic for ISIS and BGP within the test suite. The changes ensure more robust interface reference handling across different hardware vendors and improve the accuracy of telemetry verification paths by refining how interface suffixes are applied.

Highlights

ISIS Interface Configuration: Updated ISIS interface configuration to explicitly parse and set base interface and subinterface references, including handling for specific device deviations.
BGP Configuration: Modified BGP local address assignment to use the loopback interface name specifically for Cisco devices.
Telemetry Verification: Adjusted the logic for telemetry path verification to require both deviation flags to be present before appending the .0 suffix.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

OpenConfigBot · 2026-05-21T08:44:08Z

Pull Request Functional Test Report for #5495 / `75d4b68`

Virtual Devices

Device	Test	Test Documentation	Job	Raw Log
Arista cEOS		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Cisco 8000E		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Cisco XRd		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Juniper ncPTX		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Nokia SR Linux		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Openconfig Lemming		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios

Hardware Devices

Device	Test	Test Documentation	Raw Log
Arista 7808		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Cisco 8808		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Juniper PTX10008		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Nokia 7250 IXR-10e		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios

Help

gemini-code-assist

Code Review

This pull request updates the ISIS and BGP configuration logic to better handle subinterfaces and vendor-specific requirements. Key changes include parsing interface names for subinterface indices in configureISIS and adjusting BGP local address settings for Cisco devices. Review feedback highlighted a logic inconsistency in the ISIS telemetry verification regarding interface suffixing and noted a violation of the deviation guidelines where dut.Vendor() was used instead of the deviations package for vendor-specific logic.

gemini-code-assist · 2026-05-21T08:45:51Z

+	if deviations.ExplicitInterfaceInDefaultVRF(dut) && deviations.InterfaceRefInterfaceIDFormat(dut) {
 		dutIntf = dutIntf + ".0"
 	}


The condition for adding the ".0" suffix has been changed from "||" to "&&", which creates an inconsistency with the configuration logic in "configureISIS" (lines 577-579) that still uses "||". This mismatch will cause the telemetry check to look for the wrong interface name if only one of the deviations is enabled. Additionally, the check for an existing dot in the interface name should be included to prevent double-suffixing, matching the logic used in the configuration step.

Suggested change

if deviations.ExplicitInterfaceInDefaultVRF(dut) && deviations.InterfaceRefInterfaceIDFormat(dut) {

dutIntf = dutIntf + ".0"

}

if (deviations.ExplicitInterfaceInDefaultVRF(dut) || deviations.InterfaceRefInterfaceIDFormat(dut)) && !strings.Contains(dutIntf, ".") {

dutIntf = dutIntf + ".0"

}

This needs to be corrected as well. I think we should just remove InterfaceRefInterfaceIDFormat. These are two different deviations used by two different vendors and can never be True

Addressed in latest commit.

This can be true for 2 vendors independently and simultaneously as discussed. Latest commit has a better approach. PTAL

gemini-code-assist · 2026-05-21T08:45:51Z

+	if dut.Vendor() == ondatra.CISCO {
+		localAddressLeaf = loopbackIntfName
+	}


Avoid using dut.Vendor() for vendor-specific logic or configurations in tests. Instead, define and use a deviation in the deviations package to maintain test abstraction and portability across different vendors, as per the general rules.

References

Avoid using "dut.Vendor()" for vendor-specific logic or configurations in tests. Instead, use the "deviations" package to maintain test abstraction and portability across different vendors.

This will be addressed via existing deviation use_interface_name_for_ibgp_neighbor_transport_ipv4_local_address used in related tests.

Addressed in latest commit.

AmrNJ · 2026-05-21T13:33:34Z

 	}
-	if deviations.ExplicitInterfaceInDefaultVRF(dut) || deviations.InterfaceRefInterfaceIDFormat(dut) {
-		intfName = intfName + ".0"
+	// Parse interface name to extract base interface and subinterface index


This should be through OC.

will correct it

Addressed in latest commit.

…on for isis

karthikeya-remilla · 2026-05-25T08:13:21Z

/gemini review

karthikeya-remilla · 2026-05-25T08:14:37Z

Latest changes:

Nokia does not need ExplicitInterfaceInDefaultVRF to imply portX.0 in
ISIS, so we can decouple that
if the vendor needs interface-ref ID formatting and the name is a base interface, append .0
if the name already has a dot, leave it alone

gemini-code-assist

Code Review

This pull request introduces a new deviation UseInterfaceNameForIBGPNeighborTransportIpv4LocalAddress to support BGP neighborship using interface names on specific platforms. It also refactors ISIS interface configuration to use a helper function and includes logic for populating interface-ref. Review feedback identifies a logic error where GetOrCreateInterfaceRef() is called regardless of deviation checks, potentially negating them. Additionally, it was noted that redundant gNMI GET calls for interface names and subinterface indices should be replaced with known values to improve efficiency and portability.

gemini-code-assist · 2026-05-25T08:15:37Z

+	// Always populate interface-ref with base interface and subinterface index
+	isisIntf.GetOrCreateInterfaceRef().Interface = ygot.String(baseIntf)
+	isisIntf.GetOrCreateInterfaceRef().Subinterface = ygot.Uint32(subIdx)


This block contains a logic error: calling GetOrCreateInterfaceRef() will recreate the InterfaceRef container even if it was intended to be omitted by the InterfaceRefConfigUnsupported deviation check. This effectively negates the deviation. Ensure that vendor deviations are correctly respected to maintain cross-vendor compatibility.

References

Avoid using dut.Vendor() for vendor-specific logic or configurations in tests. Instead, use the deviations package to maintain test abstraction and portability across different vendors.

Resolved in latest commit.

gemini-code-assist · 2026-05-25T08:15:37Z

+	baseIntf := gnmi.Get(t, dut, gnmi.OC().Interface(intfName).Name().Config())
+	subIdx := gnmi.Get(t, dut, gnmi.OC().Interface(baseIntf).Subinterface(0).Index().Config())
+
+	isisIntfName := isisInterfaceID(dut, intfName)
+
+	isisIntf := isis.GetOrCreateInterface(isisIntfName)
+	isisIntf.GetOrCreateInterfaceRef().Interface = ygot.String(baseIntf)
+	isisIntf.GetOrCreateInterfaceRef().Subinterface = ygot.Uint32(subIdx)
+	if deviations.InterfaceRefConfigUnsupported(dut) {
+		isisIntf.InterfaceRef = nil
+	}


The gNMI GET calls to fetch the interface name and subinterface index are redundant as these values are already known (intfName and 0). Additionally, use the deviations package to handle vendor-specific configuration requirements for interface-ref to maintain test portability across different vendors.

Suggested change

baseIntf := gnmi.Get(t, dut, gnmi.OC().Interface(intfName).Name().Config())

subIdx := gnmi.Get(t, dut, gnmi.OC().Interface(baseIntf).Subinterface(0).Index().Config())

isisIntfName := isisInterfaceID(dut, intfName)

isisIntf := isis.GetOrCreateInterface(isisIntfName)

isisIntf.GetOrCreateInterfaceRef().Interface = ygot.String(baseIntf)

isisIntf.GetOrCreateInterfaceRef().Subinterface = ygot.Uint32(subIdx)

if deviations.InterfaceRefConfigUnsupported(dut) {

isisIntf.InterfaceRef = nil

}

isisIntfName := isisInterfaceID(dut, intfName)

isisIntf := isis.GetOrCreateInterface(isisIntfName)

if !deviations.InterfaceRefConfigUnsupported(dut) {

isisIntf.GetOrCreateInterfaceRef().Interface = ygot.String(intfName)

isisIntf.GetOrCreateInterfaceRef().Subinterface = ygot.Uint32(0)

}

References

Avoid using dut.Vendor() for vendor-specific logic or configurations in tests. Instead, use the deviations package to maintain test abstraction and portability across different vendors.

Agreed and resolved in latest commit.

karthikeya-remilla · 2026-05-26T05:06:13Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new deviation UseInterfaceNameForIBGPNeighborTransportIpv4LocalAddress to support configuring the BGP neighbor transport local address using an interface name instead of an IPv4 address. It also updates ISIS interface configuration logic to fetch and set interface references. The review comments point out two critical issues: first, a logic bug in verifyISISTelemetry where checking strings.Contains(dutIntf, ".") prevents appending the .0 suffix when the interface name does not already contain a dot; second, an inefficiency in configureISIS where querying the DUT configuration via gNMI for static/known values like the interface name and subinterface index adds unnecessary latency and flakiness.

gemini-code-assist · 2026-05-26T05:07:15Z

+	if strings.Contains(dutIntf, ".") {
+		dutIntf = isisInterfaceID(dut, dutIntf)
+	}	


There is a logic bug here. The condition strings.Contains(dutIntf, ".") checks if the interface name already contains a dot. If it does NOT contain a dot (e.g., "port8"), the condition is false, and isisInterfaceID is never called. This means the .0 suffix will never be appended when InterfaceRefInterfaceIDFormat is enabled, causing telemetry verification to fail. Since isisInterfaceID already internally checks !strings.Contains(intfName, "."), you should call it directly without the outer if check.

dutIntf = isisInterfaceID(dut, dutIntf)

gemini-code-assist · 2026-05-26T05:07:15Z

+	baseIntf := gnmi.Get(t, dut, gnmi.OC().Interface(intfName).Name().Config())
+	subIdx := gnmi.Get(t, dut, gnmi.OC().Interface(baseIntf).Subinterface(0).Index().Config())


Querying the DUT configuration via gNMI for static/known values like the interface name and subinterface index is unnecessary and inefficient. It adds round-trip latency and can introduce flakiness if the configuration has not fully converged. Since intfName is already known and the subinterface index is statically 0, you can assign these directly.

Suggested change

baseIntf := gnmi.Get(t, dut, gnmi.OC().Interface(intfName).Name().Config())

subIdx := gnmi.Get(t, dut, gnmi.OC().Interface(baseIntf).Subinterface(0).Index().Config())

baseIntf := intfName

subIdx := uint32(0)

Not applicable with latest commit.

Fix encap FRR ISIS interface adjacency

d5aff60

suredhar requested a review from a team as a code owner May 21, 2026 08:43

gemini-code-assist Bot reviewed May 21, 2026

View reviewed changes

Merge branch 'main' into suredhar_te-16.3_fix

b5a2427

AmrNJ reviewed May 21, 2026

View reviewed changes

suredhar and others added 4 commits May 21, 2026 20:22

Merge branch 'main' into suredhar_te-16.3_fix

14ee74b

Move the .0 suffix into OC and fix the adjacency verification conditi…

0082978

…on for isis

deviation for TE-16.2

d55b425

dev

b477183

karthikeya-remilla requested a review from a team as a code owner May 21, 2026 15:58

AmrNJ requested changes May 23, 2026

View reviewed changes

karthikeya-remilla added 3 commits May 24, 2026 12:08

Merge branch 'main' into suredhar_te-16.3_fix

c8d370b

cleaner vendor handling

b6b245d

Merge branch 'main' into suredhar_te-16.3_fix

c893768

gemini-code-assist Bot reviewed May 25, 2026

View reviewed changes

final patch

05982f6

karthikeya-remilla requested a review from AmrNJ May 25, 2026 08:36

suredhar added 2 commits May 25, 2026 18:22

Merge branch 'main' into suredhar_te-16.3_fix

d6dd492

Update encap_frr_with_reencap_vrf_test.go

f150090

gemini-code-assist Bot reviewed May 26, 2026

View reviewed changes

suredhar added 2 commits May 26, 2026 11:13

Update encap_frr_with_reencap_vrf_test.go

ef5a818

Update encap_frr_with_reencap_vrf_test.go

75d4b68

		baseIntf := gnmi.Get(t, dut, gnmi.OC().Interface(intfName).Name().Config())
		subIdx := gnmi.Get(t, dut, gnmi.OC().Interface(baseIntf).Subinterface(0).Index().Config())

Conversation

suredhar commented May 21, 2026

Uh oh!

gemini-code-assist Bot commented May 21, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

OpenConfigBot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Functional Test Report for #5495 / 75d4b68

Virtual Devices

Hardware Devices

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

karthikeya-remilla May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karthikeya-remilla commented May 25, 2026

Uh oh!

karthikeya-remilla commented May 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karthikeya-remilla commented May 26, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

OpenConfigBot commented May 21, 2026 •

edited

Loading

Pull Request Functional Test Report for #5495 / `75d4b68`

karthikeya-remilla May 21, 2026 •

edited

Loading