Skip to content

Refactor LLDP management addresses and deprecate previous management address leafs#1250

Merged
dplore merged 18 commits intoopenconfig:masterfrom
earies:lldp-mgmt-address
Jan 16, 2026
Merged

Refactor LLDP management addresses and deprecate previous management address leafs#1250
dplore merged 18 commits intoopenconfig:masterfrom
earies:lldp-mgmt-address

Conversation

@earies
Copy link
Copy Markdown
Contributor

@earies earies commented Feb 3, 2025

  • (M) release/models/lldp/openconfig-lldp-types.yang
    • Addition of management address related identities/typedef
  • (M) release/models/lldp/openconfig-lldp.yang
    • Addition of new management address hierarchy to match IEEE
      802.1ab-2016 definition
    • Deprecate flat singleton management-address leafs
    • Leverage OC types vs. previous IETF types

Change Scope

The previous definition of LLDP management addresses had the following design flaws

  • Singular management address (IPv4 or IPv6)
  • Only partial support of the Management Address TLV per IEEE 802.1AB-2016
  • Non-configurable management-address selection
  • Lack of support for conveying local management addresses

This change encompasses:

  • A new list structure to accomodate multiple management addresses
  • Expansion of the structure to support relevant Management Address TLV fields
  • Support to convey local system management addresses
  • Commencing deprecation of the previous 2 leafs representing a single management address
  • Removal of IETF types in favor of OC types (keeping same underlying types)

While openconfig-lldp has been bumped to 1.0.0 due to the import/type
changes from IETF -> OC, this change is mostly backwards compatible as we are
slowly deprecating the previous management-address related leafs.

This change better aligns Management Address TLV support to the IEEE
802.1AB-2016 spec.

Platform Implementations

For configurable management address interface derivation:

Remainder is for IEEE 802.1AB-2016 compliance

…address leafs

  * (M) release/models/lldp/openconfig-lldp-types.yang
    - Addition of management address related identities/typedef
  * (M) release/models/lldp/openconfig-lldp.yang
    - Addition of new management address hierarchy to match IEEE
      802.1ab-2016 definition
    - Deprecate flat singleton management-address leafs
    - Leverage OC types vs. previous IETF types
@earies earies requested a review from a team as a code owner February 3, 2025 21:16
@earies
Copy link
Copy Markdown
Contributor Author

earies commented Feb 3, 2025

New tree:

module: openconfig-lldp
  +--rw lldp
     +--rw config
     |  +--rw enabled?                      boolean
     |  +--rw hello-timer?                  uint64
     |  +--rw suppress-tlv-advertisement*   identityref
     |  +--rw system-name?                  string
     |  +--rw system-description?           string
     |  +--rw chassis-id?                   string
     |  +--rw chassis-id-type?              oc-lldp-types:chassis-id-type
     |  +--rw management-interface?         oc-if:interface-id
     +--ro state
     |  +--ro enabled?                      boolean
     |  +--ro hello-timer?                  uint64
     |  +--ro suppress-tlv-advertisement*   identityref
     |  +--ro system-name?                  string
     |  +--ro system-description?           string
     |  +--ro chassis-id?                   string
     |  +--ro chassis-id-type?              oc-lldp-types:chassis-id-type
     |  +--ro management-interface?         oc-if:interface-id
     |  +--ro counters
     |     +--ro frame-in?           oc-yang:counter64
     |     +--ro frame-out?          oc-yang:counter64
     |     +--ro frame-error-in?     oc-yang:counter64
     |     +--ro frame-discard?      oc-yang:counter64
     |     +--ro tlv-discard?        oc-yang:counter64
     |     +--ro tlv-unknown?        oc-yang:counter64
     |     +--ro last-clear?         oc-yang:date-and-time
     |     +--ro tlv-accepted?       oc-yang:counter64
     |     +--ro entries-aged-out?   oc-yang:counter64
     +--ro management-addresses
     |  +--ro management-address* [address]
     |     +--ro address    -> ../state/address
     |     +--ro state
     |        +--ro address?                    union
     |        +--ro interface-number?           uint32
     |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
     +--rw interfaces
        +--rw interface* [name]
           +--rw name         -> ../config/name
           +--rw config
           |  +--rw name?      oc-if:base-interface-ref
           |  +--rw enabled?   boolean
           +--ro state
           |  +--ro name?       oc-if:base-interface-ref
           |  +--ro enabled?    boolean
           |  +--ro counters
           |     +--ro frame-in?          oc-yang:counter64
           |     +--ro frame-out?         oc-yang:counter64
           |     +--ro frame-error-in?    oc-yang:counter64
           |     +--ro frame-discard?     oc-yang:counter64
           |     +--ro tlv-discard?       oc-yang:counter64
           |     +--ro tlv-unknown?       oc-yang:counter64
           |     +--ro last-clear?        oc-yang:date-and-time
           |     +--ro frame-error-out?   oc-yang:counter64
           +--ro neighbors
              +--ro neighbor* [id]
                 +--ro id                      -> ../state/id
                 +--ro config
                 +--ro state
                 |  +--ro system-name?               string
                 |  +--ro system-description?        string
                 |  +--ro chassis-id?                string
                 |  +--ro chassis-id-type?           oc-lldp-types:chassis-id-type
                 |  +--ro management-interface?      oc-if:interface-id
                 |  +--ro id?                        string
                 |  +--ro age?                       uint64
                 |  +--ro last-update?               int64
                 |  +--ro ttl?                       uint16
                 |  +--ro port-id?                   string
                 |  +--ro port-id-type?              oc-lldp-types:port-id-type
                 |  +--ro port-description?          string
                 |  x--ro management-address?        string
                 |  x--ro management-address-type?   string
                 +--ro management-addresses
                 |  +--ro management-address* [address]
                 |     +--ro address    -> ../state/address
                 |     +--ro state
                 |        +--ro address?                    union
                 |        +--ro interface-number?           uint32
                 |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
                 +--ro custom-tlvs
                 |  +--ro tlv* [type oui oui-subtype]
                 |     +--ro type           -> ../state/type
                 |     +--ro oui            -> ../state/oui
                 |     +--ro oui-subtype    -> ../state/oui-subtype
                 |     +--ro config
                 |     +--ro state
                 |        +--ro type?          int32
                 |        +--ro oui?           string
                 |        +--ro oui-subtype?   string
                 |        +--ro value?         binary
                 +--ro capabilities
                    +--ro capability* [name]
                       +--ro name      -> ../state/name
                       +--ro config
                       +--ro state
                          +--ro name?      identityref
                          +--ro enabled?   boolean

Validated related example instance-data

{
  "openconfig-interfaces:interfaces": {
    "interface": [
      {
        "name": "et-0/0/0",
        "config": {
          "name": "et-0/0/0",
          "type": "iana-if-type:ethernetCsmacd"
        },
        "state": {
          "name": "et-0/0/0",
          "type": "iana-if-type:ethernetCsmacd",
          "loopback-mode": "NONE",
          "enabled": true,
          "admin-status": "UP",
          "oper-status": "UP"
        },
        "hold-time": {
          "state": {
            "up": 0,
            "down": 0
          }
        },
        "penalty-based-aied": {
          "state": {
            "max-suppress-time": 0,
            "decay-half-life": 0,
            "suppress-threshold": 0,
            "reuse-threshold": 0,
            "flap-penalty": 0
          }
        }
      },
      {
        "name": "et-1/0/3",
        "config": {
          "name": "et-1/0/3",
          "type": "iana-if-type:ethernetCsmacd"
        },
        "state": {
          "name": "et-1/0/3",
          "type": "iana-if-type:ethernetCsmacd",
          "loopback-mode": "NONE",
          "enabled": true,
          "admin-status": "UP",
          "oper-status": "UP"
        },
        "hold-time": {
          "state": {
            "up": 0,
            "down": 0
          }
        },
        "penalty-based-aied": {
          "state": {
            "max-suppress-time": 0,
            "decay-half-life": 0,
            "suppress-threshold": 0,
            "reuse-threshold": 0,
            "flap-penalty": 0
          }
        }
      }
    ]
  },
  "openconfig-lldp:lldp": {
    "config": {
      "enabled": true,
      "management-interface": "vlan.100"
    },
    "state": {
      "enabled": true,
      "management-interface": "vlan.100"
    },
    "management-addresses": {
      "management-address": [
        {
          "address": "10.100.1.1",
          "state": {
            "address": "10.100.1.1",
            "interface-number": 504,
            "interface-number-subtype": "IFINDEX"
          }
        },
        {
          "address": "2001:db8:ffff::1",
          "state": {
            "address": "2001:db8:ffff::1",
            "interface-number": 504,
            "interface-number-subtype": "IFINDEX"
          }
        }
      ]
    },
    "interfaces": {
      "interface": [
        {
          "name": "et-0/0/0",
          "config": {
            "name": "et-0/0/0",
            "enabled": true
          },
          "state": {
            "name": "et-0/0/0",
            "enabled": true
          },
          "neighbors": {
            "neighbor": [
              {
                "id": "b8:ae:ed:7c:7c:a9-b8:ae:ed:7c:7c:a9",
                "state": {
                  "id": "b8:ae:ed:7c:7c:a9-b8:ae:ed:7c:7c:a9"
                },
                "management-addresses": {
                  "management-address": [
                    {
                      "address": "10.200.1.32",
                      "state": {
                        "address": "10.200.1.32",
                        "interface-number": 512,
                        "interface-number-subtype": "IFINDEX"
                      }
                    },
                    {
                      "address": "2001:db8:f2dd::fe",
                      "state": {
                        "address": "2001:db8:f2dd::fe",
                        "interface-number": 512,
                        "interface-number-subtype": "IFINDEX"
                      }
                    }
                  ]
                }
              }
            ]
          }
        },
        {
          "name": "et-1/0/3",
          "config": {
            "name": "et-1/0/3",
            "enabled": true
          },
          "state": {
            "name": "et-1/0/3",
            "enabled": true
          },
          "neighbors": {
            "neighbor": [
              {
                "id": "64:87:88:a5:a1:00-ge-0/1/1",
                "state": {
                  "id": "64:87:88:a5:a1:00-ge-0/1/1"
                },
                "management-addresses": {
                  "management-address": [
                    {
                      "address": "83:b3:30:8f:21:00",
                      "state": {
                        "address": "83:b3:30:8f:21:00",
                        "interface-number": 603,
                        "interface-number-subtype": "IFINDEX"
                      }
                    }
                  ]
                }
              }
            ]
          }
        }
      ]
    }
  }
}

@dplore
Copy link
Copy Markdown
Member

dplore commented Feb 19, 2025

/gcbrun

@OpenConfigBot
Copy link
Copy Markdown

OpenConfigBot commented Feb 19, 2025

Major YANG version changes in commit b199a82:
openconfig-lldp.yang: 0.2.1 -> 1.0.0

@dplore
Copy link
Copy Markdown
Member

dplore commented Feb 19, 2025

/gcbrun

@dplore dplore moved this to Ready to discuss in OC Operator Review Mar 6, 2025
@dplore
Copy link
Copy Markdown
Member

dplore commented Apr 3, 2025

/gcbrun

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Apr 3, 2025

Addressed structural (state) discussion item from today's OpenConfig community call and updated the tree/instance-data view above. cc: @dplore

@dplore
Copy link
Copy Markdown
Member

dplore commented Apr 8, 2025

/gcbrun

@dplore
Copy link
Copy Markdown
Member

dplore commented Apr 8, 2025

Reviewed in Apr 8, 2025 operators meeting. I think we don't need the SNMP derived fields unless there is strong evidence that operators require this.

     +--ro management-addresses
     |  +--ro management-address* [address subtype]
     |     +--ro subtype    -> ../state/subtype
     |     +--ro state
     |        +--ro subtype?                    identityref
     |        +--ro interface-number?           uint32
     |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
     |        +--ro oid?                        string

That just leaves:

     +--ro management-addresses
     |  +--ro management-address* [address subtype]
     |     +--ro address    -> ../state/address
     |     +--ro state
     |        +--ro address?                    union

@dplore
Copy link
Copy Markdown
Member

dplore commented Apr 8, 2025

There is a typo causing the github check errors

@dplore dplore moved this from Ready to discuss to Waiting for author in OC Operator Review Apr 8, 2025
@earies
Copy link
Copy Markdown
Contributor Author

earies commented Apr 8, 2025

Reviewed in Apr 8, 2025 operators meeting. I think we don't need the SNMP derived fields unless there is strong evidence that operators require this.

     +--ro management-addresses
     |  +--ro management-address* [address subtype]
     |     +--ro subtype    -> ../state/subtype
     |     +--ro state
     |        +--ro subtype?                    identityref
     |        +--ro interface-number?           uint32
     |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
     |        +--ro oid?                        string

That just leaves:

     +--ro management-addresses
     |  +--ro management-address* [address subtype]
     |     +--ro address    -> ../state/address
     |     +--ro state
     |        +--ro address?                    union

subtype must also remain as that is a key in the multi-key list

But I can remove oid, interface-number and interface-number-subtype if that is what we agree upon.

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Apr 8, 2025

There is a typo causing the github check errors

Addressed in latest commit - this was due to optical-transport models referencing removed/unused LLDP groupings

@dplore
Copy link
Copy Markdown
Member

dplore commented May 1, 2025

/gcbrun

@earies
Copy link
Copy Markdown
Contributor Author

earies commented May 2, 2025

Updated in latest commit per comments from OC community call, tree view updated here: #1250 (comment)

ro management-addresses
+--ro management-address* [address]
   +--ro address    -> ../state/address
   +--ro state
      +--ro address?                    union
      +--ro interface-number?           uint32
      +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
  • Removal of OID and address subtype (address subtype can be derived)
  • List reduced to single-key
  • address is of type IPv4, IPv6 or MAC address
  • interface-number and interface-number-subtype still remain at this time as there does need to be an association of what the local/remote management-address is attached to (comments? @rgwilton @dplore)

@earies earies requested a review from a team as a code owner November 11, 2025 22:32
@earies
Copy link
Copy Markdown
Contributor Author

earies commented Nov 11, 2025

@dplore @ElodinLaarz - looking for an update/review on this PR - thx

@dplore dplore moved this from Waiting for author to Ready to discuss in OC Operator Review Nov 12, 2025
Copy link
Copy Markdown
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor suggestion on a description

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 9, 2025

Reviewed in Dec 9 2025 OC Operators meeting without objection. There was also explicit support to include this as there is a need for SNMP / LLDP interoperability.

@dplore dplore moved this from Ready to discuss to last-call in OC Operator Review Dec 9, 2025
@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 9, 2025

Please also fix linter issue:
lldp/openconfig-lldp.yang (441): error:
All key arguments of a list should be quoted (address is not)

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 9, 2025

Added to last-call for comments. This will merge on Dec 23, 2025

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Dec 9, 2025

Please also fix linter issue: lldp/openconfig-lldp.yang (441): error: All key arguments of a list should be quoted (address is not)

Addressed in latest commit

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 9, 2025

/gcbrun

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 23, 2025

Diff view with changes highlighted:

 module: openconfig-lldp
   +--rw lldp
      +--rw config
      |  +--rw enabled?                      boolean
      |  +--rw hello-timer?                  uint64
      |  +--rw suppress-tlv-advertisement*   identityref
      |  +--rw system-name?                  string
      |  +--rw system-description?           string
      |  +--rw chassis-id?                   string
      |  +--rw chassis-id-type?              oc-lldp-types:chassis-id-type
+     |  +--rw management-interface?         oc-if:interface-id
      +--ro state
      |  +--ro enabled?                      boolean
      |  +--ro hello-timer?                  uint64
      |  +--ro suppress-tlv-advertisement*   identityref
      |  +--ro system-name?                  string
      |  +--ro system-description?           string
      |  +--ro chassis-id?                   string
      |  +--ro chassis-id-type?              oc-lldp-types:chassis-id-type
+     |  +--ro management-interface?         oc-if:interface-id
      |  +--ro counters
-     |     +--ro frame-in?           yang:counter64
-     |     +--ro frame-out?          yang:counter64
-     |     +--ro frame-error-in?     yang:counter64
-     |     +--ro frame-discard?      yang:counter64
-     |     +--ro tlv-discard?        yang:counter64
-     |     +--ro tlv-unknown?        yang:counter64
-     |     +--ro last-clear?         yang:date-and-time
-     |     +--ro tlv-accepted?       yang:counter64
-     |     +--ro entries-aged-out?   yang:counter64
+     |     +--ro frame-in?           oc-yang:counter64
+     |     +--ro frame-out?          oc-yang:counter64
+     |     +--ro frame-error-in?     oc-yang:counter64
+     |     +--ro frame-discard?      oc-yang:counter64
+     |     +--ro tlv-discard?        oc-yang:counter64
+     |     +--ro tlv-unknown?        oc-yang:counter64
+     |     +--ro last-clear?         oc-yang:date-and-time
+     |     +--ro tlv-accepted?       oc-yang:counter64
+     |     +--ro entries-aged-out?   oc-yang:counter64
+     +--ro management-addresses
+     |  +--ro management-address* [address]
+     |     +--ro address    -> ../state/address
+     |     +--ro state
+     |        +--ro address?                    union
+     |        +--ro interface-number?           uint32
+     |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
      +--rw interfaces
         +--rw interface* [name]
            +--rw name         -> ../config/name
            +--rw config
            |  +--rw name?      oc-if:base-interface-ref
            |  +--rw enabled?   boolean
            +--ro state
            |  +--ro name?       oc-if:base-interface-ref
            |  +--ro enabled?    boolean
            |  +--ro counters
-           |     +--ro frame-in?          yang:counter64
-           |     +--ro frame-out?         yang:counter64
-           |     +--ro frame-error-in?    yang:counter64
-           |     +--ro frame-discard?     yang:counter64
-           |     +--ro tlv-discard?       yang:counter64
-           |     +--ro tlv-unknown?       yang:counter64
-           |     +--ro last-clear?        yang:date-and-time
-           |     +--ro frame-error-out?   yang:counter64
+           |     +--ro frame-in?          oc-yang:counter64
+           |     +--ro frame-out?         oc-yang:counter64
+           |     +--ro frame-error-in?    oc-yang:counter64
+           |     +--ro frame-discard?     oc-yang:counter64
+           |     +--ro tlv-discard?       oc-yang:counter64
+           |     +--ro tlv-unknown?       oc-yang:counter64
+           |     +--ro last-clear?        oc-yang:date-and-time
+           |     +--ro frame-error-out?   oc-yang:counter64
            +--ro neighbors
               +--ro neighbor* [id]
                  +--ro id              -> ../state/id
                  +--ro config
                  +--ro state
                  |  +--ro system-name?               string
                  |  +--ro system-description?        string
                  |  +--ro chassis-id?                string
                  |  +--ro chassis-id-type?           oc-lldp-types:chassis-id-type
+                 |  +--ro management-interface?      oc-if:interface-id
                  |  +--ro id?                        string
                  |  +--ro age?                       uint64
                  |  +--ro last-update?               int64
                  |  +--ro ttl?                       uint16
                  |  +--ro port-id?                   string
                  |  +--ro port-id-type?              oc-lldp-types:port-id-type
                  |  +--ro port-description?          string
-                 |  +--ro management-address?        string
-                 |  +--ro management-address-type?   string
+                 |  x--ro management-address?        string
+                 |  x--ro management-address-type?   string
+                 +--ro management-addresses
+                 |  +--ro management-address* [address]
+                 |     +--ro address    -> ../state/address
+                 |     +--ro state
+                 |        +--ro address?                    union
+                 |        +--ro interface-number?           uint32
+                 |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
                  +--ro custom-tlvs
                  |  +--ro tlv* [type oui oui-subtype]
                  |     +--ro type           -> ../state/type
                  |     +--ro oui            -> ../state/oui
                  |     +--ro oui-subtype    -> ../state/oui-subtype
                  |     +--ro config
                  |     +--ro state
                  |        +--ro type?          int32
                  |        +--ro oui?           string
                  |        +--ro oui-subtype?   string
                  |        +--ro value?         binary
                  +--ro capabilities
                     +--ro capability* [name]
                        +--ro name      -> ../state/name
                        +--ro config
                        +--ro state
                           +--ro name?      identityref
                           +--ro enabled?   boolean

 module: openconfig-terminal-device
   +--rw terminal-device
      +--rw logical-channels
      |  +--rw channel* [index]
      |     +--rw ethernet
      |     |  +--rw lldp
      |     |     +--rw config
      |     |     |  +--rw enabled?    boolean
      |     |     |  +--rw snooping?   boolean
      |     |     +--ro state
      |     |     |  +--ro enabled?    boolean
      |     |     |  +--ro snooping?   boolean
      |     |     |  +--ro counters
-     |     |     |     +--ro frame-in?          yang:counter64
-     |     |     |     +--ro frame-out?         yang:counter64
-     |     |     |     +--ro frame-error-in?    yang:counter64
-     |     |     |     +--ro frame-discard?     yang:counter64
-     |     |     |     +--ro tlv-discard?       yang:counter64
-     |     |     |     +--ro tlv-unknown?       yang:counter64
-     |     |     |     +--ro last-clear?        yang:date-and-time
-     |     |     |     +--ro frame-error-out?   yang:counter64
+     |     |     |     +--ro frame-in?          oc-yang:counter64
+     |     |     |     +--ro frame-out?         oc-yang:counter64
+     |     |     |     +--ro frame-error-in?    oc-yang:counter64
+     |     |     |     +--ro frame-discard?     oc-yang:counter64
+     |     |     |     +--ro tlv-discard?       oc-yang:counter64
+     |     |     |     +--ro tlv-unknown?       oc-yang:counter64
+     |     |     |     +--ro last-clear?        oc-yang:date-and-time
+     |     |     |     +--ro frame-error-out?   oc-yang:counter64
      |     |     +--ro neighbors
      |     |        +--ro neighbor* [id]
      |     |           +--ro id             -> ../state/id
      |     |           +--ro config
      |     |           +--ro state
      |     |           |  +--ro system-name?               string
      |     |           |  +--ro system-description?        string
      |     |           |  +--ro chassis-id?                string
      |     |           |  +--ro chassis-id-type?           oc-lldp-types:chassis-id-type
+     |     |           |  +--ro management-interface?      oc-if:interface-id
      |     |           |  +--ro id?                        string
      |     |           |  +--ro age?                       uint64
      |     |           |  +--ro last-update?               int64
      |     |           |  +--ro ttl?                       uint16
      |     |           |  +--ro port-id?                   string
      |     |           |  +--ro port-id-type?              oc-lldp-types:port-id-type
      |     |           |  +--ro port-description?          string
-     |     |           |  +--ro management-address?        string
-     |     |           |  +--ro management-address-type?   string
+     |     |           |  x--ro management-address?        string
+     |     |           |  x--ro management-address-type?   string

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 23, 2025

The OC toolchain has a problem with the state/management-address leaf matching a list, even though the list is in a child container. Can you rename the newly introduced list? Perhaps:
mgmt-addresses/mgmt-address

F1209 22:54:52.331780 3376 generator.go:388] ERROR Generating GoStruct Code: /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/state/management-address was duplicate with /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/management-addresses/management-address

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 23, 2025

/gcbrun

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Dec 23, 2025

The OC toolchain has a problem with the state/management-address leaf matching a list, even though the list is in a child container. Can you rename the newly introduced list? Perhaps: mgmt-addresses/mgmt-address

F1209 22:54:52.331780 3376 generator.go:388] ERROR Generating GoStruct Code: /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/state/management-address was duplicate with /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/management-addresses/management-address

This sounds to me like a broken codegen assumption no? The error is indicating a conflict on 2 completely separate nodes but what is likely trying to gen a conflicting struct. This could happen in quite a few places that would impose some rules and restrictions back to the data-modeling that are not documented anywhere.

Has this been hit in any prior assumptions? How was this resolved other than trying to adjust naming which seems brute force. Does the codegen need to formulate more fully qualified struct names?

@dplore
Copy link
Copy Markdown
Member

dplore commented Dec 23, 2025

The OC toolchain has a problem with the state/management-address leaf matching a list, even though the list is in a child container. Can you rename the newly introduced list? Perhaps: mgmt-addresses/mgmt-address

F1209 22:54:52.331780 3376 generator.go:388] ERROR Generating GoStruct Code: /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/state/management-address was duplicate with /openconfig-lldp/lldp/interfaces/interface/neighbors/neighbor/management-addresses/management-address

This sounds to me like a broken codegen assumption no? The error is indicating a conflict on 2 completely separate nodes but what is likely trying to gen a conflicting struct. This could happen in quite a few places that would impose some rules and restrictions back to the data-modeling that are not documented anywhere.

Has this been hit in any prior assumptions? How was this resolved other than trying to adjust naming which seems brute force. Does the codegen need to formulate more fully qualified struct names?

I have not seen this issue before and I didn't expect it.

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Dec 23, 2025

I have not seen this issue before and I didn't expect it.

Should we open a ygot issue rather? I don't think imposing these restrictions back to the data-modeling is the correct solution and also one that will not work for other model-sets (which then means ygot compatibility issues)

@ElodinLaarz
Copy link
Copy Markdown
Contributor

Discussed at the OC Operators Meeting on Jan 6, 2026:

  1. Seems useful to open a request with ygot to better handle these issues? Blocking on a tool chain problem is surprising.
  2. We, unfortunately, kind of "do" want to block on the GitHub workflows passing, as the model change could make ygot users unable to import the updated model. (e.g. Ondatra tests that use the go structs, etc.).

Is there a rush to get this submitted? If not, we could wait until the ygot situation is resolved. (seems this was initially raised in February of last year; so, maybe not? :))

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Jan 6, 2026

1. Seems useful to open a request with ygot to better handle these issues? Blocking on a tool chain problem is surprising.

Yes and I'd have thought we should have ran into this already in other subtrees - will open an issue in the ygot repo and reference this PR

2. We, unfortunately, kind of "do" want to block on the GitHub workflows passing, as the model change could make ygot users unable to import the updated model. (e.g. Ondatra tests that use the go structs, etc.).

Is there a rush to get this submitted? If not, we could wait until the ygot situation is resolved. (seems this was initially raised in February of last year; so, maybe not? :))

Realistically, these turn around times often turn into similar of actual SDOs which defeats some of the purpose on faster delivery. Yes, consumers want this data and the more gaps in OpenConfig modeling, the more desire to look to other efforts which adds to consumer pain and perception. The longer PRs like this take, the more folks will choose native or other modeling to consume. From our perspective at this moment, it does not make any difference to wait until prereq toolchain fixes are in.

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Jan 10, 2026

@robshakir - is what you are proposing in #1424 related to the codegen issues uncovered here?

@dplore
Copy link
Copy Markdown
Member

dplore commented Jan 14, 2026

/gcbrun

@dplore
Copy link
Copy Markdown
Member

dplore commented Jan 14, 2026

@robshakir - is what you are proposing in #1424 related to the codegen issues uncovered here?

Yes, #1424 codifies a long standing rule that was never made explicit.

I expect we'll be merging that specification update after addressing the comments on the language.

To expedite this PR, can you adjust the name of the newly introduced list? Perhaps:
mgmt-addresses/mgmt-address

@earies
Copy link
Copy Markdown
Contributor Author

earies commented Jan 15, 2026

@robshakir - is what you are proposing in #1424 related to the codegen issues uncovered here?

Yes, #1424 codifies a long standing rule that was never made explicit.

I expect we'll be merging that specification update after addressing the comments on the language.

ack thx

To expedite this PR, can you adjust the name of the newly introduced list? Perhaps: mgmt-addresses/mgmt-address

Updated in latest commit per suggestion - diff tree view below

 module: openconfig-lldp
   +--rw lldp
      +--rw config
      |  +--rw enabled?                      boolean
      |  +--rw hello-timer?                  uint64
      |  +--rw suppress-tlv-advertisement*   identityref
      |  +--rw system-name?                  string
      |  +--rw system-description?           string
      |  +--rw chassis-id?                   string
      |  +--rw chassis-id-type?              oc-lldp-types:chassis-id-type
+     |  +--rw management-interface?         oc-if:interface-id
      +--ro state
      |  +--ro enabled?                      boolean
      |  +--ro hello-timer?                  uint64
      |  +--ro suppress-tlv-advertisement*   identityref
      |  +--ro system-name?                  string
      |  +--ro system-description?           string
      |  +--ro chassis-id?                   string
      |  +--ro chassis-id-type?              oc-lldp-types:chassis-id-type
+     |  +--ro management-interface?         oc-if:interface-id
      |  +--ro counters
-     |     +--ro frame-in?           yang:counter64
-     |     +--ro frame-out?          yang:counter64
-     |     +--ro frame-error-in?     yang:counter64
-     |     +--ro frame-discard?      yang:counter64
-     |     +--ro tlv-discard?        yang:counter64
-     |     +--ro tlv-unknown?        yang:counter64
-     |     +--ro last-clear?         yang:date-and-time
-     |     +--ro tlv-accepted?       yang:counter64
-     |     +--ro entries-aged-out?   yang:counter64
+     |     +--ro frame-in?           oc-yang:counter64
+     |     +--ro frame-out?          oc-yang:counter64
+     |     +--ro frame-error-in?     oc-yang:counter64
+     |     +--ro frame-discard?      oc-yang:counter64
+     |     +--ro tlv-discard?        oc-yang:counter64
+     |     +--ro tlv-unknown?        oc-yang:counter64
+     |     +--ro last-clear?         oc-yang:date-and-time
+     |     +--ro tlv-accepted?       oc-yang:counter64
+     |     +--ro entries-aged-out?   oc-yang:counter64
+     +--ro mgmt-addresses
+     |  +--ro mgmt-address* [address]
+     |     +--ro address    -> ../state/address
+     |     +--ro state
+     |        +--ro address?                    union
+     |        +--ro interface-number?           uint32
+     |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
      +--rw interfaces
         +--rw interface* [name]
            +--rw name         -> ../config/name
            +--rw config
            |  +--rw name?      oc-if:base-interface-ref
            |  +--rw enabled?   boolean
            +--ro state
            |  +--ro name?       oc-if:base-interface-ref
            |  +--ro enabled?    boolean
            |  +--ro counters
-           |     +--ro frame-in?          yang:counter64
-           |     +--ro frame-out?         yang:counter64
-           |     +--ro frame-error-in?    yang:counter64
-           |     +--ro frame-discard?     yang:counter64
-           |     +--ro tlv-discard?       yang:counter64
-           |     +--ro tlv-unknown?       yang:counter64
-           |     +--ro last-clear?        yang:date-and-time
-           |     +--ro frame-error-out?   yang:counter64
+           |     +--ro frame-in?          oc-yang:counter64
+           |     +--ro frame-out?         oc-yang:counter64
+           |     +--ro frame-error-in?    oc-yang:counter64
+           |     +--ro frame-discard?     oc-yang:counter64
+           |     +--ro tlv-discard?       oc-yang:counter64
+           |     +--ro tlv-unknown?       oc-yang:counter64
+           |     +--ro last-clear?        oc-yang:date-and-time
+           |     +--ro frame-error-out?   oc-yang:counter64
            +--ro neighbors
               +--ro neighbor* [id]
                  +--ro id              -> ../state/id
                  +--ro config
                  +--ro state
                  |  +--ro system-name?               string
                  |  +--ro system-description?        string
                  |  +--ro chassis-id?                string
                  |  +--ro chassis-id-type?           oc-lldp-types:chassis-id-type
+                 |  +--ro management-interface?      oc-if:interface-id
                  |  +--ro id?                        string
                  |  +--ro age?                       uint64
                  |  +--ro last-update?               int64
                  |  +--ro ttl?                       uint16
                  |  +--ro port-id?                   string
                  |  +--ro port-id-type?              oc-lldp-types:port-id-type
                  |  +--ro port-description?          string
-                 |  +--ro management-address?        string
-                 |  +--ro management-address-type?   string
+                 |  x--ro management-address?        string
+                 |  x--ro management-address-type?   string
+                 +--ro mgmt-addresses
+                 |  +--ro mgmt-address* [address]
+                 |     +--ro address    -> ../state/address
+                 |     +--ro state
+                 |        +--ro address?                    union
+                 |        +--ro interface-number?           uint32
+                 |        +--ro interface-number-subtype?   oc-lldp-types:mgmt-interface-number-subtype
                  +--ro custom-tlvs
                  |  +--ro tlv* [type oui oui-subtype]
                  |     +--ro type           -> ../state/type
                  |     +--ro oui            -> ../state/oui
                  |     +--ro oui-subtype    -> ../state/oui-subtype
                  |     +--ro config
                  |     +--ro state
                  |        +--ro type?          int32
                  |        +--ro oui?           string
                  |        +--ro oui-subtype?   string
                  |        +--ro value?         binary
                  +--ro capabilities
                     +--ro capability* [name]
                        +--ro name      -> ../state/name
                        +--ro config
                        +--ro state
                           +--ro name?      identityref
                           +--ro enabled?   boolean

@dplore
Copy link
Copy Markdown
Member

dplore commented Jan 15, 2026

/gcbrun

1 similar comment
@dplore
Copy link
Copy Markdown
Member

dplore commented Jan 16, 2026

/gcbrun

@dplore dplore merged commit 2e304c6 into openconfig:master Jan 16, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants