Various Network Map Improvements #3602

Merged
merged 1 commit into from Jun 9, 2016

Projects

None yet

3 participants

@keeperofdakeys
Contributor

This patch extends some of my thoughts from #3573. I feel the changes are mostly positive, but I've only got a small network to test on (and no xpd devices). So I'd welcome any feedback.

Currently the Network Map will draw multiple links between devices and ports, becoming overlapped on the graph. The choice of which links to show is also not specified. This patch fixes the first issue by only allowing a single link between any two devices, and it fixes the second issue by adding ORDER BY clauses to the sql statements (for MAC links, prioritising ports with a matching ip address).

@keeperofdakeys keeperofdakeys Improvements for the link selection for the Network Map
Currently the Network Map will draw multiple links between
devices and ports, overlapping themselves on the graph. The
criteria used to select the link is also unspecified.

Instead only allow a single link between any two devices/ports.
Also prefer links with shorter names, and for links derived
using arp/mac, prioritise links with a matching ip address.
72cebe4
@Rosiak
Contributor
Rosiak commented Jun 8, 2016

I like the idea, however I think we should indicate if more links are going to one device?
As I understand this will not deal with that?

@keeperofdakeys
Contributor

Currently multiple links are drawn over each-other, so in terms of that this isn't removing any features. Does the graphing library support multiple edges between nodes, or perhaps there would be a way to annotate the number of links in the edge description? I'll investigate these over the next week.

There is also the problem with virtual interfaces that I brought up in #3573. This could be fixed by removing interfaces that have the same MAC address (I'm not sure if this will cause issues with other networking devices though).

@laf
Member
laf commented Jun 9, 2016

We'd normally need a contributors agreement but as I know the source of the code in this file was from me then I'm ok with that but if you can please address this for any further commits that would be great.

@laf laf merged commit 69f37b3 into librenms:master Jun 9, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment