Zebra local addresses same network 2

[pguibert6WIND]

No description provided.

[greptile-apps]

Greptile Summary

This PR fixes zebra's handling of multiple connected addresses on the same network by tracking the kernel's promote_secondaries sysctl per-interface and using it to decide whether to retain connected-prefix and local /32 routes when a primary address is deleted.

• zebra/connected.c: connected_up() and connected_down() are refactored to use install_prefix/remove_prefix flags instead of early returns, so the local-route path is still reached when a prefix-route skip is needed. connected_down() also clears ZEBRA_IFA_SECONDARY on the surviving secondary when the primary is removed and promotion is active.
• zebra/if_netlink.c / zebra/interface.c: A new parser reads promote_secondaries from RTM_NEWLINK IFLA_AF_SPEC and stores it in the dplane context; zebra_if_dplane_ifp_handling syncs the value into ZIF_FLAG_IPV4_PROMOTE_SECONDARIES on the per-interface zebra_if.
• Tests: Two new topotests cover the promote-secondaries-on and promote-secondaries-off scenarios end-to-end.

Confidence Score: 3/5

Two real defects in interface.c need fixing before merging: an always-true debug guard that floods logs, and a missing UNSET_FLAG branch that prevents the promote-secondaries flag from ever being cleared.

The always-true if (1 || IS_ZEBRA_DEBUG_KERNEL) guard causes every RTM_NEWLINK to emit a log line in production. More critically, SET_FLAG is called unconditionally on any state change, so toggling promote_secondaries back to 0 via sysctl is never reflected in zebra — the flag stays set permanently, leading to stale connected-route decisions.

zebra/interface.c — the promote_secondaries sync block at line ~2250 needs both the always-true guard and the missing UNSET_FLAG path corrected before this is safe to merge.

Important Files Changed

Filename	Overview
zebra/interface.c	Adds promote_secondaries tracking and display; contains two bugs: always-true debug guard and missing UNSET_FLAG path so ZIF_FLAG_IPV4_PROMOTE_SECONDARIES can never be cleared.
zebra/connected.c	Refactors connected_up/down to skip prefix-route install/removal when other same-network addresses remain; adds secondary-flag promotion logic on primary deletion. Contains an unused zif variable in connected_up().
zebra/if_netlink.c	Adds local IPV4_DEVCONF enum and a new function to parse promote_secondaries from RTM_NEWLINK IFLA_AF_SPEC; uses bare magic number 1 for IFLA_INET_CONF.
zebra/zebra_dplane.c	Adds promote_secondaries field to dplane_intf_info with standard setter/getter pair; straightforward and correct.
zebra/zebra_dplane.h	Declares the two new promote_secondaries dplane accessor prototypes; no issues.
zebra/interface.h	Adds ZIF_FLAG_IPV4_PROMOTE_SECONDARIES (bit 7) to zebra_if_flags enum; clean change.
tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py	Adds two end-to-end topotests covering secondary local /32 route lifecycle with and without promote_secondaries; well structured.

Sequence Diagram

sequenceDiagram
    participant K as Linux Kernel
    participant NL as if_netlink.c
    participant DP as zebra_dplane
    participant IC as interface.c
    participant ZIF as zebra_if (flags)
    participant CD as connected.c

    K->>NL: RTM_NEWLINK (IFLA_AF_SPEC with IFLA_INET_CONF)
    NL->>NL: netlink_interface_promote_secondaries_update()
    NL->>DP: dplane_ctx_set_ifp_promote_secondaries(ctx, val)
    DP->>IC: zebra_if_dplane_ifp_handling(ctx)
    IC->>DP: dplane_ctx_intf_is_promote_secondaries(ctx)
    IC->>ZIF: SET/UNSET ZIF_FLAG_IPV4_PROMOTE_SECONDARIES

    K->>NL: RTM_DELADDR (primary address deleted)
    NL->>CD: connected_down(ifp, ifc)
    CD->>ZIF: CHECK_FLAG(ZIF_FLAG_IPV4_PROMOTE_SECONDARIES)
    alt promote_secondaries enabled AND secondary exists
        CD->>CD: UNSET ZEBRA_IFA_SECONDARY on surviving addr
        CD->>CD: "remove_prefix = false (keep connected route)"
        CD->>CD: remove local /32 for deleted address
    else no surviving address / promotion disabled
        CD->>CD: "remove_prefix = true"
        CD->>CD: rib_delete connected + local routes
    end

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
zebra/interface.c:2252-2253
**Debug guard always evaluates to true**

`if (1 || IS_ZEBRA_DEBUG_KERNEL)` short-circuits unconditionally, so this `zlog_debug` fires on every interface link-change event regardless of debug settings. This will flood the system log on any deployment that receives frequent RTM_NEWLINK notifications.

### Issue 2 of 4
zebra/interface.c:2250-2257
**`UNSET_FLAG` path missing for promote_secondaries**

When `promote_secondaries` is `false` and `ZIF_FLAG_IPV4_PROMOTE_SECONDARIES` is currently set, the condition is true (they differ) but the code unconditionally calls `SET_FLAG`, re-setting the flag instead of clearing it. Once the flag is set on an interface it can never be cleared, and zebra will continue treating that interface as having `promote_secondaries` enabled even after the sysctl is disabled — causing incorrect connected-route behavior.

```suggestion
			if (promote_secondaries !=
			    !!CHECK_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES)) {
				if (IS_ZEBRA_DEBUG_KERNEL)
					zlog_debug("Intf %s(%u) has promote secondaries turned %s",
						   name, ifp->ifindex,
						   promote_secondaries ? "on" : "off");
				if (promote_secondaries)
					SET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
				else
					UNSET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
			}
```

### Issue 3 of 4
zebra/connected.c:238-240
**Unused variable `zif` in `connected_up()`**

`zif` is declared but never assigned or referenced in `connected_up()`. It is likely a leftover from a copy-paste from `connected_down()`. While benign, it adds noise and may confuse future readers.

```suggestion
	bool install_local = true;
	bool install_prefix = true;
```

### Issue 4 of 4
zebra/if_netlink.c:812
**Magic number for inner netlink attribute type**

`j->rta_type == 1` refers to `IFLA_INET_CONF` (value 1 in the kernel's `ifla_inet_attr` enum). Defining a named constant alongside the `IPV4_DEVCONF_*` enum already introduced in this PR would make the intent self-documenting.

_{Reviews (1): Last reviewed commit: "zebra: upon deletion, promote secondarie..." | Re-trigger Greptile}

[greptile-apps]

Debug guard always evaluates to true

if (1 || IS_ZEBRA_DEBUG_KERNEL) short-circuits unconditionally, so this zlog_debug fires on every interface link-change event regardless of debug settings. This will flood the system log on any deployment that receives frequent RTM_NEWLINK notifications.

Prompt To Fix With AI

This is a comment left during a code review.
Path: zebra/interface.c
Line: 2252-2253

Comment:
**Debug guard always evaluates to true**

`if (1 || IS_ZEBRA_DEBUG_KERNEL)` short-circuits unconditionally, so this `zlog_debug` fires on every interface link-change event regardless of debug settings. This will flood the system log on any deployment that receives frequent RTM_NEWLINK notifications.

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

[pguibert6WIND]

done

[greptile-apps]

UNSET_FLAG path missing for promote_secondaries

When promote_secondaries is false and ZIF_FLAG_IPV4_PROMOTE_SECONDARIES is currently set, the condition is true (they differ) but the code unconditionally calls SET_FLAG, re-setting the flag instead of clearing it. Once the flag is set on an interface it can never be cleared, and zebra will continue treating that interface as having promote_secondaries enabled even after the sysctl is disabled — causing incorrect connected-route behavior.

Suggested change

	if (promote_secondaries !=
	!!CHECK_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES)) {
	if (1 || IS_ZEBRA_DEBUG_KERNEL)
	zlog_debug("Intf %s(%u) has promote secondaries turned %s",
	name, ifp->ifindex,
	promote_secondaries ? "on" : "off");
	SET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
	}
	if (promote_secondaries !=
	!!CHECK_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES)) {
	if (IS_ZEBRA_DEBUG_KERNEL)
	zlog_debug("Intf %s(%u) has promote secondaries turned %s",
	name, ifp->ifindex,
	promote_secondaries ? "on" : "off");
	if (promote_secondaries)
	SET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
	else
	UNSET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: zebra/interface.c
Line: 2250-2257

Comment:
**`UNSET_FLAG` path missing for promote_secondaries**

When `promote_secondaries` is `false` and `ZIF_FLAG_IPV4_PROMOTE_SECONDARIES` is currently set, the condition is true (they differ) but the code unconditionally calls `SET_FLAG`, re-setting the flag instead of clearing it. Once the flag is set on an interface it can never be cleared, and zebra will continue treating that interface as having `promote_secondaries` enabled even after the sysctl is disabled — causing incorrect connected-route behavior.

```suggestion
			if (promote_secondaries !=
			    !!CHECK_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES)) {
				if (IS_ZEBRA_DEBUG_KERNEL)
					zlog_debug("Intf %s(%u) has promote secondaries turned %s",
						   name, ifp->ifindex,
						   promote_secondaries ? "on" : "off");
				if (promote_secondaries)
					SET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
				else
					UNSET_FLAG(zif->flags, ZIF_FLAG_IPV4_PROMOTE_SECONDARIES);
			}
```

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

[pguibert6WIND]

done

[greptile-apps]

Unused variable zif in connected_up()

zif is declared but never assigned or referenced in connected_up(). It is likely a leftover from a copy-paste from connected_down(). While benign, it adds noise and may confuse future readers.

Suggested change

	bool install_local = true;
	bool install_prefix = true;
	struct zebra_if *zif;
	bool install_local = true;
	bool install_prefix = true;

Prompt To Fix With AI

This is a comment left during a code review.
Path: zebra/connected.c
Line: 238-240

Comment:
**Unused variable `zif` in `connected_up()`**

`zif` is declared but never assigned or referenced in `connected_up()`. It is likely a leftover from a copy-paste from `connected_down()`. While benign, it adds noise and may confuse future readers.

```suggestion
	bool install_local = true;
	bool install_prefix = true;
```

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

[pguibert6WIND]

done

[greptile-apps]

Magic number for inner netlink attribute type

j->rta_type == 1 refers to IFLA_INET_CONF (value 1 in the kernel's ifla_inet_attr enum). Defining a named constant alongside the IPV4_DEVCONF_* enum already introduced in this PR would make the intent self-documenting.

Prompt To Fix With AI

This is a comment left during a code review.
Path: zebra/if_netlink.c
Line: 812

Comment:
**Magic number for inner netlink attribute type**

`j->rta_type == 1` refers to `IFLA_INET_CONF` (value 1 in the kernel's `ifla_inet_attr` enum). Defining a named constant alongside the `IPV4_DEVCONF_*` enum already introduced in this PR would make the intent self-documenting.

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[pguibert6WIND]

done

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.