Subject: Re: [OPSAWG] Shepherd writeup for draft-ietf-opsawg-vpn-common

From: tom petch <[email-masked]>
Date: Sat, 15 May 2021 11:38:17 +0000

From: OPSAWG <[email-masked]> on behalf of Adrian Farrel <[email-masked]>
Sent: 13 April 2021 14:01

The write-up can be seen in the datatraker at
https://datatracker.ietf.org/doc/draft-ietf-opsawg-vpn-common/.  I'd be
happy to address any comments.

I recommend that this document is held until the L3NM draft is also ready so
that they can proceed together, although this is not strictly required.

<tp>
I am finally getting through the tsunami of I-D that flooded me prior to the last closure of the I-D window:-( and have some comments on this I-D, mostly editorial such as features without reference clauses which some Yang Doctors (and I) regard as a no-no but some mildly technical, such as the use of three identity when everyone else uses two (e.g. dualstack as opposed to YANG derived-from functions).  I can post them now, or at least next week, or wait until the Last Call starts, whichever is preferred.

Tom Petch

Best,
Adrian

_______________________________________________
OPSAWG mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/opsawg

---

From: "Adrian Farrel" <[email-masked]>
Date: Sat, 15 May 2021 12:44:02 +0100

Hi Tom,

I'm sure the chairs will have an opinion.

My view is that it is always best to post comments when you have them. Don't
be surprised if the authors batch them with IETF last call, but they might
address them and update the draft sooner if they can slot it in and
especially if your comments mean that subsequent reviews would be smoother.

Best,
Adrian

-----Original Message-----
From: tom petch <[email-masked]> 
Sent: 15 May 2021 12:38
To: [email-masked]; [email-masked]
Cc: [email-masked]
Subject: Re: [OPSAWG] Shepherd writeup for draft-ietf-opsawg-vpn-common

From: OPSAWG <[email-masked]> on behalf of Adrian Farrel
<[email-masked]>
Sent: 13 April 2021 14:01

<tp>
I am finally getting through the tsunami of I-D that flooded me prior to the
last closure of the I-D window:-( and have some comments on this I-D, mostly
editorial such as features without reference clauses which some Yang Doctors
(and I) regard as a no-no but some mildly technical, such as the use of
three identity when everyone else uses two (e.g. dualstack as opposed to
YANG derived-from functions).  I can post them now, or at least next week,
or wait until the Last Call starts, whichever is preferred.

Tom Petch

Best,
Adrian

=

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Sat, 15 May 2021 15:42:44 +0000

> I am finally getting through the tsunami of I-D that flooded me prior to the last closure of the I-D window:-( and have some comments on this I-D, mostly editorial such as features without referenc…

Hey, Tom.  Please post your comments now to give the authors, shepherd,
and WG more time to digest and reply.  Thanks for your continued reviews.

Joe

---

From: tom petch <[email-masked]>
Date: Mon, 17 May 2021 09:32:53 +0000

Some belated comments.

Mildly technical:

- there is often a need to model this version or that version or both
which YANG supports with a base identity 'version' from which 'this' and
'that' are derived and used with the functions 'derived-from' and
'derived-from-or-self'.  Almost every WG uses this for IP when the
alternatives are IPv4 or IPv6 or both, but not here,.  Here a third
identity has been defined for both.  This will, I think, puzzle and
confuse users and potentially lead to mistakes.  This should be changed
IMHO.

'ie-type' same idea, ditch the third identity and use the YANG
functions.

'identity qos-profile-direction' ditto

I note that l3sm-l3nm makes extensive use of these YANG functions so
clearly users have to be comfortable with using them.

feature ipv6
What does this mean?  Support for data transport, for routing protocols.
for   ...  Needs explaining.  This also may need to cater for hybrids,
such as an IPv6 routing protocol that carries IPv4 next hop addresses;
IPv4, IPv6, both, neither?

feature ospf
I note that the references are to OSPFv2.  l3sm-l3nm references OSPFv3
as in RFC6565 which suggests a base ospf identity is needed from which
ospfv2 and ospfv3 are derived, as is common with other IETF I-D both for
IP and OSPF and ....!

More editorial

Some YANG Doctors require all features to have references which I think
an excellent idea.  Here some 17 do not and most I think should.  A
'feature' statement stands in isolation, unlike a 'leaf'  or 'list', and
so is more in need of a reference to give context to make sense of what
the feature is.  Ditto most YANG type definitions.

IEEE documents are referenced in the YANG module but without a date;
this is ambiguous.  I note that the I-D References do correctly have
dates.

  feature rtg-vrrp {
...
       Protocol (VRRP) between a cutsomer LAN and the PE.";
perhaps 'customer' or better still, for consistency,
       Protocol (VRRP) as the PE/CE routing protocol

  identity multipoint {
...
      "Identity for multipoint connections, e.g., Ethernet broadcast
       segment.";
Ethernet is multiaccess, rather than multipoint.

  identity operational-status {
  identity administrative-status {
The derived identity identifiers are quite cumbersome.  There is a lack
of consistency across the IETF but usually they are shorter, just 'up',
'down', 'testing', unknown' the context supplying the rest

level1, level2, level1-2
When I look at protocol documents, e.g.
  draft-ietf-isis-yang-isis-cfg
I see level-1-2; I like consistency, less chance to make mistakes!  A
quick search suggests that level1... is not used in any IETF I-D/RFC,
always level-1... etc

  identity site-to-wan {
...
      "Identity for Site-to-WAN direction.";
Is there a difference between PE/CE and site/WAN?  If so, it needs
explaining, if not consistency is good.

  identity mld-version {
I suspect that there is a need to bundle IGMP and MLD together ie have a
common base identity.  I tend to think of them as one as I see in many I-Ds such
as
draft-ietf-pim-igmp-mld..

  typedef ccm-priority-type {
lacks a reference and a specification of what is high, what is low
priority.

Tom Petch

________________________________________
From: Joe Clarke (jclarke) <[email-masked]>
Sent: 15 May 2021 16:42
Subject: Re: [OPSAWG] Shepherd writeup for draft-ietf-opsawg-vpn-common

Joe

---

From: <[email-masked]>
Date: Mon, 17 May 2021 13:59:26 +0000

Hi Tom, 

Thank you for the comments. Much appreciated. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : OPSAWG [mailto:[email-masked]] De la part de tom petch
> Envoyé : lundi 17 mai 2021 11:33
> À : Joe Clarke (jclarke) <[email-masked]>; [email-masked];
> [email-masked]
> Cc : [email-masked]
> Objet : [OPSAWG] Comments was:Shepherd writeup for draft-ietf-opsawg-
> vpn-common
> 
> Some belated comments.
> 
> Mildly technical:
> 
> which YANG supports with a base identity 'version' from which 'this'
> and 'that' are derived and used with the functions 'derived-from' and
> confuse users and potentially lead to mistakes.  This should be
> changed IMHO.

[Med] We used to have only v4 and v6 in previous versions of the spec but this had the implication to define a list of address families each time we wanted to cover both as well. Things are simplified with the "both" value. 

Users will use whatever allowed values defined in the common module.  

> 'ie-type' same idea, ditch the third identity and use the YANG

[Med] Idem as above.

> 'identity qos-profile-direction' ditto

[Med] Idem as above. 

> such as an IPv6 routing protocol that carries IPv4 next hop
> addresses; IPv4, IPv6, both, neither?

[Med] The support of IPv6 in a VPN means that IPv6 traffic can conveyed in the VPN, IPv6 prefixes exchanged in the CE/PE link, IPv6 static routes can be created, etc.

The same description is used for both IPv4 and IPv6. Any particular reason why this is an issue for IPv6, not for IPv4?

> 
> feature ospf
> I note that the references are to OSPFv2. 

[Med] Either both should be referenced or none. Will fix that one. 

 l3sm-l3nm references
> OSPFv3 as in RFC6565 which suggests a base ospf identity is needed
> from which
> ospfv2 and ospfv3 are derived, as is common with other IETF I-D both
> for IP and OSPF and ....!
> 
> 
> More editorial
> 
> Some YANG Doctors require all features to have references

[Med] We had two yangdoctors reviews in the past. All was clean so far in that front. 

 which I
> think an excellent idea.  Here some 17 do not and most I think
> should.  A 'feature' statement stands in isolation, unlike a 'leaf'
> or 'list', and so is more in need of a reference to give context to
> make sense of what the feature is.  Ditto most YANG type definitions.

[Med] We did our best to include references when we have something "meaningful" to cite. If you have in mind valid references for any of those lacking a ref, please share it with us. We will update the module accordingly. 

> this is ambiguous.

[Med] We followed what was done in other RFCs, see for example RFC8944.

  I note that the I-D References do correctly have
> dates.
> 
>   feature rtg-vrrp {
> ...
>        Protocol (VRRP) between a cutsomer LAN and the PE."; perhaps
> 'customer' or better still, for consistency,

[Med] Sure. Will fix this one. 

> Ethernet is multiaccess, rather than multipoint.

[Med] You are right. FYI, this was borrowed from RFC8299. Will fix the description. 

> The derived identity identifiers are quite cumbersome.  There is a
> lack of consistency across the IETF but usually they are shorter,
> just 'up', 'down', 'testing', unknown' the context supplying the rest

[Med] The intent was to differentiate the admin vs ops status, hence the prefix and this long names. I don't see an issue out there. 

> quick search suggests that level1... is not used in any IETF I-D/RFC,

[Med] Can change it although what matters is the description, not the data node name.

> 
>   identity site-to-wan {
> ...
>       "Identity for Site-to-WAN direction."; Is there a difference
> between PE/CE and site/WAN?  If so, it needs explaining, if not
> consistency is good.

[Med] There might be multiple CEs on the same site and the same CE may be connected to multiple PEs. Please remember that this model can be also used for service models, not only network ones. 

> 
>   identity mld-version {
> I suspect that there is a need to bundle IGMP and MLD together ie
> have a common base identity.

[Med] Why? These are two distinct protocols that can be managed separately. 

  I tend to think of them as one as I see
> in many I-Ds such as draft-ietf-pim-igmp-mld..

[Med] I guess you meant RFC8652. They actually have distinct nodes for IGMP and MLD. Their design is suboptimal as the version is frozen by the "range" statement:

==
     grouping interface-common-config-attributes-igmp {
       description
         "Configuration attributes applied to both the interface-global
          level and interface level for IGMP.";
       uses interface-common-config-attributes;
       leaf version {
         type uint8 {
           range "1..3";
         }
         description
           "IGMP version.
            The default value is 2.
            This leaf may be configured at the interface level or the
            interface-global level, with precedence given to the value
            at the interface level.  If the leaf is not configured at
            either level, the default value is used.";
         reference
           "RFC 1112: Host Extensions for IP Multicasting,
            RFC 2236: Internet Group Management Protocol, Version 2,
            RFC 3376: Internet Group Management Protocol, Version 3.";
       }
     }

     grouping interface-common-config-attributes-mld {
       description
         "Configuration attributes applied to both the interface-global
          level and interface level for MLD.";
       uses interface-common-config-attributes;
       leaf version {
         type uint8 {
           range "1..2";
         }
         description
           "MLD version.
            The default value is 2.
            This leaf may be configured at the interface level or the
            interface-global level, with precedence given to the value
            at the interface level.  If the leaf is not configured at
            either level, the default value is used.";
         reference
           "RFC 2710: Multicast Listener Discovery (MLD) for IPv6,
            RFC 3810: Multicast Listener Discovery Version 2 (MLDv2)
            for IPv6.";
       }
     }
==

> lacks a reference and a specification of what is high, what is low

[Med] FYI, this was borrowed from RFC8466 as it is. Will see if we can add a ref. 

> > https://datatracker.ietf.org/doc/draft-ietf-opsawg-vpn-common/.
> I'd
> > be happy to address any comments.
> >
> > I recommend that this document is held until the L3NM draft is also
> > ready so that they can proceed together, although this is not
> strictly required.
> >
> > <tp>
> > I am finally getting through the tsunami of I-D that flooded me
> prior to the last closure of the I-D window:-( and have some comments
> on this I-D, mostly editorial such as features without reference
> clauses which some Yang Doctors (and I) regard as a no-no but some
> mildly technical, such as the use of three identity when everyone
> else uses two (e.g. dualstack as opposed to YANG derived-from
> functions).  I can post them now, or at least next week, or wait
> until the Last Call starts, whichever is preferred.
> 
> Hey, Tom.  Please post your comments now to give the authors,
> shepherd, and WG more time to digest and reply.  Thanks for your
> continued reviews.
> 
> Joe
> 
> 

_________________________________________________________________________________________________________________________

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: tom petch <[email-masked]>
Date: Tue, 18 May 2021 10:55:39 +0000

From: [email-masked] <[email-masked]>
Sent: 17 May 2021 14:59

Hi Tom,

Please see inline.

Cheers,
Med

<tp>
Disagree; they are more complicated!

It means that when you want to specify ipv4 you have to specify a list, ipv4 and dualstack as happens in several places in opsawg-l3sm-l3nm with constructs such as 
                         when "../address-family = 'vpn-common:ipv4' or "
                           + "'vpn-common:dual-stack'" {
 If ipv4 meant ipv4 then this gets simpler, obvious.  Ditto ipv6.  And when you want both, you get it with 'derived-from-or-self' on the base ip identity from which ipv4 and iv6 are derived.
As I say, this is what I see everyone else doing so doing it differently can only confuse users.

My comment on the implication  of feature ipv6 applies equally to ipv4; it they mean all aspects thereof, then I think that that needs spelling separately.

I will post separately references for features.

Tom Petch

[Med] Idem as above.

[Med] Idem as above.

 l3sm-l3nm references

 which I

==

---

From: <[email-masked]>
Date: Tue, 18 May 2021 11:04:03 +0000

Hi Tom, 

Please see inline. 

Cheers,
Med

> De : tom petch [mailto:[email-masked]]
> Envoyé : mardi 18 mai 2021 12:56
> À : BOUCADAIR Mohamed TGI/OLN <[email-masked]>; Joe
> Clarke (jclarke) <[email-masked]>; [email-masked];
> [email-masked]
> Objet : Re: [OPSAWG] Comments was:Shepherd writeup for draft-ietf-
> opsawg-vpn-common
> 
> [Med] We used to have only v4 and v6 in previous versions of the spec
> but this had the implication to define a list of address families
> each time we wanted to cover both as well. Things are simplified with
> the "both" value.
> 
> <tp>
> It means that when you want to specify ipv4 you have to specify a
> list, ipv4 and dualstack
 as happens in several places in opsawg-l3sm-
> l3nm with constructs such as
>                            + "'vpn-common:dual-stack'" {  If ipv4
> meant ipv4 then this gets simpler, obvious.  Ditto ipv6.  And when
> you want both, you get it with 'derived-from-or-self' on the base ip
> identity from which ipv4 and iv6 are derived.

[Med] Thanks, but still don't see any specific issue with the current design.

> 
> My comment on the implication  of feature ipv6 applies equally to
> ipv4; it they mean all aspects thereof, then I think that that needs
> spelling separately.

[Med] Done as you can see in https://tinyurl.com/vpn-common-latest 

> I will post separately references for features.

[Med] Thanks. Please take into account those that I already added to address your review: https://tinyurl.com/vpn-common-latest.

---

From: tom petch <[email-masked]>
Date: Tue, 18 May 2021 11:49:43 +0000

Hi Tom,

Please see inline.

Cheers,
Med

<tp>
I said that references were lacking for features and that I might suggest some.

lag-interface
IEEE802.1AX

rtg-bgp
RFC4271

rtg-rip
RFC2453

rtg-isis
ISO-10589

fast-reroute
RFC5714
but there are different flavours of this and I am unclear which is intended

qos
the description talks of cos which is different.  I note that RFC8466 is similar in approach but does have a discussion thereof so that could be a reference; or IEEE802.1p and/or IETF's DSCP for which there are lots of references or ...

qinany
don't know, IEEE802.1 somewhere no doubt

Tom Petch

---

From: <[email-masked]>
Date: Tue, 18 May 2021 12:22:11 +0000

Re-,

Please see inline.

Cheers,
Med

> Envoyé : mardi 18 mai 2021 13:50
> Objet : Re: [OPSAWG] TReferences: was Comments was:Shepherd writeup
> for draft-ietf-opsawg-vpn-common
> 

[Med] Fixed. Thanks. 

> ISO-10589

[Med] The reason why we don't list those is that, unlike OSPF cited RFCs, these ones do not include VPN CE/PE specifics. There some I-Ds out there, but these are expired and can't list them. See for example, draft-sheng-isis-bgp-mpls-vpn. 

> but there are different flavours of this and I am unclear which is

[Med] This one is specific to link state IGPs, which excludes repair mechanisms when other types are used. What is really important here is the description of the feature.  

> 
> qos
> the description talks of cos which is different.  I note that RFC8466

[Med] ... and also RFC8299. We maintained the same name to ease the mapping with these two are revised. Will keep the name as it is. 

> is similar in approach but does have a discussion thereof so that
> could be a reference; or IEEE802.1p and/or IETF's DSCP for which
> there are lots of references or ...
> 
> qinany

[Med] We discussed this one during the WGLC. Please check the thread with Dhruv. We failed to find an acceptable authoritative reference to cite for this one.

---

From: tom petch <[email-masked]>
Date: Tue, 18 May 2021 16:04:39 +0000

Sent: 18 May 2021 13:22
Re-,

Please see inline.

Cheers,
Med

[Med] Fixed. Thanks.

<tp>

But then you leave people with no idea where to go to understand level-1 (or how that term should be spelt:-).  I-D such as l3sm-l3nm are configuring all these routing protocols and most of that has nothing to do with VPN.  A user wanting to understand level-12 will struggle, even if they do know that it is usually spelt level-1-2; likewise they will not find keep-alive since BGP spells it keepalive:-(

The references may not be as good as RFC4577 for OSPF v2 but they help a user along, and may solve all their problems if the are comfortable with the VPN but it is the routing protocol itself they are struggling with.

Tom Petch

---

From: <[email-masked]>
Date: Tue, 18 May 2021 17:07:49 +0000

Re-,

Please see inline. 

Cheers,
Med

> Envoyé : mardi 18 mai 2021 18:05
... 
> >
> > rtg-bgp
> > RFC4271
> >
> > rtg-rip
> > RFC2453
> >
> > rtg-isis
> > ISO-10589
> >
> 
> [Med] The reason why we don't list those is that, unlike OSPF cited
> RFCs, these ones do not include VPN CE/PE specifics. There some I-Ds
> out there, but these are expired and can't list them. See for
> example, draft-sheng-isis-bgp-mpls-vpn.
> 
> <tp>
> 
> But then you leave people with no idea where to go to understand
> level-1 

[Med] I have no problem to add a reference for isis-level identity. I thought we are discussing features.  

(or how that term should be spelt:-).

[Med] There is no convention for the naming. What is really important is the description. 

  I-D such as l3sm-l3nm
> are configuring all these routing protocols and most of that has
> nothing to do with VPN.  A user wanting to understand level-12 will
> struggle, even if they do know that it is usually spelt level-1-2;

[Med] I'm afraid that we are over-exaggerating here. The L3NM has the following... which is clear this is about is-is level: 

                      leaf level {
                        type identityref {
                          base vpn-common:isis-level;
                        }
                        description
                          "Can be level1, level2, or level1-2.";
                      }

As shown in the diff I already shared with you, level1/levl2/level1-2 were changed to level-1/levl-2/level-1-2. That change will be reflected in the L3NM.  

FWIW, draft-ietf-isis-yang-isis-cfg has the following:

     typedef level {
       type enumeration {
         enum "level-1" {
           description
             "This enum indicates L1-only capability.";
         }
         enum "level-2" {
           description
             "This enum indicates L2-only capability.";
         }
         enum "level-all" {
           description
             "This enum indicates capability for both levels.";
         }
       }
       default "level-all";
       description
         "This type defines IS-IS level of an object.";

     }

> likewise they will not find keep-alive since BGP spells it
> keepalive:-(

[Med] Really?! There is no such a thing in RFC4271, but there is KeepaliveTimer or KEEPALIVE message. 

Again, what matters is the description. We do currently have the following: 

 "This timer indicates the KEEPALIVE
  messages'  frequency between a PE
  and a BGP peer..." 

> 
> The references may not be as good as RFC4577 for OSPF v2 but they
> help a user along, and may solve all their problems if the are
> comfortable with the VPN but it is the routing protocol itself they
> are struggling with.

[Med] Understood. Will see how to make things better. Thanks.