Re: [yang-doctors] Yangdoctors early review of draft-ietf-teas-sf-aware-topo-model-08

[Xufeng Liu <[e-mail]>]

Hi Andy,

Would you still like to do a verification on this?
Thanks,
- Xufeng

On Wed, May 4, 2022 at 12:56 PM Andy Bierman <[e-mail]> wrote:

> Hi,
>
> I reviewed the diffs between -08 and -09 and they look good.
> I will do a complete review of -09 soon. I think all my comments
> have been addressed but need to verify.
>
>
> Andy
>
>
> On Fri, Mar 18, 2022 at 4:08 PM Xufeng Liu <[e-mail]>
> wrote:
>
>> Hi Andy,
>>
>> Thanks a lot for the review and comments. We have posted an updated
>> revision
>> https://datatracker.ietf.org/doc/html/draft-ietf-teas-sf-aware-topo-model-09,
>> trying to address the comments, with some explanations below in-line.
>>
>> Thanks,
>> - Xufeng
>>
>> On Tue, Dec 14, 2021 at 3:50 PM Andy Bierman via Datatracker <
>> [e-mail]> wrote:
>>
>>> Reviewer: Andy Bierman
>>> Review result: Ready with Issues
>>>
>>>
>>> Draft:    draft-ietf-teas-sf-aware-topo-model-08.txt
>>> Module:   ietf-te-topology-sf
>>> Revision: 2021-06-28
>>>
>>>
>>> General:
>>>
>>> This document is well written and understandable, even though
>>> the topic is complex.
>>>
>>>
>>> Issues:
>>>
>>> 1)
>>> Not clear why
>>> information-source-entry/service-function/link-terminations/link-termination/from
>>> is just an empty NP-container. Description says "Reference to the link
>>> termination point.";
>>> Not clear how this empty NP-container is a reference to anything.
>>
>> [Xufeng]: Thanks for pointing this out. A leaf was mistakenly missing.
>> Fixed.
>>
>>>
>>> 2)
>>> The "service-function-id" and "sf-connection-point-id" leafs are each
>>> used in 4 places.
>>> Perhaps use groupings instead of cut-and-paste?
>>>
>> [Xufeng]: Right. A grouping is better, and added.
>>
>>
>>> The type is a plain string. Is this really what is desired?
>>> It is not clear if these leafs are administrative labels or real linkage.
>>> The description is (e.g.) "Reference to a network service or a network
>>> function."
>>> This implies some linkage to another YANG object somewhere, but no
>>> mention of any
>>> such objects. Should this be a leafref to a specific node (or union of
>>> leafrefs
>>> to multiple objects)?
>>>
>> [Xufeng]: Fixed by using leafref statements.
>>
>>>
>>> 3)
>>> The service-function key (id) is also a plain string.
>>> Should the type "yang-identifier" be used instead?
>>> Or maybe a different common type.  Is is not unusual to limit the length
>>> of string identifiers provided by a client.  Obvious;y a server can
>>> return a "too-big" error but it might be useful to pick a length
>>> every server must support (e.g. 1..64).
>>>
>> [Xufeng]: Such IDs are not defined within IETF. The usually accepted
>> definition is from ETSI (
>> https://www.etsi.org/deliver/etsi_gs/NFV-IFA/001_099/007/03.05.01_60/gs_NFV-IFA007v030501p.pdf).
>> There are multiple existing data models, which may be YANG, or TOSCA, or
>> others. We are not trying to limit the service-function implementation to a
>> particular model, so a service-function model is not referenced here. ETSI
>> does not impose any restriction on the format and length, so this topic was
>> discussed and the type string was chosen.
>>
>>>
>>>
>>> Module:   ietf-te-topology-sf-state
>>> Revision: 2021-06-28
>>>
>>> This is a non-NMDA version of the ietf-te-topology-sf module.
>>> It appears to completely correct and no new issues exist.
>>>
>>> (There should be a YANG extension to tell compilers and other tools
>>> that module foo-state is really the non-NMDA variant of foo, and
>>> not just a different module with a name that matches a common pattern.
>>> Not an issue for this draft.)
>>>
>>>
>>>
>>>
>>
>> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=icon> Virus-free.
>> www.avast.com
>> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=link>
>> <#m_-6266487045898824426_m_442652490399504781_m_-7613731194565471283_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>
>