bgpd: fix BGP_ATTR_NEXT_HOP flag handling in bgp_attr_default_set()

[enkechen-panw]

bgp_attr_default_set() unconditionally set the BGP_ATTR_NEXT_HOP flag
on every call, even though attr.nexthop (the IPv4 address field) is
all-zeros and not yet assigned. This flag is used by
BGP_ATTR_NEXTHOP_AFI_IP6 to distinguish IPv4 vs IPv6 nexthops, so
having it always set caused non-IPv4 routes to be misidentified.
Callers were working around this by manually calling UNSET_FLAG for
non-IPv4 cases, which was fragile and error-prone.

Remove the unconditional flag from bgp_attr_default_set() and enforce
the invariant that BGP_ATTR_NEXT_HOP is set where and only where
attr.nexthop is assigned as an actual IPv4 nexthop.

[greptile-apps]

Greptile Summary

This PR fixes a bug in bgp_static_update() where the BGP_ATTR_NEXT_HOP flag was not being cleared for non-IPv4 address families. The bgp_attr_default_set() function unconditionally sets this flag, but the BGP_ATTR_NEXTHOP_AFI_IP6 macro relies on it being unset to correctly identify IPv6 nexthops. Without this fix, IPv6 static routes could be misidentified as having IPv4 nexthops, affecting downstream consumers like SNMP, EVPN nexthop conversion, and MPLS VPN route import/export.

• Adds an else branch to clear BGP_ATTR_NEXT_HOP flag when afi != AFI_IP in bgp_static_update(), consistent with the existing pattern in bgp_redistribute_add() at line 10595-10596 of the same file
• The fix is minimal and targeted — only the necessary flag is cleared for non-IPv4 address families

Confidence Score: 5/5

• This PR is safe to merge — it is a small, well-understood bug fix that follows an existing pattern in the same file.
• The change is a 2-line addition that mirrors an identical pattern already present in bgp_redistribute_add(). The BGP_ATTR_NEXT_HOP flag semantics are well-established in the codebase and the fix correctly addresses the documented issue with BGP_ATTR_NEXTHOP_AFI_IP6 macro behavior.
• No files require special attention.

Important Files Changed

Filename	Overview
bgpd/bgp_route.c	Adds UNSET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) for non-IPv4 AFIs in bgp_static_update(), fixing incorrect nexthop AFI detection via BGP_ATTR_NEXTHOP_AFI_IP6 macro. Mirrors the existing pattern in bgp_redistribute_add().

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["bgp_static_update()"] --> B["bgp_attr_default_set()"]
    B --> C["Sets BGP_ATTR_NEXT_HOP flag"]
    C --> D{"afi == AFI_IP?"}
    D -->|Yes| E["nh_length = IPV4_MAX_BYTELEN\n(flag remains set)"]
    D -->|No| F["UNSET BGP_ATTR_NEXT_HOP flag\n(NEW: this PR)"]
    E --> G["Build NHC and set attributes"]
    F --> G
    G --> H["BGP_ATTR_NEXTHOP_AFI_IP6 macro"]
    H --> I{"flag unset AND\nmp_nexthop_len is IPv6?"}
    I -->|Yes| J["Treat as IPv6 nexthop"]
    I -->|No| K["Treat as IPv4 nexthop"]

Loading

_{Last reviewed commit: 93458ff}

[donaldsharp]

This immediately implies that the usage of bgp_attr_default_set is a problem throughout our code base. I personally would like to have a way that this function can be used properly without having to figure out how to properly set this flag after the call. Otherwise we are just setting ourselves up for more bugs in the future( and we probably already have something wrong somewhere with all the other usages ). Why are we not fixing that instead of just hoping we get it right everywhere else?

[enkechen-panw]

This immediately implies that the usage of bgp_attr_default_set is a problem throughout our code base. I personally would like to have a way that this function can be used properly without having to figure out how to properly set this flag after the call. Otherwise we are just setting ourselves up for more bugs in the future( and we probably already have something wrong somewhere with all the other usages ). Why are we not fixing that instead of just hoping we get it right everywhere else?

That's a good point, thanks. Let me investigate.

[ton31337]

LGTM

[riw777]

looks good