napalm-automation / napalm

Network Automation and Programmability Abstraction Layer with Multivendor support
Apache License 2.0
2.26k stars 554 forks source link

IOSXR driver LLDP output invalid with spaces in names #1022

Open emr-arvig opened 5 years ago

emr-arvig commented 5 years ago

Description of Issue/Question

Napalm IOSXRDriver method get_lldp_neighbors() is having a bit of trouble parsing LLDP in our environment when there are spaces in either the local interface or the remote port description. When there are spaces in the Device ID, they key is incorrect. When there are spaces in the remote port ID name we get incomplete information.

Did you follow the steps from https://github.com/napalm-automation/napalm#faq

(Place an x between the square brackets where applicable)

Setup

napalm version

napalm==2.4.0

Network operating system version

Cisco IOS XR Software, Version 6.1.4[Default]
Copyright (c) 2017 by Cisco Systems, Inc.

ROM: System Bootstrap, Version 10.57(c) 1994-2014 by Cisco Systems,  Inc.

511-ASR9010-Core1 uptime is 1 year, 12 weeks, 1 day, 11 hours, 11 minutes
System image file is "disk0:asr9k-os-mbi-6.1.4/0x100305/mbiasr9k-rsp3.vm"

Steps to Reproduce the Issue

Sample pseudo-router output

Capability codes:
        (R) Router, (B) Bridge, (T) Telephone, (C) DOCSIS Cable Device
        (W) WLAN Access Point, (P) Repeater, (S) Station, (O) Other

Device ID       Local Intf          Hold-time  Capability     Port ID
test1-device-no-spaces-1.domain.net          Te0/0/0/0            120       R          Te0/0/2/1
test2-device-no-spaces-2.net                 Te0/0/0/2            120       R          BE51
test3 device with space 1.domain.net         Gi200/0/0/0          40        B          1/g1
test4 device with space 2.domain.net         Gi200/0/0/11         40        B          1/g1
device5.domain.net                           Gi200/0/0/16         121       N/A        REMOTE-DEVICE-PORT-HAS-NO-SPACES
device6.domain.net                           Gi200/0/0/17         121       N/A        REMOTE DEVICE PORT HAS SPACES
test7 device with spaces.domain.net          Gi200/0/0/18         121       N/A        REMOTE-DEVICE-PORT-HAS-NO-SPACES
test8 device with spaces.domain.net          Gi200/0/0/19         121       N/A        REMOTE DEVICE PORT HAS SPACES

json output from napalm:

{
    "device": [
        {
            "hostname": "test3",
            "port": "1.domain.net"
        },
        {
            "hostname": "test4",
            "port": "2.domain.net"
        },
        {
            "hostname": "test7",
            "port": "Gi200/0/0/18"
        },
        {
            "hostname": "test8",
            "port": "Gi200/0/0/19"
        }
    ],
    "Te0/0/0/2": [
        {
            "hostname": "test2-device-no-spaces-2.net",
            "port": "BE51"
        }
    ],
    "Te0/0/0/0": [
        {
            "hostname": "test1-device-no-spaces-1.domain.net",
            "port": "Te0/0/2/1"
        }
    ],
    "Gi200/0/0/16": [
        {
            "hostname": "device5.domain.net",
            "port": "REMOTE-DEVICE-PORT-HAS-NO-SPACES"
        }
    ],
    "Gi200/0/0/17": [
        {
            "hostname": "device6.domain.net",
            "port": "REMOTE"
        }
    ]
}

Problem

The problem comes from using split() since obviously that is going to split the string on any spaces we find, so when you access split()[1] a lot of the time you are grabbing part of the interface description (Device ID) instead of the actual interface name in column 2 of the LLDP output.

Potential solution(s)

I did a quick and dirty hack by using re.split() on 782, 788 and 789 instead of split()[i]:

re.split('\s\s+', n)

This isn't the best way to do it since again if there is more than one space it will also fail.

I hate adding regex as much as the next person but this was what was the most reliable for me in testing:

for n in sh_lldp:
    data = re.match('(^.+?\s+)((Gi\d{0,3}|Te\d|Hu\d)(\/[0-9]{0,4})+)\s+\d{2,3}\s+.{1,3}\s+(.+)', n)
    if not data:
        continue
    data_groups = data.groups()
    hostname = data_groups[0].strip()
    local_interface = data_groups[1].strip()
    port = data_groups[4].strip()
    if local_interface not in lldp.keys():
        lldp[local_interface] = []
    lldp[local_interface].append(
        {
            "hostname": hostname,
            "port": port,
        }
    )

json output:

{
    "Gi200/0/0/16": [
        {
            "hostname": "device5.domain.net",
            "port": "REMOTE-DEVICE-PORT-HAS-NO-SPACES"
        }
    ],
    "Gi200/0/0/17": [
        {
            "hostname": "device6.domain.net",
            "port": "REMOTE DEVICE PORT HAS SPACES"
        }
    ],
    "Gi200/0/0/11": [
        {
            "hostname": "test4 device with space 2.domain.net",
            "port": "1/g1"
        }
    ],
    "Te0/0/0/2": [
        {
            "hostname": "test2-device-no-spaces-2.net",
            "port": "BE51"
        }
    ],
    "Te0/0/0/0": [
        {
            "hostname": "test1-device-no-spaces-1.domain.net",
            "port": "Te0/0/2/1"
        }
    ],
    "Gi200/0/0/18": [
        {
            "hostname": "test7 device with spaces.domain.net",
            "port": "REMOTE-DEVICE-PORT-HAS-NO-SPACES"
        }
    ],
    "Gi200/0/0/19": [
        {
            "hostname": "test8 device with spaces.domain.net",
            "port": "REMOTE DEVICE PORT HAS SPACES"
        }
    ],
    "Gi200/0/0/0": [
        {
            "hostname": "test3 device with space 1.domain.net",
            "port": "1/g1"
        }
    ]
}

There may be a better more pythonic way of doing this - I am by no means a python or regex expert but this is what we are using for now as a shim. I have tested this using the output from a handful of IOSXR routers we have and it has applied successfully to all of them.

The reason we needed to do this was essentially to get Netbox up and running and we were unable to use the LLDP connection methods without either sorting this out in Napalm or running through and fixing all the local and remote ports in our network (and, being an ISP, we only have access to a limited number of remote devices to make changes anyway).

Thank you!

Error Traceback

N/A
mirceaulinic commented 5 years ago

Hey @emr-arvig. I personally can't really think of a good reason to have spaces in the host name, but either way, the fix you suggested sounds good so please feel free to open a PR. Thanks!

ktbyers commented 5 years ago

So how do we know when the LLDP device ID field is done?

It looks like the regex is counting on enumerating the local interface names--that sounds like a non-viable solution to me. Interface name enumeration we will almost certainly get wrong and it will be inherently fragile.

On cisco IOS the device ID field is a fixed 20-character field. Is it a fixed field on IOS-XR?

What if you make a 100 character device-name on a remote device? Does the "Device ID" end at 45 characters? Does it always leave a space between "Device ID" and "Local Intf" (i.e. is the field width 44 characters or 45 characters).

Regards, Kirk

emr-arvig commented 5 years ago

@ktbyers

I did a bit of testing and it does appear that the Device ID column is restricted to 40 characters. You can see our domain getting truncated here.

RP/0/RSP0/CPU0:TRANSPORT-CISCO-ASR9010-TEST-1#sh lldp neighbors | inc BORDER   
Mon Sep 16 14:31:49.096 UTC
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/3/0/4            120       R          Te0/0/0/1           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/3/0/5            120       R          Te0/0/1/3           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/3/0/9            120       R          Te0/0/1/1           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/3/0/9            120       R          BE11                
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/3/0/10           120       R          Te0/2/0/2           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Hu0/3/1/1            120       R          BE91                
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Hu0/3/1/1            120       R          Hu0/1/0/1           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/3/4            120       R          Te0/1/0/2           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/2/4            120       R          BE10                
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/0/5            120       R          Te0/5/0/3           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/0/9            120       R          Te0/5/1/0           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/0/10           120       R          Te0/7/0/0           
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Te0/5/1/10           120       R          BE1                 
TRANSPORT-CISCO-ASR9010-TEST-BORDER-1.ar  Hu0/5/1/1            120       R          Hu0/4/1/1

It also looks like it will always be 84 characters to the beginning of the Port ID column.

Looks like I could scrap the regex entirely then:

def parse_lldp_output(show_command_data):
    sh_lldp = show_command_data.splitlines()[5:-3]

    lldp = {}

    for line in sh_lldp:
        local_interface = line[40:].split()[0]
        if local_interface not in lldp.keys():
            lldp[local_interface] = []
        hostname = line[:40].strip()
        port = line[84:].strip()
        print(local_interface, hostname, port)
        lldp[local_interface].append(
            {
                "hostname": hostname,
                "port": port,
            }
        )

    return lldp

Output:

{
"Gi100/0/0/44": [
    {
    "hostname": "SUPER LONG ROUTER NAME OVER 40 CHARS TRU",
    "port": "SPACES HERE TOO BECAUSE WHY NOT"
    }
],
"Te0/0/0/0": [
    {
    "hostname": "test1-device-no-spaces-1.domain.net",
    "port": "Te0/0/2/1"
    }
],
"Te0/0/0/2": [
    {
    "hostname": "test2-device-no-spaces-2.net",
    "port": "BE51"
    }
],
"Gi200/0/0/0": [
    {
    "hostname": "test3 device with space 1.domain.net",
    "port": "1/g1"
    }
],
"Gi200/0/0/11": [
    {
    "hostname": "test4 device with space 2.domain.net",
    "port": "1/g1"
    }
],
"Gi200/0/0/16": [
    {
    "hostname": "device5.domain.net",
    "port": "REMOTE-DEVICE-PORT-HAS-NO-SPACES"
    }
],
"Gi200/0/0/17": [
    {
    "hostname": "device6.domain.net",
    "port": "REMOTE DEVICE PORT HAS SPACES"
    }
],
"Gi200/0/0/18": [
    {
    "hostname": "test7 device with spaces.domain.net",
    "port": "REMOTE-DEVICE-PORT-HAS-NO-SPACES"
    }
]
}

Thanks! Evan

TheRealBecks commented 5 years ago

I have to be pedandic... hostnames and FQDNs have the valid characters a-z0-9- and the dot (.) for classifying the domain regions. There's no space allowed. Nevertheless lots of vendors allow other characters than that, so device names are not equal the DNS entries anymore. What I want to say is: I would prefer you change the hostnames to 'correct' ones, so no workarounds are needed in the source code. Some kind of strict upbringing from my side...

Nevertheless if a vendor decides to write interface names with spaces that should be captured by the napalm project :thumbsup:

emr-arvig commented 5 years ago

I have to be pedandic... hostnames and FQDNs have the valid characters a-z0-9- and the dot (.) for classifying the domain regions. There's no space allowed.

Nevertheless if a vendor decides to write interface names with spaces that should be captured by the napalm project 👍

I will say that I completely agree. We have used napalm to go through and edit everything we have control of to ensure hostnames are more or less aligned with RFC1034. The problem, of course, are when we need to be able to discover customer devices, and many folks out there are just not as concerned.

What next? I'd like a green light on whether or not this is a viable solution before I open a PR, just to avoid adding unnecessary static.

TheRealBecks commented 4 years ago

Yeah, maybe you're right, as a IT guy things often have simply to work, even if that strange implementation cannot be changed. Maybe I was to harsh? @mirceaulinic What do you say to this case?

mirceaulinic commented 3 years ago

Sorry I dropped the ball here. Sure @emr-arvig feel free to open a PR for the solution you suggested. :+1: