network.arp, network.ip_neighs and network.ip_neighs6 silently drop neighbour entries that share a MAC address; Windows has no neighbour functions at all
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
What happened?
network.arp, network.ip_neighs and network.ip_neighs6 all return a flat {mac: ip} dict, so any MAC that appears more than once in the neighbour table keeps only one entry. Which entry survives depends on kernel output order and is effectively arbitrary.
Multiple IPs per MAC is normal, not an edge case:
- IPv4: routers/firewalls with several addresses on one interface, VRRP/CARP virtual addresses, etc.
- IPv6: nearly every neighbour has at least a link-local and a global address, so
ip_neighs6drops entries for practically every host on the segment. In the reproduction below both global addresses are lost and only the link-locals are reported. - The
devfield is not represented either, so the same address learned on two interfaces (dual-homed host) also collapses to one entry.
Reproduction against a master checkout (7d6f2590d3), mocking only cmd.run:
from unittest.mock import MagicMock
import salt.modules.network as network
# One router owning two IPv4 and two IPv6 addresses, plus one host
# with global + link-local (RFC 5737/3849/7042 documentation addresses)
IP_NEIGH_OUT = """\
203.0.113.1 dev eth0 lladdr 00:00:5e:00:53:01 REACHABLE
203.0.113.9 dev eth0 lladdr 00:00:5e:00:53:01 STALE
2001:db8::1 dev eth0 lladdr 00:00:5e:00:53:01 router REACHABLE
fe80::200:5eff:fe00:5301 dev eth0 lladdr 00:00:5e:00:53:01 router REACHABLE
2001:db8::52 dev eth0 lladdr 00:00:5e:00:53:52 REACHABLE
fe80::200:5eff:fe00:5352 dev eth0 lladdr 00:00:5e:00:53:52 STALE
"""
network.__salt__ = {"cmd.run": MagicMock(return_value=IP_NEIGH_OUT)}
print(network.ip_neighs()) # 2 IPv4 entries in the table
print(network.ip_neighs6()) # 4 IPv6 entries in the table
Output:
{'00:00:5e:00:53:01': '203.0.113.9'}
{'00:00:5e:00:53:01': 'fe80::200:5eff:fe00:5301', '00:00:5e:00:53:52': 'fe80::200:5eff:fe00:5352'}
Half the IPv4 table and both global IPv6 addresses are gone. The same collapse happens in arp() itself (ret[comps[3]] = comps[1] when parsing arp -an, and the same shape in the SunOS/OpenBSD/AIX branches).
Relevant code on master:
arp(): https://github.com/saltstack/salt/blob/7d6f2590d3/salt/modules/network.py#L1091-L1124ip_neighs(): https://github.com/saltstack/salt/blob/7d6f2590d3/salt/modules/network.py#L1346-L1370ip_neighs6(): https://github.com/saltstack/salt/blob/7d6f2590d3/salt/modules/network.py#L1373-L1397
Proposal
Change the standard return shape of all three functions to a list of neighbour entry dicts, which cannot collapse and also preserves the interface and reachability state the current shape throws away:
[
{"ip": "2001:db8::1", "mac": "00:00:5e:00:53:01", "dev": "eth0", "state": "REACHABLE"},
...
]
Following the deprecation policy (a warning in place for at least two major releases, per doc/topics/development/deprecations.rst):
- 3009: add an opt-in kwarg (e.g.
expand=Truefor the new shape,expand=Falsefor the legacy dict). Callers relying on the unset default get the legacy shape plus asalt.utils.versions.warn_until("Scandium", ...)DeprecationWarning announcing the default change in 3011. - 3011: the default flips to the new shape and the warning is removed; whether the legacy opt-out survives beyond that is up to maintainer preference.
I'm happy to send the PR implementing this.
Windows has no neighbour table support at all
A related gap: win_network (which loads as network on Windows minions) has no arp, ip_neighs or ip_neighs6 at all, so network.arp on a Windows minion just reports the function as unavailable. Since this proposal already defines a new canonical output shape, it seems like the right moment to close that gap: implement the same three functions in win_network, backed by PowerShell Get-NetNeighbor (NetTCPIP module, available on every Windows release Salt supports), which exposes IPv4 and IPv6 neighbours with IPAddress/LinkLayerAddress/InterfaceAlias/State - a 1:1 mapping onto the entry shape proposed above.
For cross-platform consistency I'd give the Windows functions the same kwarg and the same default flip in 3011, even though they have no legacy users to protect - that way network.arp behaves identically everywhere at every point in the deprecation cycle. If maintainers would rather Windows start directly on the new shape (no legacy default to honour), that works too.
Type of salt install
Verified against a git checkout; applies to all install types.
Major version
Verified on master and 3006.x (identical parsing code); presumably everything in between.
What supported OS are you seeing the problem on?
Linux for ip_neighs/ip_neighs6; the arp() collapse applies to all platforms it supports.
salt --versions-report output
Found by source inspection; reproduced against master checkout 7d6f2590d3 with the script above rather than a packaged install, so a versions report is not meaningful here.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with arp(), ip_neighs(), and ip_neighs6() in salt/modules/network.py, then inspect the corresponding platform support in salt/modules/win_network.py. Read doc/topics/development/deprecations.rst before assessing the proposed compatibility transition. Done means neighbour entries no longer collapse by MAC, preserve the proposed fields, and have consistent Linux and Windows behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- powershell, python
- Domain
- networking
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100