Kernel skip some route updates

[donaldsharp]

See individual commits for discussion, but in some cases skip some route update installations when certain conditions are met.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR optimizes zebra's dataplane by skipping redundant kernel route updates when both the nexthop group ID and route type remain unchanged between old and new route contexts.

Key Changes

• Optimization Logic: When kernel nexthops are enabled and a route update has the same nexthop group ID and type as the existing route, the kernel update is skipped since the kernel already has the correct nexthop data
• Counter Tracking: Added dg_routes_kernel_skipped atomic counter to track skipped updates, exposed via show zebra dplane detailed command
• Test Coverage: Added test case in test_zebra_nhg.py to verify the skip counter increments during interface down/up scenarios

Use Case

This addresses the scenario where an interface goes down/up: zebra adjusts the nexthop group internally, and an upper-level protocol (BGP/OSPF) sends a route update removing the downed nexthop. Since the kernel was already updated by zebra's nexthop group management, the route update to the kernel is redundant and can be safely skipped.

Issue Found

• The test has incorrect assertion logic at line 392-394 that checks if skipped_updates_before == 0 but uses an error message about retrieval failure, when -1 indicates retrieval failure and positive values indicate successful retrieval with existing skipped updates

Confidence Score: 4/5

• Safe to merge after fixing the test assertion logic issue
• The core optimization logic in zebra_dplane.c is sound and follows existing patterns (similar check exists at line 4628). The atomic counter is properly implemented with memory_order_relaxed. However, the test contains an incorrect assertion that conflates retrieval failure (-1) with unexpected pre-existing skipped updates (> 0), which should be fixed before merge.
• tests/topotests/zebra_nhg_check/test_zebra_nhg.py requires assertion logic fix at line 392-394

Important Files Changed

Filename	Overview
zebra/zebra_dplane.c	Added optimization to skip redundant kernel route updates when nexthop group and route type unchanged, with proper counter tracking
tests/topotests/zebra_nhg_check/test_zebra_nhg.py	Added test validation for route skip counter, but contains incorrect assertion logic for error checking

Sequence Diagram

sequenceDiagram
    participant Protocol as Upper Protocol (BGP/OSPF)
    participant Zebra as Zebra RIB
    participant Dplane as Dataplane Provider
    participant Kernel as Linux Kernel
    
    Note over Protocol,Kernel: Interface Down Event Scenario
    
    Protocol->>Zebra: Route update (remove failed nexthop)
    Zebra->>Dplane: DPLANE_OP_ROUTE_UPDATE context
    Note over Dplane: ctx with old_nhe_id and nhe_id
    
    alt Skip kernel update already set
        Dplane->>Dplane: Check dplane_ctx_is_skip_kernel()
        Dplane->>Dplane: Mark success, skip kernel
    else Kernel nexthops enabled + same NHG + same type
        Dplane->>Dplane: Check zebra_nhg_kernel_nexthops_enabled()
        Dplane->>Dplane: Compare old_nhe_id == nhe_id
        Dplane->>Dplane: Compare old_type == type
        Dplane->>Dplane: Increment dg_routes_kernel_skipped
        Dplane->>Dplane: Mark ZEBRA_DPLANE_REQUEST_SUCCESS
        Note over Dplane: Skip actual kernel update
    else Normal kernel update
        Dplane->>Kernel: netlink route update
        Kernel-->>Dplane: Update result
    end
    
    Dplane->>Zebra: Update result (success)
    Zebra->>Protocol: Trigger nexthop tracking
    Zebra->>Protocol: Handle redistribution

Loading

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Assertion logic and error message are mismatched. The condition checks if skipped_updates_before == 0, but the error message says "Could not retrieve route updates skipped before shutdown". If get_route_updates_skipped() returns -1, it means retrieval failed (which should be checked separately). If it returns a positive value, it means retrieval succeeded but there were already skipped updates before the test. Consider:

Suggested change

	assert (
	skipped_updates_before == 0
	), "Could not retrieve route updates skipped before shutdown"
	assert (
	skipped_updates_before >= 0
	), "Could not retrieve route updates skipped before shutdown"

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/topotests/zebra_nhg_check/test_zebra_nhg.py
Line: 392:394

Comment:
Assertion logic and error message are mismatched. The condition checks if `skipped_updates_before == 0`, but the error message says "Could not retrieve route updates skipped before shutdown". If `get_route_updates_skipped()` returns `-1`, it means retrieval failed (which should be checked separately). If it returns a positive value, it means retrieval succeeded but there were already skipped updates before the test. Consider:

```suggestion
    assert (
        skipped_updates_before >= 0
    ), "Could not retrieve route updates skipped before shutdown"
```

How can I resolve this? If you propose a fix, please make it concise.

[donaldsharp]

I disagree with the frrbot recomendations. they are not with code I chagned.

[riw777]

looks good