napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Ping module return values #162

Closed jemcek closed 6 years ago

jemcek commented 7 years ago

Description of Issue/Question

The return values from PING module on IOS is probably not correct in case of destination unreachable response.

1) result status is "success" ("error" on Junos)? 2) with 2 echo requests sent the result is {... "packet_loss": -2, "probes_sent": 0,... ...}

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

Setup

napalm-ios version

napalm-ios==0.6.2

IOS version

Cisco IOS Software, s2t54 Software (s2t54-ADVIPSERVICESK9-M), Version 15.2(1)SY1a, RELEASE SOFTWARE (fc6)

Steps to Reproduce the Issue

Using napalm_ping module, count:2, destination: 4.4.4.4

Result on IOS:

{
...
"item": "4.4.4.4",
"results": {
"success": {
"packet_loss": -2,
"probes_sent": 0,
"results": [],
"rtt_avg": 0.0,
"rtt_max": 0.0,
"rtt_min": 0.0,
"rtt_stddev": 0.0
}

Result on Junos:

{
...
"item": "4.4.4.4",
"results": {
"error": "Packet loss 100"
}
mekisiel commented 6 years ago

I've run into this as well and it appears that napalm-ios issue #203 addresses a portion of this, namely the negative packet loss value. My preference would be to have an error statement returned indicating complete packet loss (as is the case with junos) vs. returning the 'success' dict with values indicating 100% packet loss, as it is not really a 'success' or even a partial success (which should return the 'success' dict). I can submit a pull request for returning an error statement but, am not sure how important returning '0' values for the remainder of the stats are.

ktbyers commented 6 years ago

Related issue #203

mirceaulinic commented 6 years ago

Hi @jemcek - we are currently in the process of reunification, please check https://napalm-automation.net/reunification/. Going forward, we'd like to ask you to submit Pull Requests and Issues to the main repository: https://github.com/napalm-automation/napalm. It looks like https://github.com/targuan/napalm-ios/commit/e4d6504d39430e1c84f7c42b6b165dddc37de4ad solved this bug, but if this issue persists, don't hesitate to report at https://github.com/napalm-automation/napalm.

Thanks for understanding!