Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)

[[e-mail]]

Hi Rob, 

The draft is now updated to address you point (9) on tag operations. The changes can be tracked using the same diff: https://tinyurl.com/l2nm-latest

The candidate version takes into account all your comments. 

Cheers,
Med

> -----Message d'origine-----
> De : OPSAWG <[e-mail]> De la part de
> [e-mail]
> Envoyé : mardi 5 avril 2022 12:40
> À : Rob Wilton (rwilton) <[e-mail]>; draft-ietf-opsawg-
> [e-mail]
> Cc : [e-mail]
> Objet : Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n
> Part)
> 
> Hi Rob,
> 
> Focusing on the first part of your review, except point (9).
> 
> The changes can be tracked at: https://github.com/IETF-OPSAWG-
> WG/lxnm/commit/337f65012f55e71df4105481bc28fe53ac8bb302, while the
> full changes made so far can be tracked at:
> https://tinyurl.com/l2nm-latest
> 
> Please see inline for more context.
> 
> Cheers,
> Med
> 
> > -----Message d'origine-----
> > De : Rob Wilton (rwilton) <[e-mail]> Envoyé : jeudi 17
> mars
> > 2022 21:53 À : [e-mail]
> > Cc : [e-mail]
> > Objet : AD review of draft-ietf-opsawg-l2nm-12
> >
> > Hi,
> >
> > Apologies for the delay, but I have now managed my AD review of
> draft-
> > ietf-opsawg-l2nm-12.  (Also attached in case my email is
> truncated
> > ...)
> >
> > I would like to thank the authors, WG for their work on this
> document,
> > and the shepherd for his review.  I found the document to be
> well
> > written and pretty straightforward to follow and understand.  I
> also
> > believe that this document is a useful addition to the YANG and
> > Network Management Ecosystem, to thank you for that.
> >
> > Anyway, here my comments.  I think that they mostly pretty
> minor, but
> > perhaps a few that my need a bit more thought.  Hopefully they
> will
> > help improve the doc.
> >
> > ---
> >
> > Moderate comments:
> >
> > (1)
> > 	   The VPN network access is comprised of:
> >
> > 	   'id':  Includes an identifier of the VPN network access.
> >
> > 	   'description':  Includes a textual description of the VPN
> network
> > 		  access.
> >
> > 	   'interface-id':  Indicates the interface on which the VPN
> network
> > 		  access is bound.
> >
> > 	   'global-parameters-profile':  Provides a pointer to an
> active
> > 		  'global-parameters-profile' at the VPN node level.
> > Referencing an
> > 		  active 'global-parameters-profile' implies that all
> associated
> > 		  data nodes will be inherited by the VPN network
> access.
> > However,
> > 		  some of the inherited data nodes (e.g., ACL policies)
> can be
> > 		  overridden at the VPN network access level.  In such
> case,
> > 		  adjusted values take precedence over inherited ones.
> >
> > It wasn't entirely clear to me how this works with the global
> > parameters defined at the VPN network access level and the VPN
> node
> > level work.  Is this meant to be a 3 tier hierarchy, or is it
> always
> > only 2 tiers?  Are you allowed to reference different global
> profiles
> > both at the VPN network access level and the VPN node level?
> > Possibly, some slightly expanded description here may be helpful
> (and/or in the YANG module).
> >
> >
> 
> [Med] Isn't this covered by this text:
> 
>    The 'global-parameters-profile' defines reusable parameters for
> the
>    same L2VPN service instance ('vpn-service').  Global parameters
>    profile are defined at the VPN service level and then called at
> the
>    VPN node and VPN network access levels.  Each VPN instance
> profile is
>    identified by 'profile-id'.  Some of the data nodes can be
> adjusted
>    at the VPN node or VPN network access levels.  These adjusted
> values
>    take precedence over the global ones.  The subtree of 'global-
>    parameters-profile' is depicted in Figure 7.
> 
> 
> > (2)                     |  +--rw encapsulation
> >                           |  |  +--rw encap-type?
> identityref
> >                           |  |  +--rw dot1q
> >                           |  |  |  +--rw tag-type?   identityref
> >                           |  |  |  +--rw cvlan-id?   uint16
> >
> > Did you consider adding support for ranges or sets of VLAN Ids
> (e.g.,
> > a list of non-overlapping ranges) (both for the single and
> double
> > tagged cases)?
> >
> 
> [Med] We didn't. We went for the same structure as we have in
> RFC9182.
> 
> >
> > (3)                           |  +--rw lag-interface
> >
> > I'm slightly surprised that you don't have parameters for the
> physical
> > interfaces, and I can understand your justification for this,
> but then
> > you do have configuration for LAG, including configuration for
> the
> > underlying member interfaces.  This feels slightly inconsistent
> to me
> > at the level that the configuration is being provided.
> Understanding
> > the rationale a bit more here might be helpful, and I think that
> we
> > should double check that this should definitely be in this
> model.
> >
> 
> [Med] We spent many cycles on this one (see the full list of
> related issues at https://github.com/IETF-OPSAWG-
> WG/lxnm/issues?q=lag). There was an agreement that LAG-related
> details are generic and can be defined outside the L2NM. However,
> we included some of this information because we need LACP
> configuration for the ESI auto-assignment mode based on LACP
> configuration (https://github.com/IETF-OPSAWG-WG/lxnm/issues/219).
> 
> >
> > (4)                            +--rw svc-pe-to-ce-bandwidth
> >                                  |       {vpn-common:inbound-
> bw}?
> >                                  |  +--rw pe-to-ce-bandwidth*
> > [bw-type]
> >
> > Can you specify different bandwidths for multiple CoS fields?
> It
> > looks like the list key is the bw-type, and hence you would only
> be
> > able to specify the bandwidth for a single CoS field?  Is that
> sufficient?
> >
> 
> [Med] It isn't :-(
> 
> Updated the structure to fix this + submitted an errata for
> RFC8466 to report the issue.
> 
> >
> > (5)  8.3.  Ethernet Segments
> >
> > The names used here don't seem to be particularly friendly.  Is
> giving
> > them more human understandable identity names an option, or are
> these
> > names directly mirroring some other registry?  Or perhaps even
> > something like esi-type-1-lacp, esi-type-3-mac, etc?
> >
> 
> [Med] These are taken directly form the type values in Section 5
> of RFC7432, but the comment is fair. Updated the names to be more
> human understandable.
> 
> >
> > (6)        leaf svc-mtu {
> >          type uint32;
> >          description
> >            "Service MTU, it is also known as the maximum
> transmission
> >             unit or maximum frame size. When a frame is larger
> than
> >             the MTU, it is fragmented to accommodate the MTU of
> the
> >             network.";
> >        }
> >
> > MTU's turn up in various places.  Given that MTUs seem to mean
> > different things for different people, please can you check the
> > descriptions and given that this model is for L2VPN's be super
> > explicit about whether these MTUs are representing the L3
> payload, or
> > the max frame size including the L2 header and presumably FCS
> sequence as well.
> >
> 
> [Med] This is about the max frame including L2 header. Made this
> change:
> 
> OLD:
>    'svc-mtu':  Is the service MTU for an L2VPN service.  It is
> also
>       known as the maximum transmission unit or maximum frame
> size.
>       When a frame is larger than the MTU, it is fragmented to
>       accommodate the MTU of the network.
> 
> NEW:
>    'svc-mtu':  Is the service MTU for an L2VPN service (i.e.,
> Layer 2
>       MTU).  It is also known as the maximum transmission unit or
>       maximum frame size.  When a frame is larger than the MTU, it
> is
>       fragmented to accommodate the MTU of the network.
> 
> >
> > (7)      identity mac-action {
> >        description
> >          "Base identity for a MAC action.";
> >      }
> >
> > Would an VPN implementation ever want to drop and log rather
> than just
> > doing one or the other?
> >
> 
> [Med] This is about sending a warning message. This was inherited
> form RFC8466:
> 
>    ... SPs
>    may impose a maximum number of MAC addresses learned from the
>    customer for a single service instance by using the "mac-addr-
> limit"
>    leaf and may use the "action" leaf to specify the action taken
> when
>    the upper limit is exceeded: drop the packet, flood the packet,
> or
>    simply send a warning log message.
> 
> I think "log" is confusing. I removed it from the L2NM.
> 
> >
> > (8) Hierarchical groupings and defaults
> >
> > I want to check what the model/plan is regarding hierarchical
> config
> > (e.g., grouping parameters-profile) and default values.  It
> looks like
> > some of the leaves in the provide have default values, which I
> believe
> > will be ambiguous when it comes to hierarchical config.  I.e.,
> > normally I would never expect that a leaf with a default value
> defined
> > would fall back to the hierarchical policy because logically the
> leaf
> > always has a value if it is in scope.  One solution to this
> problem
> > (that is a bit more verbose), would be to take the defaults out
> of the
> > leaves in the grouping, and then add them back in using "refine"
> them
> > using refine statements when the global parameters-profile is
> used.
> > Another choice would be to not express these using the YANG
> "default"
> > keyword at all, and instead describe the defaults in the
> description.
> >
> > Generally, defining defaults where we can is probably useful,
> but we
> > need to be careful with any hierarchical config/policies.
> 
> [Med] Good point. However, for the particular case of parameters-
> profile, the intent is that all these parameters are factorized at
> the service level. Values that have to be overridden at lower
> levels, will be explicitly called and then take precedence.
> 
> >
> >
> > (9) Various comment related to handling VLAN tag rewrites:
> >
> ...
> >
> >
> > (10)
> >                          leaf speed {
> >                            type uint32;
> >                            units "mbps";
> >                            default "10";
> >                            description
> >                              "LACP speed.";
> >                          }
> >
> > 10 Mbps seems like a slow default LACP speed.  Does this need a
> > default at all, Ethernet interfaces would generally negotiate to
> the
> > fastest supported speed.  The same comment applies to the port
> speed.
> >
> 
> [Med] The default was inherited from the service model (L2SM,
> RFC8466).
> 
> >
> > (11)
> > I did wonder whether it is okay to include the BGP and PW types
> as
> > part of this draft?  Personally, since they will be controlled
> by IANA
> > anyway, and this is where they are being used I think that is
> okay,
> 
> [Med] Idem. I don' see any issue there.
> 
>  but
> > I was wondering whether IDR/BESS had been consulted on this at
> all, it
> > might be worth checking with them that they are okay.
> >
> [Med] IDR, BESS, and PALS were solicited during the WGLC, but not
> specifically on this part. Adrian contacted the Chairs of PALS WG
> about an issue on the PW types specifically.
> 
> idr:
> https://mailarchive.ietf.org/arch/msg/idr/2Lqt2OvVOAVbP3awcl13q6aU
> 8_U/
> bess:
> https://mailarchive.ietf.org/arch/msg/bess/yv3iC3qRxcxbSkbqbQD9DWP
> DWNA/ (from Joe) +
> https://mailarchive.ietf.org/arch/msg/bess/fR6O4zSOWhIKMxlJEiawcrm
> wAUU/ (from Adrian)
> pals:
> https://mailarchive.ietf.org/arch/msg/pals/jD82yF5AWwMCgEySMofZPva
> KCag/ (from Adrian)
> 
> __________________________________________________________________
> _______________________________________________________
> 
> 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.
> 
> _______________________________________________
> OPSAWG mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/opsawg

_________________________________________________________________________________________________________________________

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.