Re: [OPSAWG] AD review of draft-ietf-opsawg-sap-09

[[e-mail]]

Hi Rob, 

Thank you for the review. The changes can be tracked at: https://tinyurl.com/sap-latest

Please note that I made a change to better allow for reuse of the SAP information in other modules (this can be tracked here: https://github.com/IETF-OPSAWG-WG/lxnm/commit/e8b406d7fad5225dd84ad7ff03f6da446eae6736). 

Please see inline for more context. 

Cheers,
Med

> -----Message d'origine-----
> De : Rob Wilton (rwilton) <[e-mail]>
> Envoyé : vendredi 23 septembre 2022 14:01
> À : [e-mail]; [e-mail]
> Objet : AD review of draft-ietf-opsawg-sap-09
> 
> Hi authors, WG,
> 
> Thank you for this document.  I also think that this document is
> well written and in good shape, and I mostly found the
> explanations and examples clear.  There were two specific points
> that I found slightly confusing related to differentiating between
> SAPs in use, and those that are not, and also parent interfaces
> also be listed as SAPs, details below.

[Med] Thanks

> 
> Moderate level comments:
> 
> (1) p 10, sec 5.  SAP Module Tree Structure
> 
>       SAPs that are associated with the interfaces that are
> directly
>       hosting services, interfaces that are ready to host per-
> service
>       sub-interfaces (but not yet activated), or service that are
>       already instantiated on sub-interfaces are listed as SAPs.
> 
> >From the model, is isn't clear to me how different differentiates
> between a SAP that has been pre-provisioned to potentially be used
> for a service in future, v.s., one is that is actively
> provisioned.

[Med] This is inferred from the service-status=down for these SAPs.


  I think that it would be helpful if these two cases
> can be clearly distinguished.  Note, I have made a similar comment
> related to appendix D about identifying a "free" SAP.

[Med] Added this NEW to the appendix: 

"SAPs that are ready to host per-service (but not yet activated) are identified by the "service-status" set to "ietf-vpn-common:op-down"."

> 
> 
> (2) p 28, sec Appendix B.  A Simple Example of SAP Network Model:
> Node Filter
> 

[Med] Please note "Simple" in the title :-)

>    A service orchestrator can query what services are provided on
> which
>    SAPs of PE1 from the network controller by sending, e.g., a GET
>    RESTCONF request.  Figure 9 shows the body of the RESTCONF
> response
>    that is received from the network controller.
> 
> In this example, you have GE0/6/4 as being ready to host SAPs, but
> I could equally imagine that given GE0/6/4 is just a UNI, then you
> may not want the parent interface to be available to host SAPs
> directly at all.  I.e., it may always the case that sub-interfaces
> are used as SAPs.  Hence, did you consider whether it makes sense
> for there to be a different category of SAP service-types for
> UNI's and NNI's that don't host services directly on the
> interface, but always on sub-interfaces?

[Med] Yes, we do need this for the LxVPN case. 

  Related to this, it
> feels slightly strange to me to have GE0/6/4 listed under both
> l3vpn and vpls SAP lists.

[Med] Actually there are attached to distinct sub-interfaces:   

   Also, let us assume that, for
   the SAPs that are associated with the physical interface "GE0/6/4",
   VPLS and L3VPN services are activated on the two sub-interfaces
   "GE0/6/4.1" and "GE0/6/4.2", respectively.

Updated the example to align with this text. 

  Having the same information twice in
> the data model introduces the risk that a device could export
> inconsistent information and hence it hard for the customer to
> know which data is accurate.  I can understand why having it twice
> might be convenient, but having an indication that it is actually
> just a container node for SAPs rather than a SAP itself might be
> better.
> 
> 
> 
> Minor level comments:
> 
> (3) p 3, sec 3.  Sample SAP Network Model Usage
> 
>    Management operations of a service provider network can be
> automated
>    using a variety of means such as interfaces based on YANG
> modules
>    [RFC8969].
> 
> Would a reference to NETCONF and potentially RESTCONF be helpful
> here in addition to RFC8969?
> 

[Med] They are indirectly cited via 8969, but does not harm to have explicit pointers.

> 
> (4) p 4, sec 3.  Sample SAP Network Model Usage
> 
>    The service orchestration layer does not need to know about the
>    internals of the underlying network (e.g., P nodes).
> 
> Would "all the internals" be better than just "internals".  I.e.,
> presumably the service orchestration layer probably does need to
> know about some of the internals of the underlying network.
> 

[Med] Indeed.

> 
> (5) p 10, sec 5.  SAP Module Tree Structure
> 
>    These service types build on the types that are already defined
> in
>    [RFC9181] and additional types that are defined in this
> document.
>    Other service types can be defined in future YANG modules, if
> needed.
> 
> In future YANG modules, or perhaps also future versions of the
> YANG model defined in this document?
> 

[Med] Changed to "future YANG modules (including future revisions of the YANG model defined in this document)"

> 
> (6) p 11, sec 5.  SAP Module Tree Structure
> 
>       This data node can be used, for example, to decide whether
> an
>       existing SAP can be (re)used to host a service or if a new
> sub-
>       interface has to be instantiated.
> 
> So, is this field effectively also being used to determine if the
> SAP is in use?
> 

[Med] No, this is determined by 'sap-status' (completed with 'service-status'). 


> 
> (7) p 12, sec 5.  SAP Module Tree Structure
> 
>       When both a sub-interface and its parent interface are
> present,
>       the status of the parent interface takes precedence over the
>       status indicated for the sub-interface.
> 
> This seems strange to me.  E.g., if the parent-interface was up,
> but the sub-interface was administratively down then would you be
> expected to report the sap-status as up?  I would think that this
> should always be reporting the sub-interface state, but that the
> sub-interface state should inherit
> 

[Med] Good catch. "but the parent interface is disabled" was missing from the OLD text. 

> 
> (8) p 16, sec 6.  SAP YANG Module
> 
>      identity logical {
>        base interface-type;
>        description
>          "Refers to a logical sub-interface that is typically
>           used to bind a service. This type is used only
>           if none of the other logical types can be used.";
>      }
> 
> Perhaps clarify what you mean by "other logical types".  E.g., do
> you just mean loopback or irb, or does this include local-bridge
> as well?
> 
> 

[Med] We meant the other non-PHY types already defined. Changed to: 

      "Refers to a logical sub-interface that is typically
       used to bind a service. This type is used only
       if none of the other types (i.e., loopback, lag, 
       irb, or local-bridge) can be used.";


> (9) p 17, sec 6.  SAP YANG Module
> 
>          leaf peer-sap-id {
>            type string;
>            description
>              "Indicates an identifier of the peer's termination
>               identifier (e.g., Customer Edge (CE)). This
>               information can be used for correlation purposes,
>               such as identifying the SAP that is attached to
>               an endpoint that is provided in a service request.";
>          }
> 
> Would it be helpful to indicate that the "peer-sap-id" is not
> necessarily the same as the peer's sap_id for that same attachment
> circuit endpoint?  E.g., it appears to differ in your example in
> Appendix C.
> 

[Med] Given that this is used for correlation purposes, the value enclosed in peer-sap-id is useful when it is known to the peer. Typically, that value will be indicated as the service delivery point. I tend to leave the description as it is.  

> 
> (10) p 19, sec 8.  Security Considerations
> 
>    *  /nw:networks/nw:network/nw:node/sap:service-type/sap:sap
> 
> Should this be:
> /nw:networks/nw:network/nw:node/sap:service/sap:sap ?
> 

[Med] This should be "/nw:networks/nw:network/nw:node/sap:service/sap:service-type/sap:sap"

> 
> (11) p 20, sec 8.  Security Considerations
> 
>    *  /nw:networks/nw:network/nw:node/sap:service-type/sap:sap
> 
> Should this be:
> /nw:networks/nw:network/nw:node/sap:service/sap:sap ?
> 
> 
> (12) p 25, sec Appendix A.  A Simplified SAP Network Example
> 
>    An example of a SAP topology that is reported by a network
> controller
>    is depicted in Figure 7.  This example echoes the topology
> shown in
>    Figure 4.  Only a minimum set of information is provided for
> each
>    SAP.
> 
> I'm surprised that service-status isn't reported for the saps that
> only have names.  Perhaps that information is elided to make the
> examples shorter, but it may be helpful to clarify that.  E.g.,
> perhaps expand "Only a minimum set of information is provided for
> each
>    SAP." to be explicit about SAP information that has been elided
> (e.g., attachment interface, interface type, role).  Or perhaps a
> bit more of an explanation about how different SAPS are being
> modelled here would be helpful

[Med] That was the intent of having "Only a minimum set of information is provided for each SAP.", but I hear this is no that clear. Added the status and a mention about the nodes that are listed for the sake of illustration. 


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.