Subject: [OPSAWG] Benjamin Kaduk's No Objection on draft-ietf-opsawg-vpn-common-10: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 21 Sep 2021 21:18:57 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-opsawg-vpn-common-10: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-opsawg-vpn-common/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I have a bold proposal for consideration: since we are defining a new
common set of groupings for VPN and, in some cases, proposing to rename
existing terminology already, can we consider moving away from the term
"VPN" itself?  The word "private" is ambiguous as to what level of
privacy is being provided (e.g., network routing vs cryptographic) and
who the conveyed content is to be private from.  A term like "virtual
network" removes that ambiguity, and lets us use the explicit attributes
to convey whether encryption is in use when appropriate.

There is no particular triggering point (at least, not yet) at which it
becomes clearly correct to make a breaking change like this, so we may
end up just having to pick a time somewhat arbitrarily, if we are to
make such a change at all.  Perhaps now is that time; perhaps not.

Section 3

          grouping vpn-profile-cfg
            +-- valid-provider-identifiers
               +-- external-connectivity-identifier* [id]
               |       {external-connectivity}?
               |  +-- id?   string

Presumably I am just misreading RFC 8340, but I thought that the list
key was implicitly mandatory, so it would appear as just "id" (as
opposed to "id?").  (Many instances.)

   'vpn-route-targets':
      *  A YANG grouping that defines Route Target (RT) import/export
         rules used in a BGP-enabled VPN (e.g., [RFC4364][RFC4664]).

On very quick skim, it's not clear what motivates the RFC 4664
reference -- "route target" does not appear, for example, nor does
"import" or "export".

Section 4

     identity pim-proto {
       if-feature "pim";
       base routing-protocol-type;

This is in the section with the group-management-protocols; is
"routing-protocol-type" really the intended base?

     identity embb {
     [...]
     identity urllc {
     [...]
     identity mmtc {

Similar to Roman's comment, a reference seems useful for these.
If we intend to implicitly rely on RFCs 8299 and/or 8466 for
terminology, we should say so in the terminology section.

       leaf vpn-id {
         type vpn-common:vpn-id;
         description
           "A VPN identifier that uniquely identifies a VPN.
            This identifier has a local meaning, e.g., within
            a service provider network.";

Thank you for indicating the scope of validity of the identifier!
(Elsewhere as well.)

     grouping service-status {
       [...]
           leaf last-change {
             type yang:date-and-time;
             description
               "Indicates the actual date and time of the service status
                change.";

What's the motiviation behind leaving this as "config true"?  When would
it be useful to set it manually?

       list vpn-target {
         [...]
         leaf id {
           type int8;
           description
             "Identifies each VPN Target.";

I wasn't able to find the motivation for limiting to int8 in RFC 4364,
though I mostly assume I'm just looking in the wrong place.
(But why *signed* int8?)

Section 5

While there may be no direct security considerations from what is
effectively just defining some new compound types for reuse, I think we
may want to consider some privacy considerations.  For example, we have
the "customer-name" field in the vpn-description, and it is sometimes
the case that customers want to remain confidential.  Thus, any
instantiations of the grouping would incur a potential need for
confidentiality and minimizing the scope of distribution.

NITS

Section 4

     feature bearer-reference {
       description
         "Indicates support for the bearer reference access constraint.
          That is, the reuse of a network connection that was already
          ordered to the service provider apart from the IP VPN site.";

I echo Roman's comment that this feature would benefit from a reference
or further discussion; the current description leaves me unclear on what
is meant and with no recourse for learning more.  (RFCs 4026 and 4176
are presented as generic references for VPN-related terms, but do not
cover the concept of "bearer reference".)

       reference
         "I-D.ietf-teas-ietf-network-slice-framework:
            Framework for IETF Network Slices";

draft-ietf-teas-ietf-network-slice-framework is replaced by
draft-ietf-teas-ietf-network-slices.

     identity rsvp-te {
       base protocol-type;
       description
         "Transport is based on RSVP-TE.";
       reference
         "RFC 3209: RSVP-TE: Extensions to RSVP for LSP Tunnels";

Is the transport itself RSVP-TE, or is it that RSVP-TE is used to
provision the tunnels used as transport?  (Similar question for bgp-lu?)

     identity dot1q {
       if-feature "dot1q";
       base encapsulation-type;
       description
         "Dot1q encapsulation.";

I suppose the feature declaration does so, but maybe some "802" is in
order here as well?

     identity ethernet-type {
       base encapsulation-type;
       description
         "Ethernet encapsulation type.";
     }

     identity vlan-type {
       base encapsulation-type;
       description
         "VLAN encapsulation.";
     }

     identity untagged-int {
       base encapsulation-type;
       description
         "Untagged interface type.";
     [...]

Should we be more consistent about whether the description ends in
"type"?

     identity lag-int {
       if-feature "lag-interface";
       base encapsulation-type;
       description
         "LAG interface type.";
       reference
         "IEEE Std. 802.1AX: Link Aggregation";

lag-int is the only encapsulation-type identify element that has a
reference stanza.  We did mention LAG in the context of 802.1AX earlier,
so maybe it's not needed?

     identity web {
       base customer-application;
       description
         "Web applications (e.g., HTTP, HTTPS).";
     [...]
     identity file-transfer {
       base customer-application;
       description
         "File transfer application (e.g., FTP, SFTP).";

Maybe we could just list HTTPS and SFTP as the (respective) examples, in 2021?

       leaf vpn-name {
         type string;
         description
           "A name used to refer to the VPN.";

Should we say a little more about how the name differs from the id,
given that both are "type string"?

         leaf import-policy {
           type string;
           description
             "Defines the 'import' policy.";
         }
         leaf export-policy {
           type string;
           description
             "Defines the 'export' policy.";

Does it "define" or merely "identify" the policy?  Naively, a
"definition" would be a rather long string...

     grouping vpn-components-group {
       [...]
       container groups {
         description
           "Lists the groups to which a VPN node,a site, or a network

space after comma.

---

From: <[email-masked]>
Date: Wed, 22 Sep 2021 09:40:17 +0000

Hi Ben, 

Thank you for the review. The changes to address your review can be tracked at: https://github.com/IETF-OPSAWG-WG/lxnm/commit/108707e8b2aea6590b0a9695756c7546887c9614 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk via Datatracker [mailto:[email-masked]]
> Envoyé : mercredi 22 septembre 2021 06:19
> À : The IESG <[email-masked]>
> Cc : [email-masked]; [email-masked];
> [email-masked]; [email-masked]; [email-masked]
> Objet : Benjamin Kaduk's No Objection on draft-ietf-opsawg-vpn-common-
> 10: (with COMMENT)
> 

[[Med]] I hear some of your observations but VPN is a well-established name for a widely deployed technology. I don't think it is a good idea to touch this. Please note that there is already generic model for VNs (https://datatracker.ietf.org/doc/draft-ietf-teas-actn-vn-yang/) that is distinct from what we are doing here.

> opposed to "id?").  (Many instances.)

[[Med]] Good question. Actually we trusted what is in Section 3.2 of 8340. That's the output we get from pyang. Note that ? is not displayed when the grouping is used in another model (L2NM/L3NM). Will double check this with Martin.  

> On very quick skim, it's not clear what motivates the RFC 4664 reference
> -- "route target" does not appear, for example, nor does "import" or
> "export".

[[Med]] This is just to insist that it can be used for both L3 and L2VPNs. Unlike L3NM, there are plenty L2VPN technologies; hence the choice to point to a generic L2 framework.

> This is in the section with the group-management-protocols; is "routing-
> protocol-type" really the intended base?

[[Med]] Yes. Both PIM and other group management protocols fall under:

  /*
   * Identities related to multicast
   */ 

> terminology, we should say so in the terminology section.

[[Med]] Added this note under the terminology section:

   The document inherits many terms from [RFC8299] and [RFC8466]
   (e.g., Enhanced Mobile Broadband (eMBB), Ultra-Reliable and Low
   Latency Communications (URLLC), Massive Machine Type Communications
   (mMTC)).

> it be useful to set it manually?

[[Med]] This can be set, for example, when a new administrative status is set. 

> though I mostly assume I'm just looking in the wrong place.

[[Med]] I confirm that there is no such concept in 4364. The intuitive design would be to use leaf-list, but the reason for having this is a list with an id is recorded in this note:

         Note that this is modelled as a list to ease the reuse of this
         grouping in modules where a pointer is needed (e.g., associate
         an operator with RTs).

We don't see the need to go beyond uint8. As illustrated, for example, in L3NM a typical usage of this would be simply:

==
                         "vpn-targets": {
                           "vpn-target": [
                             {
                               "id": "1",
                               "route-targets": [
                                 "0:65550:1"
                               ],
                               "route-target-type": "both"
                             }
                           ]
                         }
==

> (But why *signed* int8?)

[[Med]] This should be unsigned. Fixed. 

> confidentiality and minimizing the scope of distribution.

[[Med]] We do already discuss this for example in the L3NM. No problem to add a note here as well. I added this NEW text:

   Modules that use the groupings that are defined in this document
   should identify the corresponding security considerations.  For
   example, reusing some of these groupings will expose privacy-related
   information (e.g., customer-name).  Disclosing such information may
   be considered as a violation of the customer-provider trust
   relationship. 

> cover the concept of "bearer reference".)

[[Med]] The description is basically echoing RFC8299: 

==
   o  The "bearer-reference" parameter is used in cases where the
      customer has already ordered a network connection to the SP apart
      from the IP VPN site and wants to reuse this connection.
== 

We can update the description to first introduce the concept of bearer:

      "A bearer refers to properties of the CE-PE attachment that
       are below Layer 3.

> draft-ietf-teas-ietf-network-slice-framework is replaced by draft-ietf-
> teas-ietf-network-slices.

[[Med]] Shame on me as I'm contributing to that work :-( Fixed. Thanks.

> provision the tunnels used as transport?  (Similar question for bgp-lu?)

[[Med]] This means "relies upon" to setup the underlay transport. 

OLD:
  Transport is based on
NEW:
 Transport setup relies upon

> I suppose the feature declaration does so, but maybe some "802" is in

[[Med]] We didn't as we though this is redundant with the reference under the dot1q feature:

    reference
      "IEEE Std 802.1Q: Bridges and Bridged Networks"; 

> Should we be more consistent about whether the description ends in

[[Med]] Indeed. Fixed. 

> so maybe it's not needed?

[[Med]] Right. We should be consistent. 

> Maybe we could just list HTTPS and SFTP as the (respective) examples, in

[[Med]] These are only examples. I tend to keep the same description as in RFC8299 as this is related to the customer traffic. 

> given that both are "type string"?

[[Med]] The main difference is provided as part of the "id" description: the id is what is used to uniquely identify the VPN. 

That's said, I updated the description for better clarity as follows: 

NEW:
        "A name used to associate a name with the service
         in order to facilitate the identification of 
         the service";

> "definition" would be a rather long string...

[[Med]] "identifies" is more appropriate. Fixed. 

>            "Lists the groups to which a VPN node,a site, or a network

[[Med]] Fixed. Thank you. 

_________________________________________________________________________________________________________________________

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.

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 27 Sep 2021 14:38:57 -0700

Hi Med,

> Thank you for the review. The changes to address your review can be tracked at: https://github.com/IETF-OPSAWG-WG/lxnm/commit/108707e8b2aea6590b0a9695756c7546887c9614 

Thanks.  Also inline...

> [[Med]] I hear some of your observations but VPN is a well-established name for a widely deployed technology. I don't think it is a good idea to touch this. Please note that there is already generi…

Well, I don't think that just because VPN is well-established we should
ignore the ambiguity in what it describes.  So, I still want to touch it at
some point, but do not insist on doing so right now.  I am sure that we can
find some non-VPN term other than VN if there is desire to
distinguish...but we can defer that conversation, of course.

> [[Med]] Good question. Actually we trusted what is in Section 3.2 of 8340. That's the output we get from pyang. Note that ? is not displayed when the grouping is used in another model (L2NM/L3NM).…

Thanks.

> [[Med]] This is just to insist that it can be used for both L3 and L2VPNs. Unlike L3NM, there are plenty L2VPN technologies; hence the choice to point to a generic L2 framework.

Okay, so the references are just generic "VPN" (or "BGP-enabled VPN")
references, and I was misreading to ascribe more meaning to them.

If we wanted to spend a few more words (far from clear), maybe "(suitable
VPN technologies include but are not limited to [RFC4364][RFC4664])".

>    Latency Communications (URLLC), Massive Machine Type Communications

Thanks!

> [[Med]] This can be set, for example, when a new administrative status is set. 

But would it be set to a time other than the time when the new
administrative status was set, i.e., a value that can be automatically
determined?

> We don't see the need to go beyond uint8. As illustrated, for example, in L3NM a typical usage of this would be simply:

Okay.  I am not sure if "pointer" or "index" is better for conveying the
desired meaning ("pointer" might suggest "leafref" to some readers).

>                                "route-targets": [

Thanks, that looks good.

>       "A bearer refers to properties of the CE-PE attachment that

Thanks; combined with the note in the intro about 8299 terminology that
should be enough for people to figure it out.

And thanks for the other updates as well,

Ben

---

From: <[email-masked]>
Date: Tue, 28 Sep 2021 06:39:44 +0000

Hi Ben, 

Thank you for the follow-up. 

Please see inline. 

Cheers,
Med

> De : Benjamin Kaduk <[email-masked]>
> Envoyé : lundi 27 septembre 2021 23:39
> À : BOUCADAIR Mohamed INNOV/NET <[email-masked]>
> Cc : The IESG <[email-masked]>; [email-masked];
> [email-masked]; [email-masked]; [email-masked]
> Objet : Re: Benjamin Kaduk's No Objection on draft-ietf-opsawg-vpn-common-
> 10: (with COMMENT)
> 
> Hi Med,
> 
> > Thank you for the review. The changes to address your review can be
> > tracked at:
> > https://github.com/IETF-OPSAWG-WG/lxnm/commit/108707e8b2aea6590b0a9695
> > 756c7546887c9614
> 
> Thanks.  Also inline...
> 
> 
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > De : Benjamin Kaduk via Datatracker [mailto:[email-masked]] Envoyé
> > > : mercredi 22 septembre 2021 06:19 À : The IESG <[email-masked]> Cc :
> > > [email-masked]; [email-masked];
> > > [email-masked]; [email-masked]; [email-masked] Objet :
> > > Benjamin Kaduk's No Objection on draft-ietf-opsawg-vpn-common-
> > > 10: (with COMMENT)
> > >
> > > When responding, please keep the subject line intact and reply to
> > > all email addresses included in the To and CC lines. (Feel free to
> > > cut this introductory paragraph, however.)
> > >
> > >
> > > I have a bold proposal for consideration: since we are defining a
> > > new common set of groupings for VPN and, in some cases, proposing to
> > > rename existing terminology already, can we consider moving away
> > > from the term "VPN" itself?  The word "private" is ambiguous as to
> > > what level of privacy is being provided (e.g., network routing vs
> > > cryptographic) and who the conveyed content is to be private from.
> > > A term like "virtual network" removes that ambiguity, and lets us
> > > use the explicit attributes to convey whether encryption is in use
> when appropriate.
> > >
> > > There is no particular triggering point (at least, not yet) at which
> > > it becomes clearly correct to make a breaking change like this, so
> > > we may end up just having to pick a time somewhat arbitrarily, if we
> > > are to make such a change at all.  Perhaps now is that time; perhaps
> not.
> >
> > [[Med]] I hear some of your observations but VPN is a well-established
> name for a widely deployed technology. I don't think it is a good idea to
> touch this. Please note that there is already generic model for VNs
> (https://datatracker.ietf.org/doc/draft-ietf-teas-actn-vn-yang/) that is
> distinct from what we are doing here.
> 
> ignore the ambiguity in what it describes.  So, I still want to touch it
> at some point, but do not insist on doing so right now.  I am sure that we
> can find some non-VPN term other than VN if there is desire to

[Med] Sure, we can always find better names, but I think it is better to have this discussion when revising documents such as RFC4026. Thank you. 

> > > Presumably I am just misreading RFC 8340, but I thought that the
> > > list key was implicitly mandatory, so it would appear as just "id"
> > > (as opposed to "id?").  (Many instances.)
> >
> > [[Med]] Good question. Actually we trusted what is in Section 3.2 of
> 8340. That's the output we get from pyang. Note that ? is not displayed
> when the grouping is used in another model (L2NM/L3NM). Will double check
> this with Martin.
> 
> Thanks.
> 
> > >
> > >    'vpn-route-targets':
> > [[Med]] This is just to insist that it can be used for both L3 and
> L2VPNs. Unlike L3NM, there are plenty L2VPN technologies; hence the choice
> to point to a generic L2 framework.
> 

[Med] OK. Changed the text to: "This grouping can be used for both L3VPNs [RFC4364] and L2VPNs [RFC4664]."

> > > "routing- protocol-type" really the intended base?
> >
> > > What's the motiviation behind leaving this as "config true"?  When
> > > would it be useful to set it manually?
> >

[Med] Yes, think about service calendaring scenarios. 

> > > I wasn't able to find the motivation for limiting to int8 in RFC
> > > 4364, though I mostly assume I'm just looking in the wrong place.
> >
> > [[Med]] I confirm that there is no such concept in 4364. The intuitive
> design would be to use leaf-list, but the reason for having this is a list
> with an id is recorded in this note:
> >
> > We don't see the need to go beyond uint8. As illustrated, for example,
> in L3NM a typical usage of this would be simply:
> 

[Med] Changed to "where an RT identifier". 

> > > effectively just defining some new compound types for reuse, I think
> > > we may want to consider some privacy considerations.  For example,
> > > we have the "customer-name" field in the vpn-description, and it is
> > > sometimes the case that customers want to remain confidential.
> > > Thus, any instantiations of the grouping would incur a potential
> > > need for confidentiality and minimizing the scope of distribution.
> > >
> >
> > [[Med]] We do already discuss this for example in the L3NM. No problem
> to add a note here as well. I added this NEW text:
> >
> > > I echo Roman's comment that this feature would benefit from a
> > > reference or further discussion; the current description leaves me
> > > unclear on what is meant and with no recourse for learning more.
> > > (RFCs 4026 and 4176 are presented as generic references for
> > > VPN-related terms, but do not cover the concept of "bearer
> > > reference".)
> >