Re: [Gen-art] Genart last call review of draft-ietf-opsawg-vpn-common-09

[[e-mail]]

Hi Joel, 

Thank you for the review. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Joel Halpern via Datatracker [mailto:[e-mail]]
> Envoyé : vendredi 30 juillet 2021 18:38
> À : [e-mail]
> Cc : [e-mail]; [e-mail];
> [e-mail]
> Objet : Genart last call review of draft-ietf-opsawg-vpn-common-09
> 
> Reviewer: Joel Halpern
> Review result: Ready with Issues
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed by
> the IESG for the IETF Chair.  Please treat these comments just like
> any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-opsawg-vpn-common-09
> Reviewer: Joel Halpern
> Review Date: 2021-07-30
> IETF LC End Date: 2021-08-06
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: This document is ready for publication as a Proposed
> Standard RFC
> 
> Major issues: N/A
> 
> Minor issues:
>     I just want to confirm that one form of reference is normal for
> YANG
>     models?  There are a few identities whose meaning is defined by
> I-Ds that
>     are under way.  The references section of the identity gives the
> I-D name.
>     Which is fine.  The references at the bottom of the document
> makes those
>     informative references.  That seems a little odd since those
> references are
>     necessary to understand the meaning of those identities.  Is
> this a normal
>     practice for YANG models?

[Med] I confirm. We are following this part from RFC8407:

    " If a YANG module
      contains reference or "description" statements that refer to an
      I-D, then the I-D is included as an informative reference. "

>      I am a little confused as to why the srv6 identity includes in
> its
>      references clause RFC 8663 (MPLS SR).  Is this a copy-and-paste
> error?

[Med] Please note that we are referring RFC8663, not RFC8660. This is because we assumed that RFC8663 uses IPv6 as an underlay, but that's may be confusing. 

I suggest to go with this change: 

OLD:
  identity srv6 {
    base sr;
    description
      "Transport is based on SR over IPv6.";
    reference
      "RFC 8663: MPLS Segment Routing over IP
       RFC 8754: IPv6 Segment Routing Header (SRH)";
  }

NEW:
  identity srv6 {
    base sr;
    description
      "Transport is based on SR over IPv6.";
    reference
      "RFC 8754: IPv6 Segment Routing Header (SRH)";
  }

  identity sr-mpls-over-ip {
    base sr;
    description
      "Transport is based on SR over MPLS over IP.";
    reference
      "RFC 8663: MPLS Segment Routing over IP";
  }

Please let me know if this is better. Thanks. 

>     I hope I am misreading the qos-classification-policy definition.
> It
>     appears to have an id, a match rule, and a match action.   The
> match rule
>     can be a match-flow or a match-application.  So far, so good.
> (putting
>     aside the nit below on customer-application.)   But the match
> rule is a
>     choice between an L3 and an L4 rule.

[Med] This is not a choice between these two, but each of them is a choice in its own. FWIW, here is an excerpt from RFC8340 to distinguish between choices and cases:

==
       <name> is the name of the node
         (<name>) means that the node is a choice node
        :(<name>) means that the node is a case node
==  

Things would be problematic if we defined L3/L4 as "cases" of the same choice, but we aren't.

  As I understand it, there
> are many
>     cases where the actual classification is based on a combination
> of l3 and
>     l4 information.  How is that to be represented?

[Med] An example to illustrate how both can be included in shown below (DNS traffic destined to 2001:db8::/32)

A valid rule for DNS traffic for example is shown below:

{
  "id": "a-rule-id",
  "ipv6": {
    "destination-ipv6-network": "2001:db8::/32"
  },
  "udp": {
    "destination-port-range-or-operator": {
      "operator": "eq",
      "port": 53
    }
  }
}

> 
> Nits/editorial comments:
>     The "customer-application" identity seems to be asking for
> problems in two
>     regards.

[Med] FYI, we inherited this from service modules (RFC8299 and RFC8466) where these common identities are used.

    It seems that it is a shorthand way of expressing
> some
>     combination of protocols and ports.

[Med] This can be indeed expressed as such, but at more likely at the network level. We are covering both as the common module can be used by both service and network models.  

   The larger concern I have
> is that
>     there is no reference that explains what classification rules
> should be
>     used for any of the derived identities.   Which means that for
> an
>     interoperable implementation, there seems to be some difficulty
> in ensuring
>     that the client and server mean the same thing when using them.
> (For
>     example, just what filer corresponds to "voice"?)  As a lesser
> issue, the
>     current IAB work on path signals and the care that should be
> taken with
>     them would seem to suggest that this is a less than desirable
> approach to
>     the problem.
> 
> 


_________________________________________________________________________________________________________________________

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.