Improve topology discovery matching

This commit is contained in:
Graham McIntire 2026-03-10 15:44:46 -05:00
parent 232915e3e4
commit b4ef6b2020
No known key found for this signature in database
5 changed files with 423 additions and 46 deletions

View file

@ -297,16 +297,21 @@ defmodule Towerops.Snmp.NeighborDiscovery do
end
defp format_chassis_id(value) when is_binary(value) do
# LLDP Chassis ID has subtype byte + value
# If it's a printable string (subtype 4 = MAC as text, 7 = local), use it directly
if String.valid?(value) and String.printable?(value) do
String.trim(value)
else
# Otherwise convert binary to hex MAC format
value
|> :binary.bin_to_list()
|> Enum.map_join(":", &String.pad_leading(Integer.to_string(&1, 16), 2, "0"))
|> String.downcase()
cond do
value == "" ->
nil
printable_binary?(value) ->
String.trim(value)
byte_size(value) == 6 ->
format_binary_mac(value)
byte_size(value) > 1 ->
parse_chassis_id_with_subtype(value)
true ->
nil
end
end
@ -323,6 +328,42 @@ defmodule Towerops.Snmp.NeighborDiscovery do
defp format_port_id(value), do: to_string_safe(value)
defp printable_binary?(value) do
String.valid?(value) and String.printable?(value)
end
defp parse_chassis_id_with_subtype(<<subtype, rest::binary>>) do
case subtype do
4 when byte_size(rest) == 6 ->
format_binary_mac(rest)
5 ->
format_ip_address(rest)
_ ->
cond do
subtype == 7 and printable_binary?(rest) ->
String.trim(rest)
printable_binary?(rest) ->
String.trim(rest)
byte_size(rest) == 6 ->
format_binary_mac(rest)
true ->
format_binary_mac(<<subtype, rest::binary>>)
end
end
end
defp format_binary_mac(value) when is_binary(value) do
value
|> :binary.bin_to_list()
|> Enum.map_join(":", &String.pad_leading(Integer.to_string(&1, 16), 2, "0"))
|> String.downcase()
end
defp format_ip_address(value) when is_binary(value) and byte_size(value) == 4 do
value
|> :binary.bin_to_list()

View file

@ -12,11 +12,13 @@ defmodule Towerops.Topology do
alias Towerops.Snmp.ArpEntry
alias Towerops.Snmp.Device, as: SnmpDevice
alias Towerops.Snmp.Interface
alias Towerops.Snmp.IpAddress
alias Towerops.Snmp.MacAddress
alias Towerops.Snmp.Neighbor
alias Towerops.Topology.DeviceLink
alias Towerops.Topology.DeviceLinkEvidence
alias Towerops.Topology.DeviceNeighbor
alias Towerops.Topology.Identifier
alias Towerops.Topology.Lldp
require Logger
@ -39,16 +41,30 @@ defmodule Towerops.Topology do
left_join: sd in assoc(d, :snmp_device),
left_join: i in Interface,
on: sd.id == i.snmp_device_id,
preload: [snmp_device: {sd, interfaces: i}]
left_join: ip in IpAddress,
on: i.id == ip.snmp_interface_id,
preload: [snmp_device: {sd, interfaces: {i, ip_addresses: ip}}]
)
)
by_ip = Map.new(devices, fn d -> {d.ip_address, d.id} end)
by_ip =
devices
|> Enum.flat_map(fn device ->
[device.ip_address | device_interface_ips(device)]
|> Enum.map(&Identifier.normalize_ip/1)
|> Enum.reject(&is_nil/1)
|> Enum.map(&{&1, device.id})
end)
|> Map.new()
by_name =
devices
|> Enum.filter(&(&1.name != nil))
|> Map.new(&{String.downcase(&1.name), &1.id})
|> Enum.flat_map(fn device ->
[device.name, device.snmp_device && device.snmp_device.sys_name]
|> Enum.flat_map(&Identifier.candidate_names/1)
|> Enum.map(&{&1, device.id})
end)
|> Map.new()
by_mac =
devices
@ -60,7 +76,9 @@ defmodule Towerops.Topology do
sd ->
sd.interfaces
|> Enum.filter(&(&1.if_phys_address != nil))
|> Enum.map(&{&1.if_phys_address, device.id})
|> Enum.map(&Identifier.normalize_mac(&1.if_phys_address))
|> Enum.reject(&is_nil/1)
|> Enum.map(&{&1, device.id})
end
end)
|> Map.new()
@ -71,19 +89,23 @@ defmodule Towerops.Topology do
@doc """
Finds a device ID by IP address from the lookup.
"""
def find_device_by_ip(lookup, ip), do: Map.get(lookup.by_ip, ip)
def find_device_by_ip(lookup, ip), do: Map.get(lookup.by_ip, Identifier.normalize_ip(ip))
@doc """
Finds a device ID by name (case-insensitive) from the lookup.
"""
def find_device_by_name(lookup, name) when is_binary(name), do: Map.get(lookup.by_name, String.downcase(name))
def find_device_by_name(lookup, name) when is_binary(name) do
name
|> Identifier.candidate_names()
|> Enum.find_value(&Map.get(lookup.by_name, &1))
end
def find_device_by_name(_lookup, _name), do: nil
@doc """
Finds a device ID by MAC address from the lookup.
"""
def find_device_by_mac(lookup, mac), do: Map.get(lookup.by_mac, mac)
def find_device_by_mac(lookup, mac), do: Map.get(lookup.by_mac, Identifier.normalize_mac(mac))
@doc """
Resolves a device ID from evidence fields, trying MAC first, then IP, then name.
@ -136,9 +158,9 @@ defmodule Towerops.Topology do
evidence_type: evidence_type,
confidence: confidence,
source_interface_id: row.source_interface_id,
remote_mac: row.remote_chassis_id,
remote_ip: row.remote_address,
remote_name: row.remote_system_name,
remote_mac: Identifier.normalize_mac(row.remote_chassis_id),
remote_ip: Identifier.normalize_ip(row.remote_address),
remote_name: Identifier.normalize_name(row.remote_system_name),
remote_port: row.remote_port_id,
remote_capabilities: row.remote_capabilities,
evidence_data: %{
@ -175,7 +197,7 @@ defmodule Towerops.Topology do
evidence_type: "mac_on_interface",
confidence: 0.7,
source_interface_id: row.interface_id,
remote_mac: row.mac_address,
remote_mac: Identifier.normalize_mac(row.mac_address),
remote_ip: nil,
remote_name: nil,
remote_port: nil,
@ -217,8 +239,8 @@ defmodule Towerops.Topology do
evidence_type: "arp_entry",
confidence: 0.6,
source_interface_id: row.interface_id,
remote_mac: row.mac_address,
remote_ip: row.ip_address,
remote_mac: Identifier.normalize_mac(row.mac_address),
remote_ip: Identifier.normalize_ip(row.ip_address),
remote_name: nil,
remote_port: nil,
remote_capabilities: nil,
@ -244,15 +266,7 @@ defmodule Towerops.Topology do
evidence_attrs = Map.get(attrs, :evidence, [])
link_attrs = Map.delete(attrs, :evidence)
existing =
Repo.one(
from(l in DeviceLink,
where:
l.source_device_id == ^link_attrs.source_device_id and
l.source_interface_id == ^link_attrs[:source_interface_id] and
l.discovered_remote_mac == ^link_attrs[:discovered_remote_mac]
)
)
existing = find_existing_link(link_attrs)
result =
case existing do
@ -477,11 +491,11 @@ defmodule Towerops.Topology do
defp build_discovered_nodes(links, "all") do
links
|> Enum.filter(&is_nil(&1.target_device_id))
|> Enum.uniq_by(& &1.discovered_remote_mac)
|> Enum.uniq_by(&discovered_node_id/1)
|> Enum.map(fn link ->
%{
id: "discovered_#{link.discovered_remote_mac}",
label: link.discovered_remote_name || link.discovered_remote_mac,
id: discovered_node_id(link),
label: link.discovered_remote_name || link.discovered_remote_ip || link.discovered_remote_mac,
type: :unknown,
device_role: nil,
status: :unknown,
@ -503,7 +517,7 @@ defmodule Towerops.Topology do
target_id =
if link.target_device_id,
do: link.target_device_id,
else: "discovered_#{link.discovered_remote_mac}"
else: discovered_node_id(link)
%{
id: link.id,
@ -536,12 +550,13 @@ defmodule Towerops.Topology do
defp upsert_grouped_evidence({_key, evidence_list}, device_id, lookup, now) do
best = Enum.max_by(evidence_list, & &1.confidence)
remote_identity = summarize_remote_identity(evidence_list)
matched_id =
resolve_device(lookup, %{
mac: best.remote_mac,
ip: best.remote_ip,
name: best.remote_name
mac: remote_identity.remote_mac,
ip: remote_identity.remote_ip,
name: remote_identity.remote_name
})
if matched_id == device_id do
@ -553,9 +568,9 @@ defmodule Towerops.Topology do
source_interface_id: best.source_interface_id,
link_type: evidence_type_to_link_type(best.evidence_type),
confidence: combine_evidence_confidence(evidence_list),
discovered_remote_mac: best.remote_mac,
discovered_remote_ip: best.remote_ip,
discovered_remote_name: best.remote_name,
discovered_remote_mac: remote_identity.remote_mac,
discovered_remote_ip: remote_identity.remote_ip,
discovered_remote_name: remote_identity.remote_name,
metadata: %{},
last_confirmed_at: now,
evidence:
@ -571,9 +586,7 @@ defmodule Towerops.Topology do
end
defp group_evidence_by_remote(evidence_list) do
Enum.group_by(evidence_list, fn ev ->
ev.remote_mac || ev.remote_ip || ev.remote_name || "unknown"
end)
Enum.group_by(evidence_list, &evidence_group_key/1)
end
defp combine_evidence_confidence(evidence_list) do
@ -659,6 +672,108 @@ defmodule Towerops.Topology do
defp platform_match?(description, platforms), do: Enum.any?(platforms, &String.contains?(description, &1))
defp device_interface_ips(%Device{snmp_device: nil}), do: []
defp device_interface_ips(%Device{snmp_device: %SnmpDevice{} = snmp_device}) do
Enum.flat_map(snmp_device.interfaces, fn interface ->
Enum.map(interface.ip_addresses, & &1.ip_address)
end)
end
defp find_existing_link(link_attrs) do
source_device_id = link_attrs.source_device_id
source_interface_id = Map.get(link_attrs, :source_interface_id)
base_query =
from(l in DeviceLink,
where: l.source_device_id == ^source_device_id and l.source_interface_id == ^source_interface_id
)
link_attrs
|> existing_link_query(base_query)
|> Repo.one()
end
defp existing_link_query(%{discovered_remote_mac: remote_mac}, base_query) when is_binary(remote_mac) do
from(l in base_query, where: l.discovered_remote_mac == ^remote_mac)
end
defp existing_link_query(%{discovered_remote_ip: remote_ip} = link_attrs, base_query) when is_binary(remote_ip) do
remote_name = Map.get(link_attrs, :discovered_remote_name)
case remote_name do
name when is_binary(name) ->
from(l in base_query,
where:
is_nil(l.discovered_remote_mac) and l.discovered_remote_ip == ^remote_ip and
l.discovered_remote_name == ^name
)
_ ->
from(l in base_query,
where: is_nil(l.discovered_remote_mac) and l.discovered_remote_ip == ^remote_ip
)
end
end
defp existing_link_query(%{discovered_remote_name: remote_name}, base_query) when is_binary(remote_name) do
from(l in base_query,
where:
is_nil(l.discovered_remote_mac) and is_nil(l.discovered_remote_ip) and
l.discovered_remote_name == ^remote_name
)
end
defp existing_link_query(%{link_type: link_type}, base_query) do
from(l in base_query,
where:
is_nil(l.discovered_remote_mac) and is_nil(l.discovered_remote_ip) and
is_nil(l.discovered_remote_name) and l.link_type == ^link_type
)
end
defp evidence_group_key(ev) do
remote_mac = Identifier.normalize_mac(ev.remote_mac)
remote_ip = Identifier.normalize_ip(ev.remote_ip)
remote_name = Identifier.normalize_name(ev.remote_name)
case remote_mac || remote_ip || remote_name do
nil ->
{"anonymous", ev.source_interface_id, ev.evidence_type, inspect(ev.evidence_data)}
_ ->
{ev.source_interface_id, remote_mac, remote_ip, remote_name}
end
end
defp summarize_remote_identity(evidence_list) do
%{
remote_mac: select_remote_field(evidence_list, :remote_mac),
remote_ip: select_remote_field(evidence_list, :remote_ip),
remote_name: select_remote_field(evidence_list, :remote_name)
}
end
defp select_remote_field(evidence_list, field) do
evidence_list
|> Enum.filter(&(is_binary(Map.get(&1, field)) and Map.get(&1, field) != ""))
|> Enum.max_by(& &1.confidence, fn -> nil end)
|> case do
nil -> nil
ev -> Map.get(ev, field)
end
end
defp discovered_node_id(link) do
suffix =
link.discovered_remote_mac ||
link.discovered_remote_ip ||
link.discovered_remote_name ||
"anonymous_#{link.id}"
"discovered_#{suffix}"
end
# --- LLDP Neighbor Discovery (via SNMP) ---
@doc """

View file

@ -0,0 +1,93 @@
defmodule Towerops.Topology.Identifier do
@moduledoc """
Identifier normalization helpers for topology discovery and matching.
"""
@spec normalize_mac(term()) :: String.t() | nil
def normalize_mac(nil), do: nil
def normalize_mac(<<a, b, c, d, e, f>>) when is_integer(a) do
[a, b, c, d, e, f]
|> Enum.map_join(":", &(&1 |> Integer.to_string(16) |> String.pad_leading(2, "0")))
|> String.downcase()
end
def normalize_mac(mac) when is_binary(mac) do
mac
|> String.trim()
|> String.downcase()
|> String.replace(~r/[^0-9a-f]/, "")
|> case do
<<a1, a2, b1, b2, c1, c2, d1, d2, e1, e2, f1, f2>> ->
"#{<<a1, a2>>}:#{<<b1, b2>>}:#{<<c1, c2>>}:#{<<d1, d2>>}:#{<<e1, e2>>}:#{<<f1, f2>>}"
_ ->
nil
end
end
def normalize_mac(_), do: nil
@spec normalize_ip(term()) :: String.t() | nil
def normalize_ip(nil), do: nil
def normalize_ip(ip) when is_binary(ip) do
ip
|> String.trim()
|> strip_ipv6_zone()
|> case do
"" ->
nil
candidate ->
case :inet.parse_address(String.to_charlist(candidate)) do
{:ok, tuple} ->
tuple
|> :inet.ntoa()
|> to_string()
|> String.downcase()
_ ->
String.downcase(candidate)
end
end
end
def normalize_ip(_), do: nil
@spec normalize_name(term()) :: String.t() | nil
def normalize_name(nil), do: nil
def normalize_name(name) when is_binary(name) do
case name |> String.trim() |> String.trim_trailing(".") |> String.downcase() do
"" -> nil
normalized -> normalized
end
end
def normalize_name(_), do: nil
@spec candidate_names(term()) :: [String.t()]
def candidate_names(name) do
case normalize_name(name) do
nil ->
[]
normalized ->
short_name =
normalized
|> String.split(".", parts: 2)
|> List.first()
[normalized, short_name]
|> Enum.reject(&is_nil/1)
|> Enum.uniq()
end
end
defp strip_ipv6_zone(ip) do
ip
|> String.split("%", parts: 2)
|> List.first()
end
end

View file

@ -97,6 +97,25 @@ defmodule Towerops.Snmp.NeighborDiscoveryTest do
assert neighbor.remote_address == "192.168.1.100"
end
test "normalizes subtype-prefixed LLDP MAC chassis IDs", %{interfaces: interfaces} do
expect(SnmpMock, :walk, fn _, _oid, _ ->
{:ok,
[
%{oid: "1.0.8802.1.1.2.1.4.1.1.5.0.1.1", value: <<4, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF>>},
%{oid: "1.0.8802.1.1.2.1.4.1.1.9.0.1.1", value: "switch-a.example.com"}
]}
end)
expect(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
expect(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
client_opts = [ip: "192.168.1.1", community: "public", version: "2c"]
{:ok, neighbors} = NeighborDiscovery.discover_neighbors(client_opts, interfaces)
[neighbor] = neighbors
assert neighbor.remote_chassis_id == "aa:bb:cc:dd:ee:ff"
end
test "discovers LLDP neighbors without management addresses", %{interfaces: interfaces} do
# First walk: LLDP remote table
expect(SnmpMock, :walk, fn _, _oid, _ ->

View file

@ -5,6 +5,7 @@ defmodule Towerops.TopologyTest do
alias Towerops.Snmp.Device
alias Towerops.Snmp.Interface
alias Towerops.Snmp.IpAddress
alias Towerops.Topology
alias Towerops.Topology.DeviceLink
@ -75,12 +76,39 @@ defmodule Towerops.TopologyTest do
assert Topology.find_device_by_ip(lookup, "10.0.0.2") == switch.id
end
test "matches device by discovered interface IP address", %{
organization: org,
router_iface: router_iface,
router: router
} do
%IpAddress{}
|> IpAddress.changeset(%{
snmp_interface_id: router_iface.id,
ip_address: "10.0.10.1",
ip_type: "ipv4",
last_checked_at: DateTime.utc_now()
})
|> Repo.insert!()
lookup = Topology.build_device_lookup(org.id)
assert Topology.find_device_by_ip(lookup, "10.0.10.1") == router.id
end
test "matches device by name (case-insensitive)", %{organization: org, router: router} do
lookup = Topology.build_device_lookup(org.id)
assert Topology.find_device_by_name(lookup, "core-router") == router.id
assert Topology.find_device_by_name(lookup, "CORE-ROUTER") == router.id
end
test "matches device by sysName short hostname", %{organization: org, router: router, router_snmp: router_snmp} do
router_snmp
|> Device.changeset(%{sys_name: "core-router.example.net"})
|> Repo.update!()
lookup = Topology.build_device_lookup(org.id)
assert Topology.find_device_by_name(lookup, "core-router") == router.id
end
test "matches device by MAC address", %{organization: org, router: router} do
lookup = Topology.build_device_lookup(org.id)
assert Topology.find_device_by_mac(lookup, "aa:bb:cc:00:11:22") == router.id
@ -508,6 +536,87 @@ defmodule Towerops.TopologyTest do
assert {:ok, :unchanged} = Topology.process_device(router, org.id)
assert Topology.list_device_links(router.id) == []
end
test "matches ARP-discovered devices by interface IPs", %{
organization: org,
router: router,
switch: switch,
router_iface: router_iface
} do
switch_snmp =
%Device{}
|> Device.changeset(%{device_id: switch.id, sys_name: "main-switch"})
|> Repo.insert!()
switch_iface =
%Interface{}
|> Interface.changeset(%{
snmp_device_id: switch_snmp.id,
if_index: 10,
if_name: "vlan10",
if_phys_address: "bb:cc:dd:00:11:22"
})
|> Repo.insert!()
%IpAddress{}
|> IpAddress.changeset(%{
snmp_interface_id: switch_iface.id,
ip_address: "10.0.10.2",
ip_type: "ipv4",
last_checked_at: DateTime.utc_now()
})
|> Repo.insert!()
{:ok, _} =
Towerops.Snmp.upsert_arp_entry(%{
device_id: router.id,
interface_id: router_iface.id,
ip_address: "10.0.10.2",
mac_address: "ff:ff:ff:00:10:02",
last_seen_at: DateTime.utc_now()
})
assert {:ok, :changed} = Topology.process_device(router, org.id)
[link] = Topology.list_device_links(router.id)
assert link.target_device_id == switch.id
assert link.discovered_remote_ip == "10.0.10.2"
end
test "keeps anonymous neighbors separate per interface", %{
router: router,
organization: org,
router_snmp: router_snmp
} do
iface_two =
%Interface{}
|> Interface.changeset(%{
snmp_device_id: router_snmp.id,
if_index: 2,
if_name: "eth1",
if_phys_address: "aa:bb:cc:00:11:23"
})
|> Repo.insert!()
{:ok, _} =
Towerops.Snmp.upsert_neighbor(%{
device_id: router.id,
interface_id: iface_two.id,
protocol: "lldp",
last_discovered_at: DateTime.utc_now()
})
{:ok, _} =
Towerops.Snmp.upsert_neighbor(%{
device_id: router.id,
interface_id: iface_two.id,
protocol: "cdp",
last_discovered_at: DateTime.utc_now()
})
assert {:ok, :changed} = Topology.process_device(router, org.id)
assert length(Topology.list_device_links(router.id)) == 2
end
end
describe "list_device_links/1" do