search

tsaad-dev / te

IETF TE Tunnels YANG models
16 stars 19 forks source link

Some Med's Minor WG LC comments #290

Closed italobusi closed 1 month ago

italobusi commented 3 months ago

Minor comments

I would avoid changing the structure of sections to ease mapping with 8776. For example, I would keep 1.1/1.2 as it was in 8776.

Italo: Ok, done

Acronyms: order them in an alphabetic order. Not sure the reasoning of the current ordering.

Italo: Ok, done

Include trees with -print-groupings or a pointer to where to retrieve the full trees.

Italo: added appendix with the generated YANG tree

The tree in 3.2 is not aligned with the YANG module: the bandwidth tree indicates that CIR/CBS are optional (with ?), while these are tagged as mandatory in the module.

Fixed

I have doubts about many of enums (e.g., path-type). Please check that these are justified vs using identities.

Most of the enumerations are from RFC 8776 and changing them would be an NBC change

The path-type should be an enumeration since it is follows the structure of the TE tunnel model. It can and shall be updated only when new types of paths are defined, through standard actions, in the TE tunnel model.

The 'te-generic-node-id' should also be an enumeration. It can and shall be updated only when new formatting for the te-gen-node-id data type are defined, through standard actions, in the TE types model .

MEF 6.2 EVC Ethernet Services Definitions Phase 3 - MEF : standard superseded. Please double check this reference.

I cannot find any reference to MEF 6.2

Originally posted by @italobusi in https://github.com/tsaad-dev/te/issues/279#issuecomment-2243651404_

italobusi commented 2 months ago

Not all the changes have been implemented in PR #280

Minor comments

I would avoid changing the structure of sections to ease mapping with 8776. For example, I would keep 1.1/1.2 as it was in 8776.

Italo: Ok, done

Acronyms: order them in an alphabetic order. Not sure the reasoning of the current ordering.

Italo: Ok, done

Still TBD

Include trees with -print-groupings or a pointer to where to retrieve the full trees.

Italo: added appendix with the generated YANG tree

Still TBD

The tree in 3.2 is not aligned with the YANG module: the bandwidth tree indicates that CIR/CBS are optional (with ?), while these are tagged as mandatory in the module.

Fixed

I have doubts about many of enums (e.g., path-type). Please check that these are justified vs using identities.

Most of the enumerations are from RFC 8776 and changing them would be an NBC change

The path-type should be an enumeration since it is follows the structure of the TE tunnel model. It can and shall be updated only when new types of paths are defined, through standard actions, in the TE tunnel model.

The 'te-generic-node-id' should also be an enumeration. It can and shall be updated only when new formatting for the te-gen-node-id data type are defined, through standard actions, in the TE types model .

MEF 6.2 EVC Ethernet Services Definitions Phase 3 - MEF : standard superseded. Please double check this reference.

I cannot find any reference to MEF 6.2

Originally posted by @italobusi in #279 (comment)_