openconfig/public

Inconsistent 'enabled' structure in STP

rgwilton opened this issue · 3 comments

Looking at the STP model, we noticed that is configures protocol instance enable differently from how this is normally done in OC, i.e., via a per protocol instance enabled flag:

  +--rw stp
     +--rw global
     |  +--rw config
     |  |  +--rw enabled-protocol*               identityref

when the enabled-protocol is a leaf-list that default to empty, or otherwise contains RSTP, MSTP or RAPID_PVST.

Is there any reason why this shouldn't just be done as simple enabled flags for the rstp, mstp, and rapid-pvst containers, i.e., mirroring how this is generally done in other parts the OC YANG model?

E.g.,

  +--rw stp
     +--rw rstp
     |  +--rw config
     |  |  +--rw enabled?         boolean
     +--rw mstp
     |  +--rw config
     |  |  +--rw enabled?         boolean
     +--rw pvst
     |  +--rw config
     |  |  +--rw enabled?         boolean
dplore commented

For quick reference, /stp/global/config/enabled-protocol and the git blame for the addition to the models.

Comments from internal notes only show this is intended as a way to allow multiple STP protocols to be enabled at the same time. More comments from others on this topic are appreciated.

dplore commented

Recommend submitting a PR to refactor this as suggested. To prevent a breaking change, please use deprecate on the existing list.