Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IOS XE Traceroute ttl #1320

Closed
1 task done
MatthiasGabriel opened this issue Nov 12, 2020 · 13 comments
Closed
1 task done

IOS XE Traceroute ttl #1320

MatthiasGabriel opened this issue Nov 12, 2020 · 13 comments

Comments

@MatthiasGabriel
Copy link
Contributor

Description of Issue/Question

The command string created when calling traceroute on IOS device leads to an incomplete command and to an empy success in the result.

The problem is that the generated command in the IOS driver is missing the minimum ttl.
The output from the device is then '^% Invalid input detected at \'^\' marker.

I noticed that the minimum ttl got removed in the pr #1174. Does IOS XE implement different traceroute commands on different versions?

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

(Place an x between the square brackets where applicable)

  • Yes
  • [] No

Setup

napalm version

(Paste verbatim output from pip freeze | grep napalm between quotes below)

napalm==3.2.0

Network operating system version

(Paste verbatim output from show version - or equivalent - between quotes below)

Cisco IOS XE Software, Version 16.11.01a

Steps to Reproduce the Issue

  1. Use IOS driver to create a new device connection and call traceroute.
driver = napalm.get_network_driver("ios")
    device = driver(hostname="10.20.0.31", username=<user>, password=<pw>)
    with device as napalm_device:
        napalm_result = napalm_device.traceroute(destination=<reachable_destination>)
  1. Analyse the command and output variables in the ios driver.
  2. Analyse the command variable on the device.
    image
    image

Error Traceback

(Paste the complete traceback of the exception between quotes below)


@MatthiasGabriel
Copy link
Contributor Author

MatthiasGabriel commented Nov 12, 2020

During the analysis of this problem I also noticed that there is no possiblity to skip the ttl parameter.
If one calls the method without the ttl parameter, the default value of 255 is taken.
If one calls the method with ttl parameter set to None and therefore the ttl part is not added to the command string, the calculation of the max_loops variable fails.
Maybe for the calculation another default value should be used. (It would be good to use the default ttl of IOS devices which according to a little googling is 30.)
Let me know what you think of this and I'll add it to my pr.

@MatthiasGabriel
Copy link
Contributor Author

MatthiasGabriel commented Nov 12, 2020

Maybe the minimum ttl should be set to 1 instead of 0 as this seems to be the default value
image

@MatthiasGabriel
Copy link
Contributor Author

Hi,
We checked this on different IOS Devices, and either the ttl option does not exist at all or it requires both min and max ttl.

with_ttl_17_3
with_ttl_16_6

without_ttl_12_2
without_ttl_15_0
without_ttl_15_1

@MatthiasGabriel
Copy link
Contributor Author

MatthiasGabriel commented Nov 13, 2020

During the analysis I discovered another small issue with the traceroute.

If we want to skip the ttl parameter of the command string ttl in my opinion the logical input value for ttl would be None.
However, if we set ttl to None, the calculation of max_loops fails, because 5*None raises an exception.
The only possibility would be to set ttl to 0. But in this case the max_loop is (normally) to low for a response.

I checked the other drivers and eos and iosxr seems to have similiar issues.

What do you think about this?
Should I open another issue/pr or include it in the current one as it is somehow related? I already implemented a possible fix napalm fork?

@mirceaulinic
Copy link
Member

However, if we set ttl to None, the calculation of max_loops fails, because 5*None raises an exception.

I'm not sure I'm following, where does 5*<something> come from? The default TTL is 255: https://github.com/napalm-automation/napalm/blob/develop/napalm/base/constants.py#L45.

So if you want to use a lower value than 255, pass in the argument, otherwise this is that value it should use.

Now, if the syntax should be traceroute <destination> ttl 0 <max_ttl> or traceroute <destination> ttl <max_ttl> I wouldn't know for sure. Have you check in with the community what other people are seeing on their IOS / IOS-XE versions?

@MatthiasGabriel
Copy link
Contributor Author

Sorry for the confusion. I'll try to be a little bit more consise.
Imagine that you do not want to specify the ttl in the command that is sent to the device(e.g. for devices that do not support ttl, or just to get the default value of the device itself)

In my opinion the best way to do that would be to pass None into the traceroute function.
device.traceroute("8.8.8.8", ttl=None)

In the most drivers this method is even (partially) implemented by the if ttl: case.
However some drivers also use the ttl to calculate a bunch of other things such as the max_loops variable.
max_loops = (5 * ttl * timeout) + 150

@MatthiasGabriel
Copy link
Contributor Author

@mirceaulinic Is there anything I can do to push this forward?

@mirceaulinic
Copy link
Member

mirceaulinic commented Jan 11, 2021

Hi @MatthiasGabriel. I think the behaviour shouldn't change from one driver to another because of the platform differences. In particular:

for devices that do not support ttl

Then we just ignore the ttl argument.
In that case, for declarations like max_loops = (5 * ttl * timeout) + 150, ttl defaults to 255 as defined in the NAPALM constants, and there shouldn't be any issue with that.

just to get the default value of the device itself

That would break the consistency of the NAPALM API.

Back to the core issue, regarding IOS-XE devices that don't support the TTL argument, should identify which are those particular platforms or versions and nest this if ttl under an if statement that checks whether the platform or version supports the TTL argument (i.e., if the platform doesn't support, won't add the ttl bit to the traceroute command). Let me know if you have more questions.

@MatthiasGabriel
Copy link
Contributor Author

Hi @mirceaulinic thanks for your response.

The core issue is that if IOS allows to specify a TTL, it requires a min ttl as shown in the screenshots.
The min ttl was set to 0 until PR #1174.

Regarding the consistency of the NAPALM API, some drivers(e.g. junos, nxos) already support ttl=None, while others run into exceptional behaviour due to the calculations.
I suggest to open another issue as it is only related.

@mirceaulinic
Copy link
Member

The core issue is that if IOS allows to specify a TTL, it requires a min ttl as shown in the screenshots.
The min ttl was set to 0 until PR #1174.

I see, wasn't aware some platforms require a minimum TTL. We should probably just revert #1174 as it seems to rather be based on feelings than facts. Thoughts?

@MatthiasGabriel
Copy link
Contributor Author

I think the fix which renamed timeout to ttl was okay, however the second change was not.
https://github.com/napalm-automation/napalm/pull/1174/files#diff-513ca510c8962153b7b69d75d628fe85c3956d74a4ad863f0220a723ccff0905R3195

mirceaulinic added a commit that referenced this issue Jan 14, 2021
…um_ttl

Set minimum ttl to zero to fix issue #1320
@mirceaulinic
Copy link
Member

As #1321 is now merged, was there anything else you wanted to discuss here @MatthiasGabriel ?

@MatthiasGabriel
Copy link
Contributor Author

Hi @mirceaulinic
Thanks for merging, I'll open another issue next week regarding the ttl behaviour.
But this (core issue) is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants