Default values for red/enable-ecn and wred/enable-ecn mean both are effectively enabled.

[rgwilton]

In the following model, both enable-ecn and drop have default values that makes the model ambiguous as to whether red or wred is applies to the queue.

Under:

     +--rw queue-management-profiles
        +--rw queue-management-profile* [name]
           +--rw wred
           |  +--rw uniform
           |     +--rw config
           |     |  +--rw min-threshold?                  uint64
           |     |  +--rw max-threshold?                  uint64
           |     |  +--rw enable-ecn?                     boolean
           |     |  +--rw drop?                           boolean
           |     |  +--rw weight?                         uint32
           |     |  +--rw max-drop-probability-percent?   oc-types:percentage
           |     +--ro state
           |        +--ro min-threshold?                  uint64
           |        +--ro max-threshold?                  uint64
           |        +--ro enable-ecn?                     boolean
           |        +--ro drop?                           boolean
           |        +--ro weight?                         uint32
           |        +--ro max-drop-probability-percent?   oc-types:percentage
           +--rw red
              +--rw uniform
                 +--rw config
                 |  +--rw min-threshold?   uint64
                 |  +--rw max-threshold?   uint64
                 |  +--rw enable-ecn?      boolean
                 |  +--rw drop?            boolean
                 +--ro state
                    +--ro min-threshold?   uint64
                    +--ro max-threshold?   uint64
                    +--ro enable-ecn?      boolean
                    +--ro drop?            boolean

[dplore]

Hi @rgwilton , they are both disabled by default, so that much is clear I think. However, is the issue that RED and WRED should be mutually exclusive? ie: we shouldn't be able to enable both.

OpenConfig style guide doesn't support using yang choice but we could update the descriptions of the leafs to indicate they are mutually exclusive.

Thoughts?

[dplore]

Closing this issue, but if needed, please feel free to reopen

[rgwilton]

Sorry for being slow to respond.

This is just one of the examples where there are mutually exclusive features (e.g., only one of them is expected to be present) but the use of YANG defaults means that they are both in effect.

I think that this needs to be fixed, with the possible different approaches being:

1) Remove the default values. 2) Allow the use choice or presence containers to fix the model at the language level (but OpenConfig doesn't usually use these constructs because it prevents operational state from being reported, if it hasn't been configured). 3) For this particular case, rework the model so that the config is merged into a single container (but this can't be a generic solution). I.e., have a single container called 'red' but that contains all the configuration that you have under 'wred'. If no weight has been specified then it is red, if a weight has been specified then it is wred.

Fixing this in the descriptions doesn't really help, because controllers that enforce the YANG language semantics will just end up doing the wrong thing, which will probably mean that vendors end up deviating the model to one of the other solutions presented above.

Thanks, Rob

[dplore]

Perhaps we could achieve what is described in 3. by refactoring this to use an enum for queue-management, with three options: disable, red, wred. This would make the options mutually exclusive. Does that sound like what you're looking for? If yes, please submit a PR with your suggestion.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. If you wish to keep this issue active, please remove the stale label or add a comment, otherwise will be closed in 14 days.