Re: [OPSAWG] Last Call: <draft-ietf-opsawg-service-assurance-yang-09.txt> (YANG Modules for Service Assurance) to Proposed Standard

[tom petch <[e-mail]>]

On 28/11/2022 12:28, Jean Quilbeuf wrote:
> Hi Tom,
> Thanks for your comments, we tried to address them (see below).
>
> The diff of the changes is here:           https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-service-assurance-yang-10

I think that this still has some quirks.

In several places an unrestricted string is used as a key which means 
that the key can be 18446744073709551615 characters long.  Perhaps worth 
a mention in e.g. Security Considerations as an attack surface.

the description of container agents has a non-ASCII character in it

identifyin a routing protocol instance with an unrestricted string seems 
onerous

Tom Petch

>
> Best,
> Jean
>
>> -----Original Message-----
>> From: tom petch [mailto:[e-mail]]
>> Sent: Wednesday 9 November 2022 12:04
>> To: [e-mail]
>> Cc: [e-mail]; [e-mail];
>> [e-mail]; [e-mail]
>> Subject: Re: Last Call: <draft-ietf-opsawg-service-assurance-yang-09.txt>
>> (YANG Modules for Service Assurance) to Proposed Standard
>>
>> On 08/11/2022 10:13, The IESG wrote:
>>>
>>> The IESG has received a request from the Operations and Management
>>> Area Working Group WG (opsawg) to consider the following document: -
>>> 'YANG Modules for Service Assurance'
>>>     <draft-ietf-opsawg-service-assurance-yang-09.txt> as Proposed
>>> Standard
>>>
>>> The IESG plans to make a decision in the next few weeks, and solicits
>>> final comments on this action. Please send substantive comments to the
>>> [e-mail] mailing lists by 2022-11-22. Exceptionally,
>>> comments may be sent to [e-mail] instead. In either case, please
>>> retain the beginning of the Subject line to allow automated sorting.
>>
>> I have never seen a YANG module with so few references and the one and
>> only one there is is missing from the I-D Normative References.
>
> RFC6991 is now a normative reference.
>
>> I think at least there should be a reference to the companion Architecture
>> document and wonder if there is a non-IETF literature around for this topic
>> which could be referenced.
>
> There is a reference to the architecture document in version 09. Do you mean that we should reference it from the YANG model itself?
> For the non-IETF literature, this is covered by the architecture document which refers to https://doi.org/10.1016/B978-0-12-803773-7.00007-3  (I’ll add that URL to the next revision of the -architecture draft as it’s not straightforward to find the DOI link).
>
>> I was going to comment on the lack of explanation of what a type is but see
>> that as a weakness of the architecture and so have commented thereon on
>> that I-D, which could then be a further reference for this I-D.
>
> What we call "subservice type" is what you call "function" in your next comment. I have tried to clarify that in this draft as well.
>
>> My impression from Appendix B is that this could spawn a large number of
>> related YANG modules for different functions  such as 'device'.  A registry of
>> such functions giving a canonical set of names seems a likely need.  This I-D
>> could REQUIRE all such I-D to use a YANG prefix of sain-....
>
> I re-added an old section about Guidelines to add new types of subservices that covers this point.
>
>
>>     identity service-instance-type {
>>         "Identity representing a service instance."; service instance or service
>> instance type?
>
> Changed to:
>     "Specific type of subservice that represents a service
>        instance. Instance of this type will depend on other subservice
>        to build the top of the assurance graph."
>
>
>> XPath within a grouping without a prefix leaves me wondering if that prefix
>> will be needed e.g. by additional type modules.
>
> Indeed, pyang complains when trying to import the grouping. I inlined the symptoms grouping and precised that others groupings where not meant to be imported.
>
>> stop-date-time
>> What if the symptom is ongoing?
>
> Specified:
>           "Date and time at which the symptom stopped being
>                detected. must after the start-date-time. If the symptom
>                is ongoing, this field should not be populated." ;
>
>
>> 'must (be) after' could be a YANG constraint.
>
> Tried to do that but XPATH1.0 does not have date manipulation function. Maybe I missed something?
>
>>         leaf id {
>>           type string;
>>           description
>>             "Identifier of the subservice instance. Must be unique...
>> YANG string can be very, very long and can contain all sorts of characters.
>> This is a recurrent problem with YANG (which did not adopt the SMI
>> approach) and came up -again - on the Netmod list in October but without a
>> clear resolution IMHO, just that the current situation is ....
>> well, unsatisfactory.
>
> Agreed, however, I don’t see which restrictions we can safely take without being in the way of implementors.  Any leads?
>
>    typedef yang-identifier {
>           type string {
>             length "1..max";
>             pattern '[a-zA-Z_][a-zA-Z0-9\-_.]*';
>             pattern '.|..|[^xX].*|.[^mM].*|..[^lL].*';
>           }
>
>>         leaf label {
>>           type string;
>>           config false;
>>           description
>>             "Label of the subservice, i.e., text describing what the
>>              subservice is to be displayed on a human interface.
>> Again, without constraint this could be a nonsense.  At least one AD is keen
>> on the I18N implications of display strings (which was an issue with I2NSF I-
>> D).
>
> We put the following to avoid the language tag issues:
>          "  It is not intended for random end users but for
>             network/system/software engineers that are able to interpret
>             it. Therefore, no mechanism for language tagging is needed.";"
>
> In general, do you have an suggestions on how to solve the unrestricted string issue?
>
>>
>>           leaf contact {
>>             type string;
>> Here there is some guidance but only as to the semantics - I suspect guidance
>> on the length and character set e.g. would be useful.
>>
>
> Unrestricted string
>
>>             leaf service {
>>               type string;
>> ...
>>             }
>>             leaf instance-name {
>>               type string;
>> Again unqualified string
>
> Unrestricted string
>
>>
>>         leaf service {
>>           type leafref {
>>             path "/subservices/subservice/service-instance-parameter/"
>>                + "service";
>>           }
>>           description
>>             "Name of the service type.";
>> The more I read the more confused I become.  'service' has just been
>> defined as the name of the service; how can it be 'service type' here?
>> As I have said, the use of 'type' in general needs more attention.
>
> Updated the descriptions of this part of the YANG module to remove "type".
>
>>         list instances {
>>           key "name";
>>           description
>>             "Instances of the parent service type. The list must contain
>>              an entry for every instance of the parent service.";
>>           leaf name {
>>             type leafref {
>>               path
>>                 "/subservices/subservice/service-instance-parameter/"
>>               + "instance-name";
>> Another string as a key; vide supra.
>
>
>>              identifier (device id, hostname, management IP) depends
>> Is that an e.g. or an i.e.?
>
> Added an e.g.
>
>>
>> s.5.3
>>            leaf interface {
>>              type string;
>>              mandatory true;
>>              description
>>                "Name of the interface.";
>> As above, unconstricted string.  This could be a leafref in order to
>> reference the YANG interface module; most RFC do just that.
>
> I would love to have a leafref, however don’t forget that the NETCONF server being configured runs on the SAIN agent, which might not be the device. Also not all devices support ietf-interfaces (assuming that’s the one you refer to). So which YANG model do we do the leafref to?
>
> Let’s assume we add the leafref to ietf-interfaces (interfaces/interface/name), then when adding an interface subservice into the SAIN agent, we must refer to an interface that exists in the agent configuration.
>
> I see two possible implementations here:
> 1 - ietf-interfaces is not bound to anything on the agent side, just open for configuration. In that case, adding an interface subservice will only be accepted if an interface of that name is defined on the agent. Since ietf-interfaces does not put any restriction on the name (type: string), we can add any name we want, so equivalent to what we have currently, just more annoying to configure.
> 2 - ietf-interfaces is mirroring what we have on the devices monitored by the agent. That means mapping whatever YANG model/SNMP MIB/show command the devices supports to ietf-interfaces.
> Problem 1: an agent can monitor several devices so we need to augment/mount the ietf-interfaces to specify to which device the interface belongs.
> Problem 2: we introduce a mandatory mapping to ietf-interfaces, which requires some implementation effort, especially if the assurance frameworks relies internally on another model
>
> Referencing leaves that are not in the YANG context (i.e. imported modules) is a well-known YANG problem.
>
>> Tom Petch
>>
>>> Abstract
>>>
>>>
>>>      This document specifies YANG modules for representing assurance
>>>      graphs.  These graphs represent the assurance of a given service by
>>>      decomposing it into atomic assurance elements called subservices.  A
>>>      companion document, Service Assurance for Intent-based Networking
>>>      Architecture, presents an architecture for implementing the assurance
>>>      of such services.
>>>
>>>      The YANG data models in this document conforms to the Network
>>>      Management Datastore Architecture (NMDA) defined in RFC 8342.
>>>
>>>
>>>
>>>
>>> The file can be obtained via
>>> https://datatracker.ietf.org/doc/draft-ietf-opsawg-service-assurance-
>> yang/
>>>
>>>
>>> The following IPR Declarations may be related to this I-D:
>>>
>>>      https://datatracker.ietf.org/ipr/3859/
>>>
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> IETF-Announce mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/ietf-announce
>>> .
>>>