search

openconfig / public

Repository for publishing OpenConfig models, documentation, and other material for the community.
Apache License 2.0
893 stars 652 forks source link

Ethernet port speed for aggregated interfaces #1181

Open cfyo opened 1 week ago

cfyo commented 1 week ago

We have an aggregated interface that is bundle made up of 5x 100G physical interfaces. The /interfaces/interface/aggregation/state/lag-speed shows up correctly for the aggregated interface but for the path /interfaces/interface/ethernet/state/port-speed & negotiated-port-speed the vendor assigns the closest value which in this case was speed_600GB which is not accurate. What should be the behavior for this use case in OpenConfig?

dplore commented 1 week ago

Hi, it seems port-speed maybe didn't think through the use case for aggregated ports and only considered speed of physical ports.

Solutions for this could be

  1. add more ETHERNET_SPEED identities. (backwards compatible, but not great scalability as there would need to be very long list of speeds to cover operational use cases)
  2. Create a new leaf specifically for aggregated interface speeds, of type uint64
  3. Other ideas?
cfyo commented 1 week ago

If there was a new leaf added called "aggregate-port-speed" what should the behavior be for the other leaves port-speed and negotiated-port-speed in this situation? I would still not want them showing an incorrect value. It seems like we may still need a new ETHERNET_SPEED identity added for this use case even with the addition of a new leaf. And if so, what would you recommend for the name?

LimeHat commented 1 week ago

I would argue that port-speed is simply not applicable to the LAG interfaces.

re.

  1. Create a new leaf specifically for aggregated interface speeds, of type uint64

this already exists as indicated by the OP, /interfaces/interface/aggregation/state/lag-speed

earies commented 1 week ago

I don't think (1) is feasible for the reasons already mentioned. The identities are tied to physical ethernet interface speeds and creating multiples to cover all aggregate use-cases is a non-starter imo.

Today, I believe an implementation should omit ethernet/state/port-speed for LAG interfaces for this reason rather than randomly choose the closest identity value, that is just incorrect. This leaf should only be applicable to physical interfaces (and not logical)

aggregation/state/lag-speed is already what should represent this for LAG interfaces and is less restrictive as a uint32 measured in Mbps. There is probably some usefulness to understand how the bundle is configured (e.g. 8x100G thus is desired to be 800Gbps) vs. dynamic adjustments should members fail (e.g. 6x100G == 600Gbps as 2 members went down)

It must be known however there are separate paths in order to access the speed of physical vs. logical but this can be determined by the type of interface

earies commented 1 week ago

Submitted https://github.com/openconfig/public/pull/1183 to put restrictions around physical vs. aggregate for ethernet related config/state