Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
Top posting on point 8. I have commented at IETF Last Call that I think that the references to 'default' in the YANG descriptions in several places is misleading. There is no default, there is no YANG default statement and YANG users will know what default means, except that here it does not. Unless you are proposing that users add deviations with default statements, which is not how I read the text, then what I see you suggest is that at different levels (and I note that you, like me, are confused about what I refer to as levels), different values are recommended. This is common practice with many specifications but they are not called defaults and I think it wrong to use the term here. Recommended value would seem what is called for. Tom Petch From: OPSAWG on behalf of Rob Wilton (rwilton) Sent: 27 April 2022 13:38 Hi Med, Catching up with email, sorry for the delay, please see further comments inline ... > -Original Message- > From: mohamed.boucad...@orange.com > > Sent: 05 April 2022 11:40 > To: Rob Wilton (rwilton) ; draft-ietf-opsawg- > l2nm@ietf.org > Cc: opsawg@ietf.org > Subject: RE: 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) > > Envoyé : jeudi 17 mars 2022 21:53 > > À : draft-ietf-opsawg-l2nm@ietf.org > > Cc : opsawg@ietf.org > > 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 ...) > > > > > > (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. At a YANG language level, I'm not convinced this works. To take an example, if you look at mac-policies: | +--rw mac-policies | | +--rw mac-addr-limit | | | +--rw limit-number?uint16 | | | +--rw time-interval? uint32 | | | +--rw action? identityref | | +--rw mac-loop-prevention | | +--rw window?uint32 | | +--rw frequency? uint32 | | +--rw retry-timer? uint32 | | +--rw protection-type? identityref Then the leaf mac-policies/mac-addr-limit/limit-number is always in scope and has a default value assigned. So, even if the global policy "foo" sets limit-number to 200, and this "foo" policy is activated for a given VPN Node then even if limit-number isn't explicitly set under active-global-parameters-profiles the default value for limit-number is in scope and hence would always override the value in the global scope. I think that the best solution here is to not have any defaults in any the leaves under the parameters-profile grouping. Where that grouping is used to define the top level global parameters then you can make use of refine statements under the "uses parameters-profile" to add default values back in only
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
Hi Med, Thanks, just ack'ing that I agree with your proposed fixes, and sorry for getting the terminology mixed up. Thanks, Rob > -Original Message- > From: mohamed.boucad...@orange.com > > Sent: 28 April 2022 12:27 > To: Rob Wilton (rwilton) ; draft-ietf-opsawg- > l2nm@ietf.org > Cc: opsawg@ietf.org > Subject: RE: AD review of draft-ietf-opsawg-l2nm-12 (2n Part) > > Hi Rob, > > Thank you for the follow-up. > > Please see inline. > > Cheers, > Med > > > -Message d'origine- > > De : Rob Wilton (rwilton) > > Envoyé : mercredi 27 avril 2022 14:38 > > À : BOUCADAIR Mohamed INNOV/NET > ; > > draft-ietf-opsawg-l2nm....@ietf.org > > Cc : opsawg@ietf.org > > Objet : RE: AD review of draft-ietf-opsawg-l2nm-12 (2n Part) > > > > Hi Med, > > > > Catching up with email, sorry for the delay, please see further > > comments inline ... > > > > > -Original Message- > > > From: mohamed.boucad...@orange.com > > > > > > Sent: 05 April 2022 11:40 > > > To: Rob Wilton (rwilton) ; draft-ietf-opsawg- > > > l2nm@ietf.org > > > Cc: opsawg@ietf.org > > > Subject: RE: 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) Envoyé : jeudi > > 17 mars > > > > 2022 21:53 À : draft-ietf-opsawg-l2nm@ietf.org > > > > Cc : opsawg@ietf.org > > > > 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
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
Hi Rob,
Thank you for the follow-up.
Please see inline.
Cheers,
Med
> -Message d'origine-
> De : Rob Wilton (rwilton)
> Envoyé : mercredi 27 avril 2022 14:38
> À : BOUCADAIR Mohamed INNOV/NET ;
> draft-ietf-opsawg-l2nm@ietf.org
> Cc : opsawg@ietf.org
> Objet : RE: AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
>
> Hi Med,
>
> Catching up with email, sorry for the delay, please see further
> comments inline ...
>
> > -Original Message-
> > From: mohamed.boucad...@orange.com
> >
> > Sent: 05 April 2022 11:40
> > To: Rob Wilton (rwilton) ; draft-ietf-opsawg-
> > l2nm....@ietf.org
> > Cc: opsawg@ietf.org
> > Subject: RE: 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) Envoyé : jeudi
> 17 mars
> > > 2022 21:53 À : draft-ietf-opsawg-l2nm....@ietf.org
> > > Cc : opsawg@ietf.org
> > > 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 depic
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
Hi Med,
Catching up with email, sorry for the delay, please see further comments inline
...
> -Original Message-
> From: mohamed.boucad...@orange.com
>
> Sent: 05 April 2022 11:40
> To: Rob Wilton (rwilton) ; draft-ietf-opsawg-
> l2nm@ietf.org
> Cc: opsawg@ietf.org
> Subject: RE: 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)
> > Envoyé : jeudi 17 mars 2022 21:53
> > À : draft-ietf-opsawg-l2nm@ietf.org
> > Cc : opsawg@ietf.org
> > 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.
I think that have figured this out. My understanding is:
1) 1 or more profiles can be defined globally with particular parameters.
2) Each VPN service can activate a subset of those global profiles, overriding
parameters if they wish (need to check defaults).
3) Each vpn-node may reference one of the active "global-parameters-profiles".
Is this correct?
I will note that the model doesn't allow a single global profile to create two
slightly different vpn-node profiles based on the same global profile (since
the names match at all levels). Possibly this is a good thing to avoid any
possible confusion, but I wanted to ensure that you had noted it.
I still think that clarifying some of this text might be helpful in a couple of
ways:
(i) In 7.4, possibly tweak the text something like:
OLD
The 'global-parameters-profile' defines reusable parameters for the
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12 (2n Part)
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 De la part de
> mohamed.boucad...@orange.com
> Envoyé : mardi 5 avril 2022 12:40
> À : Rob Wilton (rwilton) ; draft-ietf-opsawg-
> l2nm@ietf.org
> Cc : opsawg@ietf.org
> 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) Envoyé : jeudi 17
> mars
> > 2022 21:53 À : draft-ietf-opsawg-l2nm@ietf.org
> > Cc : opsawg@ietf.org
> > 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 ha
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12
Re-, The clarification looks good to me. fixed. I prefer to not add a dependency to rfc6991-bis now. Thanks. Cheers, Med > -Message d'origine- > De : Rob Wilton (rwilton) > Envoyé : mardi 5 avril 2022 11:04 > À : BOUCADAIR Mohamed INNOV/NET ; draft- > ietf-opsawg-l2nm@ietf.org > Cc : opsawg@ietf.org > Objet : RE: AD review of draft-ietf-opsawg-l2nm-12 > > Hi Med, > > Thanks, your changes all look good, but I have a bit more comment on one > specific point: > > > > (3) > > > 'mac-loop-prevention': Container for MAC loop prevention. > > > > > > 'window': The timer when a MAC mobility event is detected. > > > > > > 'frequency': The number of times to detect MAC > duplication, > > > where a 'duplicate MAC address' situation has occurred > and > > > the duplicate MAC address has been added to a list of > > > duplicate MAC addresses. > > > > > > The description of frequency wasn't really clear to me, perhaps this > > > could be made clearer, or perhaps I just need educating! > > > > [Med] Please check https://github.com/IETF-OPSAWG-WG/lxnm/issues/241 > > (especially the response from Raul when I asked the same question > > about frequency vs window). > > Perhaps it would help if these descriptions were tweaked slightly? E.g., > something like: > >'window': The time interval over which a MAC mobility event is > detected and checked. > >'frequency': The number of times to detect MAC duplication, >where a 'duplicate MAC address' situation has occurred within >the 'window' time interval and the duplicate MAC address has >been added to a list of duplicate MAC addresses. > > If you do decide to update the descriptions here, you might also want to > check the YANG descriptions as well. And, in case it is of interest > rfc6991-bis adds a type for a time interval in seconds (and it may have > other useful types as well). This has gone through WG LC, so I would > expect it to be sent to the IESG relatively soon, but equally I > understand if you don't want to create the dependency. > > Thanks, > Rob > > > > -Original Message- > > From: mohamed.boucad...@orange.com > > > > Sent: 05 April 2022 08:06 > > To: Rob Wilton (rwilton) ; draft-ietf-opsawg- > > l2nm@ietf.org > > Cc: opsawg@ietf.org > > Subject: RE: AD review of draft-ietf-opsawg-l2nm-12 > > > > Hi Rob, > > > > Many thanks for the careful AD review. > > > > Staring with the last part. You can track the changes at: > > https://tinyurl.com/l2nm-latest. Please see inline for more context. > > > > There are also other edits that I made to fix nits, update references, > etc. > > > > Cheers, > > Med > > > > > -Message d'origine- > > > De : Rob Wilton (rwilton) Envoyé : jeudi 17 mars > > > 2022 21:53 À : draft-ietf-opsawg-l2nm@ietf.org > > > Cc : opsawg@ietf.org > > > 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. > > > > > > --- > > ... > > > > > > > > > Minor comments/nits: > > > > > > (1) > > >In particular, the model can be used in the communication > > >interface between the entity that interacts directly with the > > >customer, the service orchestrator (either fully automated or a > human > > >operator), and the entity in charge of network orchestration and > > >control (a.k.a., network controller/orchestrator) by allowing for > > >more network-centric information to be included. > > > > > > Nit (since this is explained more clearly later in the document): It > > > w
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)
> Envoyé : jeudi 17 mars 2022 21:53
> À : draft-ietf-opsawg-l2nm@ietf.org
> Cc : opsawg@ietf.org
> 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)+
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12
Hi Med, Thanks, your changes all look good, but I have a bit more comment on one specific point: > > (3) > > 'mac-loop-prevention': Container for MAC loop prevention. > > > > 'window': The timer when a MAC mobility event is detected. > > > > 'frequency': The number of times to detect MAC duplication, > > where a 'duplicate MAC address' situation has occurred and > > the duplicate MAC address has been added to a list of > > duplicate MAC addresses. > > > > The description of frequency wasn't really clear to me, perhaps this > > could be made clearer, or perhaps I just need educating! > > [Med] Please check https://github.com/IETF-OPSAWG-WG/lxnm/issues/241 > (especially the response from Raul when I asked the same question about > frequency vs window). Perhaps it would help if these descriptions were tweaked slightly? E.g., something like: 'window': The time interval over which a MAC mobility event is detected and checked. 'frequency': The number of times to detect MAC duplication, where a 'duplicate MAC address' situation has occurred within the 'window' time interval and the duplicate MAC address has been added to a list of duplicate MAC addresses. If you do decide to update the descriptions here, you might also want to check the YANG descriptions as well. And, in case it is of interest rfc6991-bis adds a type for a time interval in seconds (and it may have other useful types as well). This has gone through WG LC, so I would expect it to be sent to the IESG relatively soon, but equally I understand if you don't want to create the dependency. Thanks, Rob > -Original Message- > From: mohamed.boucad...@orange.com > > Sent: 05 April 2022 08:06 > To: Rob Wilton (rwilton) ; draft-ietf-opsawg- > l2nm@ietf.org > Cc: opsawg@ietf.org > Subject: RE: AD review of draft-ietf-opsawg-l2nm-12 > > Hi Rob, > > Many thanks for the careful AD review. > > Staring with the last part. You can track the changes at: > https://tinyurl.com/l2nm-latest. Please see inline for more context. > > There are also other edits that I made to fix nits, update references, etc. > > Cheers, > Med > > > -Message d'origine----- > > De : Rob Wilton (rwilton) > > Envoyé : jeudi 17 mars 2022 21:53 > > À : draft-ietf-opsawg-l2nm....@ietf.org > > Cc : opsawg@ietf.org > > 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. > > > > --- > ... > > > > > > Minor comments/nits: > > > > (1) > >In particular, the model can be used in the communication > >interface between the entity that interacts directly with the > >customer, the service orchestrator (either fully automated or a human > >operator), and the entity in charge of network orchestration and > >control (a.k.a., network controller/orchestrator) by allowing for > >more network-centric information to be included. > > > > Nit (since this is explained more clearly later in the document): It was > > unclear to me whether this this sentence is about 3 entities or 2. > > I.e., specifically, whether the "entity that interacts directly with the > > customer" is the service orchestrator. For clarity, would it be clearer > > as: ",i.e., the service orchestrator,". > > > > [Med] I adjusted the sentence for better clarity. > > > (2)'signaling-type': Indicates the signaling that is used for > > setting > > pseudowires. > > > > Nit: setting pseudowires => setting up pseudowires? > > [Med] Fixed. > > > > > (3) > > 'mac-loop-prevention': Container for MAC loop prevention. > > > > 'window': The timer when a MAC mobility event is detected. > > > > 'frequency': The number of times to detect MAC duplication, > > where a 'duplicate MAC addre
Re: [OPSAWG] AD review of draft-ietf-opsawg-l2nm-12
Hi Rob, Many thanks for the careful AD review. Staring with the last part. You can track the changes at: https://tinyurl.com/l2nm-latest. Please see inline for more context. There are also other edits that I made to fix nits, update references, etc. Cheers, Med > -Message d'origine- > De : Rob Wilton (rwilton) > Envoyé : jeudi 17 mars 2022 21:53 > À : draft-ietf-opsawg-l2nm@ietf.org > Cc : opsawg@ietf.org > 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. > > --- ... > > > Minor comments/nits: > > (1) >In particular, the model can be used in the communication >interface between the entity that interacts directly with the >customer, the service orchestrator (either fully automated or a human >operator), and the entity in charge of network orchestration and >control (a.k.a., network controller/orchestrator) by allowing for >more network-centric information to be included. > > Nit (since this is explained more clearly later in the document): It was > unclear to me whether this this sentence is about 3 entities or 2. > I.e., specifically, whether the "entity that interacts directly with the > customer" is the service orchestrator. For clarity, would it be clearer > as: ",i.e., the service orchestrator,". > [Med] I adjusted the sentence for better clarity. > (2)'signaling-type': Indicates the signaling that is used for > setting > pseudowires. > > Nit: setting pseudowires => setting up pseudowires? [Med] Fixed. > > (3) > 'mac-loop-prevention': Container for MAC loop prevention. > > 'window': The timer when a MAC mobility event is detected. > > 'frequency': The number of times to detect MAC duplication, > where a 'duplicate MAC address' situation has occurred and > the duplicate MAC address has been added to a list of > duplicate MAC addresses. > > The description of frequency wasn't really clear to me, perhaps this > could be made clearer, or perhaps I just need educating! [Med] Please check https://github.com/IETF-OPSAWG-WG/lxnm/issues/241 (especially the response from Raul when I asked the same question about frequency vs window). > > (4)'multicast-like': Controls whether multicast is allowed in the > service. > > 'multicast-like' seems like a strange name. Wouldn't the service either > support/emulate multicast or not? [Med] Indeed. removed "-like". I lost the context why this name ended in the draft. > > (5) 7.5. VPN Nodes > >+--rw vpn-nodes > +--rw vpn-node* [vpn-node-id] > +--rw vpn-node-idvpn-common:vpn-id > +--rw description? string > +--rw ne-id? string > +--rw role? identityref > > 'role' doesn't seem to be described in the description that follows, or > specifically, it is not in the description where I expected it to be. [Med] Good catch. Fixed. Thanks. > > (6) > | +--rw (signaling-option)? > | ... > | +--:(bgp) > | | ... > | +--:(ldp-or-l2tp) > > It's not obvious to me why LDP and L2TP are bundled together in the same > signaling option. [Med] Because they share a set of common attributes. > > More generally, I was surprised that you don't have containers at the > top-level of the signaling-options, e.g, to be explicit about what > signaling option is being used (i.e., what branch of the choice is being > selected). Is the thinking that you already have "signaling-type" > earlier in the definition. Personally, I think that having containers > here would probably be clearer, but I'm happy to leave it to the authors > discretion. > [Med] The signaling type is set at the service level (signaling-type). Among the values that can be used for that attribute, we do have l2tp-signaling or ldp-signaling
[OPSAWG] 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).
(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)?
(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.
(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?
(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?
(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.
(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?
(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.
