Skip to content

Update nxos show interface #2112

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

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

Mikeg2881
Copy link
Contributor

mjbear added 5 commits April 27, 2025 18:58
Some output is formatted differently

- Singular vs plural on error(s)
- CRC is also seen as CRC/FCS
- This caused some values not to be captured
Some lines definitely have leading whitespace so swapping from
zero or more to one or more made sense.
@mjbear
Copy link
Collaborator

mjbear commented Apr 28, 2025

@Mikeg2881
I'm working on some improvements (not your fault) since there wasn't an error directive in this nxos template before your changes.

I've already found an instance where values wouldn't be captured since the formatting changed over time. 🤷‍♂️
(Edit: We don't see this unless we diff against your commit and my later commits since the giants/CRCs values weren't captured before this.)

Question: Is RX_STORM_SUPPRESSION_BYTES really in bytes?

@mjbear mjbear self-assigned this Apr 28, 2025
@mjbear mjbear added question WIP Work in Progress labels Apr 28, 2025
@Mikeg2881
Copy link
Contributor Author

Yes they are in bytes. See below. Is there anything else i need to do on this or the other one i submitted?

Ethernet1/5 is down (Link not connected)
admin state is up, Dedicated Interface
  Hardware: 10/100/1000 Ethernet, address: 9088.555f.3254 (bia 9088.555f.3254)
  MTU 1500 bytes, BW 1000000 Kbit , DLY 10 usec
  reliability 255/255, txload 1/255, rxload 1/255
  Encapsulation ARPA, medium is broadcast
  Port mode is access
  auto-duplex, auto-speed
  Beacon is turned off
  Auto-Negotiation is turned on  FEC mode is Auto
  Input flow-control is off, output flow-control is off
  Auto-mdix is turned on
  Switchport monitor is off 
  EtherType is 0x8100 
  EEE (efficient-ethernet) : Disabled
    admin fec state is auto, oper fec state is off
  Last link flapped never
  Last clearing of "show interface" counters never
  0 interface resets
  Load-Interval #1: 30 seconds
    30 seconds input rate 0 bits/sec, 0 packets/sec
    30 seconds output rate 0 bits/sec, 0 packets/sec
    input rate 0 bps, 0 pps; output rate 0 bps, 0 pps
  Load-Interval #2: 5 minute (300 seconds)
    300 seconds input rate 0 bits/sec, 0 packets/sec
    300 seconds output rate 0 bits/sec, 0 packets/sec
    input rate 0 bps, 0 pps; output rate 0 bps, 0 pps
  RX
    0 unicast packets  0 multicast packets  0 broadcast packets
    0 input packets  0 bytes
    0 jumbo packets  0 storm suppression bytes
    0 runts  0 giants  0 CRC  0 no buffer
    0 input error  0 short frame  0 overrun   0 underrun  0 ignored
    0 watchdog  0 bad etype drop  0 bad proto drop  0 if down drop
    0 input with dribble  0 input discard
    0 Rx pause
    0 Stomped CRC
  TX
    0 unicast packets  0 multicast packets  0 broadcast packets
    0 output packets  0 bytes
    0 jumbo packets
    0 output error  0 collision  0 deferred  0 late collision
    0 lost carrier  0 no carrier  0 babble  0 output discard
    0 Tx pause

@mjbear
Copy link
Collaborator

mjbear commented Apr 28, 2025

Yes they are in bytes. See below.

Ok, some of the older output may not be in bytes, but I'll have to validate that.

Is there anything else i need to do on this

If you locally worked on this, pull the changes that have been made to your branch (git pull).

Please add the Eth1/1 output that had runts/giants/CRC errors in a raw file (ex: tests/cisco_nxos/show_interface/cisco_nxos_show_interface4.raw). Then use the helper scripts to generate the structured data. Commit both of those files to your branch and push the commit(s) to GitHub.

or the other one i submitted?

I have only looked at this one so far.
Once I have had time to look at the other I'll drop feedback there. Thank you for understanding!

@mjbear mjbear linked an issue Apr 28, 2025 that may be closed by this pull request
@mjbear
Copy link
Collaborator

mjbear commented Apr 28, 2025

Yes they are in bytes. See below.

Ok, some of the older output may not be in bytes, but I'll have to validate that.

Yeah, there are storm suppression packets in the output that follow Jumbo packet output. And adding another rule captures jumbo values that were otherwise not captured.

The bandwidth rates existed before your changes, but I do want to point out (for discussion) that if there are multiple rates, only the second one is captured. If we were to change some of those capture groups to List we could gather them up in one variable but that data type change is a "breaking change". And do we want all that data?

@Mikeg2881
Copy link
Contributor Author

I am all for adding data as long as it doesn't produce a breaking change.

@mjbear
Copy link
Collaborator

mjbear commented Apr 28, 2025

I am all for adding data as long as it doesn't produce a breaking change.

Sounds good.
I'll have to take a closer look later on.

In the meantime if you would add raw cli text and create the structured output (yaml) as hinted at earlier in this thread that would be greatly appreciated.

@Mikeg2881
Copy link
Contributor Author

cisco_nxos_show_interface.zip
see attached

@mjbear
Copy link
Collaborator

mjbear commented Apr 28, 2025

cisco_nxos_show_interface.zip see attached

  1. Please add the raw cli text to the changed files on the PR. It will be necessary to either use a local development environment or spin up a GitHub Codespace to be able to run the helper scripts.

  2. On another note, cisco_ios_show_interfaces has used the capture group name OVERRUN so in the spirit of normalization/standardization -- we'll want to discuss what makes sense in terms of capture group names. (That is not unnecessarily introducing a major shift or breaking changes unless it is warranted.)

@mjbear
Copy link
Collaborator

mjbear commented Apr 29, 2025

cisco_nxos_show_interface5.raw ended up in the wrong directory sh_ip_interface, so I moved it.

I noticed 4.raw in the Arista PR so I pulled a copy of it to this PR only to find it seems to be the same file. 🙂

I noticed some of those raw files are around 2500+ lines huge. We can likely get away with much less than that and still have test coverage so I'm going to whittle the file size down.

Please say something if there were certain sections of those files you felt were unique to the existing test data. For example, certainly the port-channel members bit required unique changes to accommodate.

I also had to monkey with converting that raw file from DOS format to Unix so we didn't end up with some unexpected characters (see below).

-    interface: "\uFEFFEthernet1/1"
+    interface: "Ethernet1/1"

🤘 😁

@mjbear
Copy link
Collaborator

mjbear commented Apr 29, 2025

  1. On another note, cisco_ios_show_interfaces has used the capture group name OVERRUN so in the spirit of normalization/standardization -- we'll want to discuss what makes sense in terms of capture group names. (That is not unnecessarily introducing a major shift or breaking changes unless it is warranted.)

We might be at a point to begin discussing the capture group names. (No need to make changes right away.)

I'm of the opinion that we should utilize the existing capture group names to maintain consistency and normalization across multiple vendors (where possible). So this could mean the RX_ and TX_ pieces might be something to remove.

Hopefully we can get a few more voices in here for a wider conversation.

@mjbear
Copy link
Collaborator

mjbear commented Apr 29, 2025

Lots and lots of formats have been added to this point to support various versions of output and attempt capture the data there's capture groups for.

😅 phew!!!!!!!!!!! 😇

@mjbear
Copy link
Collaborator

mjbear commented Apr 30, 2025

  1. On another note, cisco_ios_show_interfaces has used the capture group name OVERRUN so in the spirit of normalization/standardization -- we'll want to discuss what makes sense in terms of capture group names. (That is not unnecessarily introducing a major shift or breaking changes unless it is warranted.)

We might be at a point to begin discussing the capture group names. (No need to make changes right away.)

I'm of the opinion that we should utilize the existing capture group names to maintain consistency and normalization across multiple vendors (where possible). So this could mean the RX_ and TX_ pieces might be something to remove.

Hopefully we can get a few more voices in here for a wider conversation.

To quote @Mikeg2881 in a conversation outside of this PR, "Also not apposed to ditching RX and TX. Keeping it uniform with other modules seems more logical". Thank you MG!

It may take a moment to investigate each capture group, but the normalization will be beneficial in the long run!

@mjbear mjbear changed the title nxos show interface changes Update nxos show interface Apr 30, 2025
@mjbear
Copy link
Collaborator

mjbear commented May 4, 2025

  1. On another note, cisco_ios_show_interfaces has used the capture group name OVERRUN so in the spirit of normalization/standardization -- we'll want to discuss what makes sense in terms of capture group names. (That is not unnecessarily introducing a major shift or breaking changes unless it is warranted.)

We might be at a point to begin discussing the capture group names. (No need to make changes right away.)
I'm of the opinion that we should utilize the existing capture group names to maintain consistency and normalization across multiple vendors (where possible). So this could mean the RX_ and TX_ pieces might be something to remove.
Hopefully we can get a few more voices in here for a wider conversation.

To quote @Mikeg2881 in a conversation outside of this PR, "Also not apposed to ditching RX and TX. Keeping it uniform with other modules seems more logical". Thank you MG!

It may take a moment to investigate each capture group, but the normalization will be beneficial in the long run!

For example when searching for OVERRUN (or similarly GIANTS):

Some templates do not have the RX/TX errors prefixed with RX_ or TX_.

However some templates do.

@mjbear
Copy link
Collaborator

mjbear commented May 17, 2025

Some templates do not have the RX/TX errors prefixed with RX_ or TX_.

... snipped ...

However some templates do.

Responding to myself.
Since there are more templates without the prefix (and particularly two other Cisco platform templates), we probably should standardize without RX_ and TX_.

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

Successfully merging this pull request may close these issues.

cisco_nxos_show_interface.textfsm (Updates)
2 participants