Fix ospf lsdb router-lsa link list

[dplore]

Fixes #1074

Operational use case

OSPF router-lsa describes a list of link-ids, but the OC model only allows for a single link id. This PR moves the link-id and it's attributes to a new list inside of the router-lsa.

Change Scope

• Set OSPF lsdb router-lsa /network-instances/network-instance/protocols/protocol/ospfv2/areas/area/lsdb/lsa-types/lsa-type/lsas/lsa/router-lsa/state/link-id to status obsolete
• Add a links/link [link-id] list to the router-lsa
• Move types-of-service into the link list
• This change is not backwards compatible. This change proposes that since the router-lsa model is broken as is, making a breaking change here is appropriate. Comments appreciated if the existing link-id leaf should be deprecated instead of deleted.

Also note this proposes using the yang status obsolete for the leaf. OpenConfig has not traditionally used this status.
Consider this a solicitation for comments if we should start using obsolete for 'deleted' leafs, versus removing them from the model entirely.

Tree view

--- /Users/dloher/old-tree.txt  2024-03-21 10:52:00
+++ /Users/dloher/ospf-tree.txt 2024-03-21 13:40:35
@@ -5878,18 +5878,23 @@
         |     |        |              |  +--ro age?                  uint16
         |     |        |              +--ro router-lsa
         |     |        |              |  +--ro state
-        |     |        |              |  |  +--ro type?                 identityref
-        |     |        |              |  |  +--ro link-id?              yang:dotted-quad
-        |     |        |              |  |  +--ro link-data?            union
-        |     |        |              |  |  +--ro metric?               oc-ospf-types:ospf-metric
-        |     |        |              |  |  +--ro number-links?         uint16
-        |     |        |              |  |  +--ro number-tos-metrics?   uint16
-        |     |        |              |  +--ro types-of-service
-        |     |        |              |     +--ro type-of-service* [tos]
-        |     |        |              |        +--ro tos      -> ../state/tos
+        |     |        |              |  |  +--ro metric?         oc-ospf-types:ospf-metric
+        |     |        |              |  |  +--ro number-links?   uint16
+        |     |        |              |  +--ro links
+        |     |        |              |     +--ro link* [link-id]
+        |     |        |              |        +--ro link-id             -> ../state/link-id
         |     |        |              |        +--ro state
-        |     |        |              |           +--ro tos?      uint8
-        |     |        |              |           +--ro metric?   oc-ospf-types:ospf-metric
+        |     |        |              |        |  +--ro link-id?              yang:dotted-quad
+        |     |        |              |        |  +--ro type?                 identityref
+        |     |        |              |        |  +--ro link-data?            union
+        |     |        |              |        |  +--ro number-tos-metrics?   uint16
+        |     |        |              |        +--ro types-of-service
+        |     |        |              |           +--ro type-of-service* [tos]
+        |     |        |              |              +--ro tos      -> ../state/tos
+        |     |        |              |              +--ro state
+        |     |        |              |                 +--ro tos?      uint8
+        |     |        |              |                 +--ro metric?   oc-ospf-types:ospf-metric
         |     |        |              +--ro network-lsa
         |     |        |              |  +--ro state
         |     |        |              |     +--ro network-mask?      uint8

[OpenConfigBot]

No major YANG version changes in commit ae1fb73

[robshakir]

What's the motivation for obsolete vs deprecated?

[dplore]

What's the motivation for obsolete vs deprecated?

In this case it's because the link-id leaf is not valid. Per OSPF implementation, router-lsa must contain a list of all the directly connected links of a given router. This is nearly always going to be > 1. So the current OC model is 'broken' and I suspect has not been implemented? I guess someone might have implemented this and chose to place just one of the router-lsa links?

[robshakir]

What's the motivation for obsolete vs deprecated?

In this case it's because the link-id leaf is not valid. Per OSPF implementation, router-lsa must contain a list of all the directly connected links of a given router. This is nearly always going to be > 1. So the current OC model is 'broken' and I suspect has not been implemented? I guess someone might have implemented this and chose to place just one of the router-lsa links?

Whilst I see the argument here -- it seems that the two things are the same -- i.e., this leaf is no longer in the model. If I'm a developer now implementing the model whether it's obsolete or deprecated is the same thing -- "do not implement this leaf" :-)

From a tooling perspective though, we now need to ensure that all toolchains support both variants -- which are the same "don't generate for this leaf" where is code, documentation etc.

I prefer to limit our use of YANG "twiddles" to the absolute minimum possible -- supporting them in toolchains has been painful.

[dplore]

Whilst I see the argument here -- it seems that the two things are the same -- i.e., this leaf is no longer in the model. If I'm a developer now implementing the model whether it's obsolete or deprecated is the same thing -- "do not implement this leaf" :-)

From a tooling perspective though, we now need to ensure that all toolchains support both variants -- which are the same "don't generate for this leaf" where is code, documentation etc.

I prefer to limit our use of YANG "twiddles" to the absolute minimum possible -- supporting them in toolchains has been painful.

No worries, I'll deprecate the node instead. The node seems not usable as is, but the new node which replaces it doesn't conflict, so it seems there is little harm to leaving this node present in a deprecated form until a future major OC release. I appreciate any thoughts on this approach as well.

FWIW, deprecated means supported, but scheduled for deletion in an future release. Obsolete is the same as deleting the node from the model, but leaves the information what what the node used to be intact. I proposed obsolete here as I was curious about whether we should use 'obsolete' as we do the keyword 'reserved' in proto. Of course, the same issue with field numbering doesn't exist in yang (except perhaps with enums).

[dplore]

Upon reviewing with OC operators, we realized that there are many breaking changes here due to moving the leafs for links into the link list and moving the types of service list underneath the new link list. Since router-lsa is currently a broken node, it seems to make sense to just delete the link-id as well and make this a breaking change.

[dplore]

@DonaMaria2024 for your review

[dplore]

@DonaMaria2024 please provide any comments. I am waiting for an OC reviewer to give the final LGTM and then this can move to last call.

[dplore]

Adding to last-call. This will be merged on May 30, 2024

[DonaMaria2024]

Metric is the cost of a router-link which is a per link attribute.
Metric should be moved inside links/link [link-id] list.

[marcushines]

hmm is this an extra uses that is no longer necessary - i didn't notice this before that there are now 2 metrics?

[DonaMaria2024]

Hi,

Please find comments from [email protected] below.

1. /router-lsa/state should have 2 leafs
   • V,E B flags
   • Number-of-links
2. There should be a metric under
   /router-lsa/links/link

Thanks,
Dona