Napalm LLDP port names are splitted by dots (.)

[m2martin]

NetBox version

v3.0.1

Python version

3.9

Steps to Reproduce

1. Create device interfaces with names containing dots (e.g. port2.5.3)
2. Fetch LLDP information via Napalm for such interfaces

Expected Behavior

LLDP neighbor information is displayed in the corresponding interface row.

Observed Behavior

LLDP neighbor information is not displayed in the interface row.

[m2martin]

Despite this is issue is 100% reproducible, it is very hard to setup a functional Netbox/Napalm setup with these interface names if you don't have this kind of hardware. Because of that, I already investigated the cause for this issue.

Interface names returned by Napalm are splitted by dots (.).

netbox/netbox/project-static/src/device/lldp.ts

Line 20 in 27c0e6d

const [iface] = fullIface.split('.');

For port2.5.3 this results in iface = ["port2","5","3"] which will make it impossible for document.getElementById(iface) to find the correct element.

netbox/netbox/project-static/src/device/lldp.ts

Line 22 in 27c0e6d

const row = document.getElementById(iface) as Nullable<HTMLTableRowElement>;

We need to review the reason for split(".").
A Slack discussion led to the point that it may be a cleanup for sub-interfaces which are seperated from the main interface by dots.
As LLDP is a link-layer protocol which does not run on sub-interfaces, these sub-interfaces should never be returned by an LLDP agent.

Ergo, there are two questions:

• Are sub-interfaces the reason for integrating the split-function?
• If yes, is Netbox responsible for cleaning up invalid Napalm data?

Btw. Before someone asks why dots inside interface names are necessary: I work a lot with Allied Telesis gear running Allied Ware+ where interfaces are named port<chassis>.<slot>.<port>

[jeremystretch]

Are sub-interfaces the reason for integrating the split-function?

Yes.

If yes, is Netbox responsible for cleaning up invalid Napalm data?

No.

The NAPALM-backed tools provided by NetBox are offered for convenience only. They do not and cannot feasibly accommodate every potential scenario. This particular naming format is so niche and unwieldy that I don't expect the effort required to support it would be worthwhile.

[m2martin]

The NAPALM-backed tools provided by NetBox are offered for convenience only. They do not and cannot feasibly accommodate every potential scenario.

Fully agree.

This particular naming format is so niche and unwieldy that I don't expect the effort required to support it would be worthwhile.

Niche or not - Why should a 'cleanup helper' in one direction influence the other direction negatively, when as you said, its NAPALM's responsibility to provide valid data?

[jeremystretch]

I don't understand your question. NetBox treats dots in the interface name as a separator for subinterfaces. My point is that modifying the logic solely to support this specific use case would be unduly burdensome.

[m2martin]

I fully understand your point in terms of support. The main question is why does Netbox have to support sub-interface separation.
As per my understanding this is solely to map invalid Napalm interface names to valid Netbox interface names (no matter the reason). If I'm wrong please correct me.

Please don't get me wrong. I don't want you to support dots under all circumstances. I just want to understand why the use of dots is prohibited for the benefit of the separator.

If an interface name contains dots, then it contains dots.
If an interface name contains dots because its for example an VLAN sub-interface and has to be mapped to the main interface in Netbox, then it's definitly the wrong way. In that case NAPALM should provide valid interface names instead of forcing Netbox to split them.

Background: LLDP is Link-layer, so any VLAN-sub-interface cannot be LLDP-enabled.

For example on Linux:
eth0 can be LLDP-enabled, VLAN-sub-interface eth0.200 cannot. So there is no need for Netbox to strip eth0.200 down to eth0.

As said, I'm not sure about the reason for the split. But if its because of my example above, it's wrong to support it in my opinion.

[jeremystretch]

The main question is why does Netbox have to support sub-interface separation.

Probably because when the original code was written four years ago, it was deemed necessary to do so. Likely because some platforms/drivers do report as subinterfaces. Removing this logic to fit your use case will probably break someone else's use case.

[m2martin]

Yes, I understand that. What do you think about a way which preserves the old behavior but also fulfills other needs?

e.g. in lldp.ts: after split()

if (row === null) {
    row = document.getElementById(fullIface) as Nullable<HTMLTableRowElement>;
}

This would give precedence to the existing methodology but also fail back to 'full match'. It would also be neccessary in the lldp.js.

But...this is not applicable to the LLDP neighbor interfaces returned by NAPALM. The split() happens on both local and neighbor interfaces :(
I think this is a matter of sink or swim.

[thatmattlove]

The exact implementation of your solution would be something along the lines of:

let row = document.getElementById(iface) as Nullable<HTMLTableRowElement>;

if (row === null) {
    row = document.getElementById(fullIface) as Nullable<HTMLTableRowElement>;
}

if (row !== null) {
    // Current logic
}

I'm not opposed to this approach, I agree that it provides a fallback mechanism for non-subinterfaces interfaces that do contain a . character, without introducing a breaking change.

But...this is not applicable to the LLDP neighbor interfaces returned by NAPALM. The split() happens on both local and neighbor interfaces :(

I think this could be achieved by abstracting the logic to a separate function to get the element by interface name. For example:

function getInterfaceRow(iface: string): HTMLTableCellElement | null {
    const [first] = iface.split('.');
    let row = document.getElementById(first);
    if (row === null) {
        row = document.getElementById(iface);
    }
    if (row !== null) {
        return row;
    }
    return null;
}

If everyone's good with this approach, I can implement and test it.

[m2martin]

Your proposal would resolve the problem finding the correct interface row the NAPALM result is applied to. But it would not resolve the second one (neighbor information).

netbox/netbox/project-static/src/device/lldp.ts

Lines 37 to 40 in 2a293d5

	const nHost = neighbor.remote_system_name ?? '';
	const nPort = neighbor.remote_port ?? '';
	const [nDevice] = nHost.split('.');
	const [nInterface] = nPort.split('.');

This code will also strip neighbor port port2.5.3 down to port2 and for example neighbor chassis ID (as MAC address) xxxx.yyyy.zzzz down to xxxx.

Before we think about how to mitigate this, let me summarize the facts:

• The current mechanism is to strip full interface names (including subinterfaces) down to parent interface names. e.g.:
  • Gi1/0/5.200 to Gi1/0/5
  • eth0.200 to eth0
• No matter if it is correct or not (based on the facts how LLDP is defined to run on), the current behavior should continue to work to avoid breaking any implementation

Would it be OK to base the decision on the occurance count of char .? On subinterfaces, the . count is 1.
This could be done just by replacing split('.') with a ternary.

function updateRowStyle(data: LLDPNeighborDetail) {
  for (const [fullIface, neighbors] of Object.entries(data.get_lldp_neighbors_detail)) {

    // only split if interface name is splittable into two parts separated by '.'
    const [iface] = fullIface.split('.').length === 2 ? fullIface.split('.')[0] : fullIface;

    const row = document.getElementById(iface) as Nullable<HTMLTableRowElement>;

    if (row !== null) {
      for (const neighbor of neighbors) {
      
        // other code

        const nHost = neighbor.remote_system_name ?? '';
        const nPort = neighbor.remote_port ?? '';
        // only split if name is splittable into two parts separated by '.'
        const [nDevice] = nHost.split('.').length === 2 ? nHost.split('.')[0] : nHost;
        const [nInterface] = nPort.split('.').length === 2 ? nPort.split('.')[0] : nPort;

        // more code
      }
    }
  }
}

[m2martin]

@thatmattlove @jeremystretch I want to kindly ask how we can bring this topic forward.
If my last proposal is acceptable I can open a PR for this. I have it already running in my dev-env.
In case anyone has any better solution or proposal, I'd be happy to implement it.

[jeremystretch]

Honestly, I think it's reasonable to draw the line at not supporting interface names which use dots as separators. I understand this doesn't satisfy your use case, however what you're proposing is a considerable amount of extra logic to maintain for a very niche requirement.

[m2martin]

I expected this answer and I also can understand it. Currently, for me it's not a must to have it.

But....Is there any way for you to reach out to users to "poll" how much are having LLDP interfaces with dot chars?
I'm simply interested if the root cause we're discussing about is worth it - are there LLDP dot-interfaces in the wild? (Because there shouldn't)

I'm also interested in improving Netbox and this (unfortunately) includes discussions about past implementations, if they were right or wrong. You're already quite certain about my opinion.

[DanSheps]

Users are encouraged to vote on items that are tagged as "under review"

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

[jeremystretch]

Marking this as blocked by #8152. We may be able to come up with a solution if the evaluation logic is moved inside the UI view.

[jeremystretch]

Closing this out, to be revisited once NetBox's NAPALM proxy functionality has been moved to a plugin per #10520.