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

Update NXOS show-ip-route for more scenarios #138

Merged
merged 4 commits into from
Oct 16, 2017
Merged

Update NXOS show-ip-route for more scenarios #138

merged 4 commits into from
Oct 16, 2017

Conversation

jamiecaesar
Copy link
Contributor

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT

cisco_nxos_show_ip_route.template, cisco_nxos, show ip route

SUMMARY
  • Modified values to match the IOS show ip route template, so that all the same information is captured in values of the same name and in the same order. Any additional values (from VXLAN entries, etc) come after the values common to both templates. That way the same code can extract IOS or NXOS routes, as long as it is only accessing the value common to both device types. The code would only have to account for value being different for IOS and NXOS ("O" or "ospf-"). Additional details of changes made as a part of this:

    • Protocol is now being captured in its own value - previously was picked up under "type"
    • Uptime is now being captured as its own value - previously was ignored.
    • Tag is now being captured as its own value - previously was picked up under "type"
  • Interfaces with a single digit (Lo0, Lo1, etc) were not detected, but multiple digits (e.g. Lo10, which was the test case) were. Changed the regex to correct for this and other uncommon interface names.

  • VXLAN routes will now be matched (lots of extra fields parsed into separate values)

  • When IP% was found, was being recorded as the interface, instead of the VRF name. This format is most often seen when using VXLAN. The routes in the overlay reference the underlay nexthop.

  • Added rules to cover cases where certain values were missing (some don't have a nexthop IP, some don't have a nexthop interface, some don't have a type, etc.


@itdependsnetworks
Copy link
Contributor

Why the additional capture group(%${NEXTHOP_VRF})?
Can you add an example in comments for each one?

@jamiecaesar
Copy link
Contributor Author

In the legend section of a "show ip route" command, the following message is printed:

'%<string>' in via output denotes VRF <string>

I most commonly see this show up when VXLAN is being used. In the overlay route table, it will specify the next-hop that lives in the underlay VRF (default in this case). This one test case below demonstrates that, as well as all the other new capture values that I've added:

10.151.0.0/16, ubest/mbest: 1/0
    *via 172.24.0.240%default, [150/51712], 3d17h, bgp-65004, internal, tag 65004 (evpn) segid: 10000 tunnelid: 0xac1800f0 encap: VXLAN

And while I didn't have this in the test cases, I've seen other examples of the NEXTHOP_VRF showing up, such as in ACI route tables, where you might seen something like:

10.3.2.0/24, ubest/mbest: 1/0, attached, direct, pervasive
    *via 10.0.112.64%overlay-1, [1/0], 02:54:48, static

I've also occasionally seen a route with the %management tag on it, but I can't find any examples handy where I could paste in a real output.

But basically, I assume it is possible for that VRF designator to show up for any protocol, so that is why it is an optional capture for any route statement with a next-hop IP in it. I don't have real-world examples of it showing up in every case, but I don't think it hurts anything for it to be there just in case a particular configuration I haven't seen causes a route to be tagged with the VRF.

@jamiecaesar
Copy link
Contributor Author

@itdependsnetworks Sorry -- lost track of this for a while. Did you want me to put the explanation here in comments, like I did above, or were you asking me to put a comment about the VRF in the template file itself?

@itdependsnetworks
Copy link
Contributor

Apologies for taking so long in getting back. Can you change (\w+) to (\S+), specifically in case of vrf my-blue would trip it up. Also, i'm generally weary about using type, how would route_type?

@jamiecaesar
Copy link
Contributor Author

I'm pushing the change for the \w -> \S. I don't have any strong preference on the "TYPE" name, but I was trying to keep all the common fields between IOS and NXOS route templates exact the same, and in the same order. The reason is I wanted those 2 templates to be interchangeable with each other for any code that processes the route tables on IOS or NXOS.

It is your repository, so I'll change it however you'd like. Just let me know if you'd prefer to 1) change neither, 2) change just this one, or 3) change both NXOS and IOS to have the same updated "TYPE" name.

@itdependsnetworks
Copy link
Contributor

ok, fair enough about type.

Do you think any of the other \w's should be \S?

@jamiecaesar
Copy link
Contributor Author

jamiecaesar commented Oct 16, 2017

I don't think so. I don't remember all the nitty-gritty details since it has been a few months, but I know I had some issues finding a balance between how broad/strict to make the matches. You'll notice how many different lines are in the template, because each is a mix of the same values, but with various ones missing in each case. If I made them too broad, then the wrong template line would match and get the wrong data in the different fields (since it is just comma separated with no identifier). This is a prime example of where NXAPI/structured data would be a lot easier, but I often have to work off these CLI outputs. :)

Also, I (and a few on my team) have been using this template for months at various customers and have yet to notice any problems with it being too strict. By no means will I claim it is perfect, but I feel pretty good about the accuracy.

@itdependsnetworks itdependsnetworks merged commit ba4c0f5 into networktocode:master Oct 16, 2017
@itdependsnetworks
Copy link
Contributor

Thanks @jamiecaesar and apologies for the wait

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

Successfully merging this pull request may close these issues.

2 participants