Skip to content

openconfig-vlan-types is missing definition of canonical form #663

Open

Description

@mmiklus

VLAN trunk-vlans are defined as a union of integer (vlan-id) and string (vlan-range).

This type might be good for human to computer interaction (to mimic CLI behavior),
unfortunately it is not so good for machine to machine interactions.

The model is missing description of canonical form of the data, thus the actual behavior of
trunk-vlans can vary from vendor to vendor.

For example, when you request a device to attach interface to following VLANs:

{
    "openconfig-vlan:trunk-vlans": [
      1,
      2,
      4,
      5,
      6,
      8
    ]
}

MUST the device normalize the data like this:

{
    "openconfig-vlan:trunk-vlans": [
        "1..2",
        "4..6",
        8
    ]
}

or MUST the data (type) remain intact from the original SetRequest ?

Activity

  1. dplore commented on Jun 30, 2022

    @dplore
    Member

    My interpretation of the yang specification is either format is permitted and it is the client's responsibility to attempt to validate the data type in the order specified by the union type.

    ref: https://datatracker.ietf.org/doc/html/rfc6020#section-9.12

  2. mmiklus commented on Jun 30, 2022

    @mmiklus
    Author

    @dplore, that's right. The question is a bit different though:
    When client set VLANS '[1,2,3]' (type=list of integer), is server allowed to normalize the data to a list of strings '["1..3"]' ?
    Or when client set VLANs [string(1..5), integer(8)], is the server allowed to change the data type to a list of integers [1,2,3,4,5] ?

  3. dplore commented on Jul 1, 2022

    @dplore
    Member

    I think this is a question of the yang specification and not openconfig. However, I'll still offer my opinion! A device can 'change' the data type as you have described.

    My interpretation of the yang specification is it allows expression of a union data type as any of the subtypes and it is irrelevant which type was used by a client or server in the past. The server has the freedom to use any and all of the datatypes present in the union and a client must implement all the types as well (or it won't be compliant with the model).

    A gNMI server supporting openconfig is expected to serialize data into and out of a its configuration data store. During that serialization I expect data type conversions to occur which result in compliant yang objects. The server must choose
    one of the datatypes in a union, but it can be any of them.

  4. mmiklus commented on Jul 1, 2022

    @mmiklus
    Author

    Usually, if server changes data it is because canonical form of the data is defined... here is a couple of examples:
    https://github.com/openconfig/public/blob/master/third_party/ietf/ietf-yang-types.yang#L330
    https://github.com/openconfig/public/blob/master/third_party/ietf/ietf-yang-types.yang#L394
    https://github.com/openconfig/public/blob/master/third_party/ietf/ietf-yang-types.yang#L409
    https://github.com/openconfig/public/blob/master/third_party/ietf/ietf-yang-types.yang#L441
    https://github.com/openconfig/public/blob/master/third_party/ietf/ietf-yang-types.yang#L452

    Unfortunately, openconfig model do not define canonical representation of trunk-vlans, thus this issue is issue of the model.

    A few of options how to fix this:

    1. describe canonical form in the "trunk-vlans" description - this is the most easiest & backward compatible "fix"
    2. define additional leaf-list "trunk-vlans-list" (or range) with normalized data and decide which type to use integers[] or string[]
    3. define additional "trunk-vlan" container, with 2 leaf-lists - 1. trunk-vlans-ranges (string[]) 2. trunk-vlan-ids(integer[])
  5. dplore commented on Jul 1, 2022

    @dplore
    Member

    Thanks for the conversation. Those examples of canonical forms applied to a single type make sense to me. But here we are dealing with a union datatype.

    For option 1, are you requesting that one of the types in the union be declared as canonical? Which implies that one of those types should be used as the 'default' by a server responding to a read request? If so, is there precedent for this in yang? I do not find a reference for it? All I can find is in rfc7950-section-9.12.3 which I interpret as each member type may have a canonical form, but not the union itself.

    For option 2 and 3, it sounds like you are suggesting that we add leafs which do not implement the union data type?

    Is the root issue a desire to not support the variability required by a union data type? I can see where option 2 and 3 make it unnecessary to support this union and instead allow the client to specifically request the datatype it wants by using the desired leaf.

    While there is precedent to have one than one way to get/set data in OC models, I like to avoid it because it negatively impacts vendor consistency/neutrality. This is because vendor A could support leaf 1 and vendor B could support leaf 2. An operator is then obligated to implement different client behavior depending on the vendor implementation.

  6. mmiklus commented on Jul 13, 2022

    @mmiklus
    Author

    @dplore, thank you for your time to discuss this issue with me.

    trunk-vlans are exactly such a precedent. When you take for example a list of VLANs [1..4] , there are many different ways how to represent them and any of following combinations is valid:

    [1,2,3,4]
    ["1..2",3,4]
    [1,"2..3",4]
    [1,2,"3..4"]
    ["1..3",4]
    [1,"2..4"]
    ["1..4"]
    [1,"1..4"]
    [2,"1..4"]
    

    That affects mostly update and delete operations. For example, there are many ways how to delete a VLAN 2 from a range of "1..4":
    SetRequest(delete([2])
    SetRequest(replace [1,2,3,4] with [1,3,4])
    SetRequest(replace ["1..4"] with [1,3,4])
    SetRequest(replace ["1..2"] with [1])
    SetRequest(replace ["2..3"] with [3])
    ...

    If you want to implement an external datastore (e.g. configuration cache / backup / restore.. ) you can't simply get the data from a device and compare them with your internal datastore by comparing object to object. As you mentioned in previous comment - you have to write a custom 'trunk-vlan' parser, which is not based on the YANG model, but on a semantic normalization. So instead of O(1) comparison, you're somewhere between O(1) - O(n) complexity, most probably wasting CPU cycles with data normalization.

    And you're right - all of the proposed options how to fix this ambiguity are a bit radical for a widely-adopted YANG model.

    There might be a less-invasive way how to fix the OC-YANG 'vlan-trunks' ambiguity without changing the data model, by making the leaf-list immutable. Basically extend the description with following details: "Server should not transform data between different data types nor change the size of the list. Following transformations are not allowed: ([1,2,3] -> ["1..3"], ["1..2"] -> [1,2])"

    Btw. this is IEEE's way of handling VLAN ranges:
    'vid-range-type'
    String gives you less alternatives how to perform UPDATE operation and when you're some config db (e.g. cache) microservice in between the sever and client(s), then you only need to be YANG-aware, you don't need to be YANG-ieee802-dot1q-aware.

  7. mmiklus commented on Jul 13, 2022

    @mmiklus
    Author

    Based on RFC7950 7.7 .. https://datatracker.ietf.org/doc/html/rfc7950#section-7.7

     Conceptually, the values in the data tree MUST be in the canonical form
    
    

    it seems like all of the data in data-tree MUST have canonical form defined, which means that the openconfig-vlan-types YANG model is not RFC7950 compliant.

  8. dplore commented on Jul 15, 2022

    @dplore
    Member

    Thanks for finding this. I think the RFC's requirement is a good idea. I will take your proposal to make this model rfc7950 compliant to the OC operator's meeting next week.

  9. github-actions commented on May 17, 2024

    @github-actions

    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.

  10. mmiklus commented on May 17, 2024

    @mmiklus
    Author

    Hi @dplore, any update on this ?

  11. github-actions commented on Jan 12, 2025

    @github-actions

    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.

  12. mmiklus commented on Jan 13, 2025

    @mmiklus
    Author

    @aashaikh @robshakir - Do you have any thoughts on #663 (comment) ?

  13. github-actions commented on Sep 29, 2026

    @github-actions

    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.

  14. added a commit that references this issue on Oct 8, 2026
    6645d1a
  15. mmiklus commented on Oct 8, 2026

    @mmiklus
    Author

    Proposed fix in #1566
    @dplore - please review

  16. added a commit that references this issue on Oct 16, 2026
    f62c1e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions