Closed kbirkeland closed 7 years ago
@kbirkela How long is a long time (roughly)?
About 30 seconds for 48 ports, but it scales linearly with number of interfaces.
salt 1-switch-stack net.interfaces 1.43s user 0.37s system 5% cpu 35.351 total
salt 5-switch-stack net.interfaces 3.47s user 0.52s system 2% cpu 2:49.12 total
salt 9-switch-stack net.interfaces 5.83s user 0.66s system 2% cpu 4:59.39 total
Executing one single request rather than one request per interface is an optimization we may want to have. I'm not referring here to TextFSM/non-TextFSM, but to the concept itself. As an aside, we had a similar discussion under https://github.com/napalm-automation/napalm-eos/pull/28 when the user proposed a method that, in a very similar way, it was executing a separate device for each and every BGP neighbor, so we asked to refactor in such a way that it reduced dramatically the runtime.
Yes, I think we should do this. I am still contemplating whether to do this using TextFSM or to just do show interfaces
and then split on the individual interfaces in Python.
Yes, I think we should do this. I am still contemplating whether to do this using TextFSM or to just do show interfaces and then split on the individual interfaces in Python.
Doing that is very easy and would allow us to reuse the code we already have which doesn't make use of textfsm:
We should avoid using textfsm as any regular expression we build here will be able to be reused later on in napalm-yang
.
Sounds good. This will need re-implemented in Python + Regular Expressions.
roughly speaking this should work. I can put in an PR, but figured you would probably want to make it a bit more "napalm-like".
interface = is_enabled = is_up = description = mac_address = speed = speedformat = ''
last_flapped = -1
interfaces = {}
for line in output.splitlines():
interface_regex = r"^(\S+?) is (\S+), line protocol is (\S+)"
if re.search(interface_regex, line):
interface_match = re.search(interface_regex, line)
interface = interface_match.groups()[0]
is_enabled = interface_match.groups()[0]
is_up = interface_match.groups()[0]
#ip_regex = r"^\s+Internet address is ({}/\d+)".format(IP_ADDR_REGEX)
#if re.search(ip_regex, line):
# ip_match = re.search(ip_regex, line)
# ip_address = ip_match.groups()[0]
mac_addr_regex = r"^\s+Hardware.+address is ({})".format(MAC_REGEX)
if re.search(mac_addr_regex, line):
mac_addr_match = re.search(mac_addr_regex, line)
mac_address = mac_addr_match.groups()[0]
descr_regex = "^\s+Description: (.+?)$"
if re.search(descr_regex, line):
descr_match = re.search(descr_regex, line)
description = descr_match.groups()[0]
speed_regex = r"^\s+MTU (\d+).+ BW (\d+) ([KMG]?b)"
if re.search(speed_regex, line):
speed_match = re.search(speed_regex, line)
speed = speed_match.groups()[1]
speedformat = speed_match.groups()[2]
interfaces[interface] = { 'is_enabled': is_enabled, 'is_up': is_up,
'description':description, 'mac_address': mac_address,
'last_flapped': last_flapped, 'speed': speed + speedformat }
interface = is_enabled = is_up = description = mac_address = speed = speedformat = ''```
@itdependsnetworks Yes, definitely feel free to submit a PR on it. Key item is just to convert to a single 'show interfaces' query.
Other key item for us is to have some reasonably good unit tests (i.e. at least a couple of 'show interfaces' from devices).
I can get these setup if you submit the PR on the method change.
Should be able to combine what @kbirkeland did and what I have here and put in a PR.
@itdependsnetworks I edited my PR.
It's failing on python3.5 but I'll figure that out later.
This was implemented in this PR:
get_interfaces() was taking a long time due to running
show interface <int>
for each interface, so I changed it to just use oneshow interface
and parse all the information out.Also, description was defaulting to
N/A
instead of empty string. This seemed to contradict base and the other drivers.changed get_interfaces to use textfsm changed empty description to return "" instead of "N/A"