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.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.
(9) Various comment related to handling VLAN tag rewrites:
container dot1q {
container rewrite {
description
leaf pop {
type enumeration {
enum 1 {
description
"Allows one (1) tag removal.";
}
enum 2 {
description
"Allows two (2) tags removal.";
}
}
description
"Tag removal.";
}
leaf translate {
type enumeration {
enum 1-to-1 {
description
"Allows one (1) to one (1) tag
translation.";
}
enum 1-to-2 {
description
"Allows one (1) to two (2) tags
translation.";
}
enum 2-to-1 {
description
"Allows two (2) to one (1) tag
translation.";
}
enum 2-to-2 {
description
"Allows two (2) to two (2) tags
translation.";
}
(i) This rewrite command is currently under dot1q and not qinq, hence it only
seems to apply to single tagged interfaces.
(ii) Logically, any pops should be limited to only popping matched tags (e.g.,
don't allow a 2 tag pop if a single tag is matched) or otherwise the operation
cannot be done symmetrically.
(iii) leaf pop could just be defined as an int8 (range 1-2) rather than an
enumeration.
(iv) Currently there is no way of pushing more than one tag (which currently
must be a C-VLAN tag).
(v) There is no choice in the Ethertype of the tag that is being pushed (but
perhaps this is okay, if a single tag is pushed it is always a C-VLAN, if two
tags are pushed then it is S-VLAN, C-VLAN).
(vi) Do you want/need the translate enumeration leaf alongside the push and pop?
(vii) The "direction" enum only has one option, perhaps allowing a deviation to
support asymmetric rewrites? But those cannot really be expressed with how the
current model is defined anyway. It might be worth making symmetric the
default choice, but I'm not sure whether you want the leaf at all?
(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.
(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, 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.
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,".
(2) 'signaling-type': Indicates the signaling that is used for setting
pseudowires.
Nit: setting pseudowires => setting up pseudowires?
(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!
(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?
(5) 7.5. VPN Nodes
+--rw vpn-nodes
+--rw vpn-node* [vpn-node-id]
+--rw vpn-node-id vpn-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.
(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.
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.
(7) Section 8.1/8.2
Quite a lot of these types look like they are probably dead or not useful. I
guess that publishing them all to keep them directly in sync with the
associated IANA registries makes sense, but I wonder if it would be helpful to
have any text indicating that only some of these types are likely to be useful,
or perhaps highlight the ones that are more likely to be used?
(8)
leaf mac-num-limit {
type uint16;
default "2";
description
"Maximum number of MAC addresses learned from
the customer for a single service instance.";
}
"2" feels like a slightly strange default here, rather than say "1" or having
no default. What is the basis of this value.
(9)
leaf action {
type identityref {
base mac-action;
}
default "warning";
description
"specify the action when the upper limit is
exceeded: drop the packet, flood the
packet, or simply send a warning log
message.";
}
In the case of sending a warning log message, does this mean that the packet is
still forwarded normally? In the case of flood, does that mean that the MAC
address is not learned?
(10)
leaf router-id {
type rt-types:router-id;
description
"A 32-bit number in the dotted-quad format that is
used to uniquely identify a node within an
autonomous system (AS). ";
}
Nit: Trailing space in the description.
(11) container bum-management {
description
"Broadcast-unknown-unicast-multicast
management.";
leaf discard-broadcast {
type boolean;
description
"Discards broadcast, when enabled.";
}
leaf discard-unknown-multicast {
type boolean;
description
"Discards unknown multicast, when
enabled.";
}
leaf discard-unknown-unicast {
type boolean;
description
"Discards unknown unicast, when
enabled.";
}
}
>From the descriptions, it looks like these could be default "false" (subject
>to the comment about hierarchical configuration)?
(12)
9. Security Considerations
* 'ethernet-segments' and 'vpn-services': An attacker who is able to
access network nodes can undertake various attacks, such as
deleting a running L2VPN service, interrupting all the traffic of
a client.
Is there a risk that an attacker could add a VPN endpoint that they control,
and hence either inject or snoop traffic in the L2VPN service (which is
arguably worse than either interrupting or deleting the L2VPN service)?
(13)
10.2. BGP Layer 2 Encapsulation Types
This document defines the initial version of the IANA-maintained
"iana-bgp-l2-encaps" YANG module.
Perhaps include the reference back to the section where the YANG module is
defined? And similarly for the PW types YANG module.
(14)
When a Layer 2 encapsulation type is added to the "BGP Layer 2
Encapsulation Types" registry, a new "identity" statement must be
added to the "iana-bgp-l2-encaps" YANG module. The name of the
"identity" is the lower-case of encapsulation name provided in the
description. The "identity" statement should have the following sub-
statements defined:
Nit: of encapsulation name => of the encapsultion name
(15)
When the "iana-bgp-l2-encaps" YANG module is updated, a new
"revision" statement must be added in front of the existing revision
statements.
Possibly refine this to (for both modules) - there was a case where IANA
accidentally created two entries with the same revision date when adding 2
types:
When the "iana-bgp-l2-encaps" YANG module is updated, a new
"revision" statement with a unique revision date must be added in front
of the existing revision statements.
(16)
A.5
"auto-ethernet-segment-identifier": "01:11:00:11:00:11:\
11:9A:00:00"
lowercase A would be canonical in the hex string.
--------
Grammar nits from an automated tool.
Grammar Warnings:
Section: 2, draft text:
The VPN node will identify the service providers node on which the VPN is
deployed.
Warning: Apostrophe might be missing.
Suggested change: "providers'"
Section: 7.2, draft text:
An external connectivity may be an access to the Internet or a restricted
connectivity such as access to a public/private cloud.
Warning: Uncountable nouns are usually not used with an indefinite article.
Use simply access.
Suggested change: "access"
Section: 7.4, draft text:
Some of the data nodes can be adjusted at the VPN node or VPN network access
levels.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "Some"
Section: 7.6, draft text:
A 'vpn-network-access' ([vpn_network_access_tree]) represents an entry point to
a VPN service .
Warning: Don't put a space before the full stop.
Suggested change: "."
Section: 7.6, draft text:
A 'vpn-network-access' includes information such as the connection on which the
access is defined , the specific Layer 2 service requirements, etc.
Warning: Put a space after the comma, but not before the comma.
Suggested change: ","
Section: 7.6, draft text:
The VPN network access is comprised of:
Warning: Did you mean comprises or consists of or is composed of?
Suggested change: "comprises"
Section: 7.6, draft text:
However, some of the inherited data nodes (e.g., ACL policies) can be
overridden at the VPN network access level.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "some"
Section: 7.6.1, draft text:
The L2NM inherits the 'member-link-list' structure from the L2SM (including
indication of OAM 802.3ah support [IEEE-802-3ah].
Warning: Unpaired symbol: ')' seems to be missing
Section: 7.6.4, draft text:
An ACL can be based on source MAC address, source MAC address mask, destination
MAC address , and destination MAC address mask.
Warning: Put a space after the comma, but not before the comma.
Suggested change: ","
Section: 9, draft text:
Some of the readable data nodes in the "ietf-l2vpn-ntw" YANG module may be
considered sensitive or vulnerable in some network environments.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "Some"
Section: A.1, draft text:
This section provides an example to illustrate how the L2NM can be used to
mange BGP-based VPLS.
Warning: Did you mean manage?
Suggested change: "manage"
Regards,
Rob
Hi,
Apologies for the delay, but I have now managed my AD review of
draft-ietf-opsawg-l2nm-12.
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.
Anyway, here my comments. I think that they mostly pretty minor, but probably
a few that my need a bit more thought. But 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.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.
(9) Various comment related to handling VLAN tag rewrites:
container dot1q {
container rewrite {
description
leaf pop {
type enumeration {
enum 1 {
description
"Allows one (1) tag removal.";
}
enum 2 {
description
"Allows two (2) tags removal.";
}
}
description
"Tag removal.";
}
leaf translate {
type enumeration {
enum 1-to-1 {
description
"Allows one (1) to one (1) tag
translation.";
}
enum 1-to-2 {
description
"Allows one (1) to two (2) tags
translation.";
}
enum 2-to-1 {
description
"Allows two (2) to one (1) tag
translation.";
}
enum 2-to-2 {
description
"Allows two (2) to two (2) tags
translation.";
}
(i) This rewrite command is currently under dot1q and not qinq, hence it only
seems to apply to single tagged interfaces.
(ii) Logically, any pops should be limited to only popping matched tags (e.g.,
don't allow a 2 tag pop if a single tag is matched) or otherwise the operation
cannot be done symmetrically.
(iii) leaf pop could just be defined as an int8 (range 1-2) rather than an
enumeration.
(iv) Currently there is no way of pushing more than one tag (which currently
must be a C-VLAN tag).
(v) There is no choice in the Ethertype of the tag that is being pushed (but
perhaps this is okay, if a single tag is pushed it is always a C-VLAN, if two
tags are pushed then it is S-VLAN, C-VLAN).
(vi) Do you want/need the translate enumeration leaf alongside the push and pop?
(vii) The "direction" enum only has one option, perhaps allowing a deviation to
support asymmetric rewrites? But those cannot really be expressed with how the
current model is defined anyway. It might be worth making symmetric the
default choice, but I'm not sure whether you want the leaf at all?
(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.
(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, 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.
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,".
(2) 'signaling-type': Indicates the signaling that is used for setting
pseudowires.
Nit: setting pseudowires => setting up pseudowires?
(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!
(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?
(5) 7.5. VPN Nodes
+--rw vpn-nodes
+--rw vpn-node* [vpn-node-id]
+--rw vpn-node-id vpn-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.
(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.
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.
(7) Section 8.1/8.2
Quite a lot of these types look like they are probably dead or not useful. I
guess that publishing them all to keep them directly in sync with the
associated IANA registries makes sense, but I wonder if it would be helpful to
have any text indicating that only some of these types are likely to be useful,
or perhaps highlight the ones that are more likely to be used?
(8)
leaf mac-num-limit {
type uint16;
default "2";
description
"Maximum number of MAC addresses learned from
the customer for a single service instance.";
}
"2" feels like a slightly strange default here, rather than say "1" or having
no default. What is the basis of this value.
(9)
leaf action {
type identityref {
base mac-action;
}
default "warning";
description
"specify the action when the upper limit is
exceeded: drop the packet, flood the
packet, or simply send a warning log
message.";
}
In the case of sending a warning log message, does this mean that the packet is
still forwarded normally? In the case of flood, does that mean that the MAC
address is not learned?
(10)
leaf router-id {
type rt-types:router-id;
description
"A 32-bit number in the dotted-quad format that is
used to uniquely identify a node within an
autonomous system (AS). ";
}
Nit: Trailing space in the description.
(11) container bum-management {
description
"Broadcast-unknown-unicast-multicast
management.";
leaf discard-broadcast {
type boolean;
description
"Discards broadcast, when enabled.";
}
leaf discard-unknown-multicast {
type boolean;
description
"Discards unknown multicast, when
enabled.";
}
leaf discard-unknown-unicast {
type boolean;
description
"Discards unknown unicast, when
enabled.";
}
}
From the descriptions, it looks like these could be default "false" (subject to
the comment about hierarchical configuration)?
(12)
9. Security Considerations
* 'ethernet-segments' and 'vpn-services': An attacker who is able to
access network nodes can undertake various attacks, such as
deleting a running L2VPN service, interrupting all the traffic of
a client.
Is there a risk that an attacker could add a VPN endpoint that they control,
and hence either inject or snoop traffic in the L2VPN service (which is
arguably worse than either interrupting or deleting the L2VPN service)?
(13)
10.2. BGP Layer 2 Encapsulation Types
This document defines the initial version of the IANA-maintained
"iana-bgp-l2-encaps" YANG module.
Perhaps include the reference back to the section where the YANG module is
defined? And similarly for the PW types YANG module.
(14)
When a Layer 2 encapsulation type is added to the "BGP Layer 2
Encapsulation Types" registry, a new "identity" statement must be
added to the "iana-bgp-l2-encaps" YANG module. The name of the
"identity" is the lower-case of encapsulation name provided in the
description. The "identity" statement should have the following sub-
statements defined:
Nit: of encapsulation name => of the encapsultion name
(15)
When the "iana-bgp-l2-encaps" YANG module is updated, a new
"revision" statement must be added in front of the existing revision
statements.
Possibly refine this to (for both modules) - there was a case where IANA
accidentally created two entries with the same revision date when adding 2
types:
When the "iana-bgp-l2-encaps" YANG module is updated, a new
"revision" statement with a unique revision date must be added in front
of the existing revision statements.
(16)
A.5
"auto-ethernet-segment-identifier": "01:11:00:11:00:11:\
11:9A:00:00"
lowercase A would be canonical in the hex string.
--------
Grammar nits from an automated tool.
Grammar Warnings:
Section: 2, draft text:
The VPN node will identify the service providers node on which the VPN is
deployed.
Warning: Apostrophe might be missing.
Suggested change: "providers'"
Section: 7.2, draft text:
An external connectivity may be an access to the Internet or a restricted
connectivity such as access to a public/private cloud.
Warning: Uncountable nouns are usually not used with an indefinite article.
Use simply access.
Suggested change: "access"
Section: 7.4, draft text:
Some of the data nodes can be adjusted at the VPN node or VPN network access
levels.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "Some"
Section: 7.6, draft text:
A 'vpn-network-access' ([vpn_network_access_tree]) represents an entry point to
a VPN service .
Warning: Don't put a space before the full stop.
Suggested change: "."
Section: 7.6, draft text:
A 'vpn-network-access' includes information such as the connection on which the
access is defined , the specific Layer 2 service requirements, etc.
Warning: Put a space after the comma, but not before the comma.
Suggested change: ","
Section: 7.6, draft text:
The VPN network access is comprised of:
Warning: Did you mean comprises or consists of or is composed of?
Suggested change: "comprises"
Section: 7.6, draft text:
However, some of the inherited data nodes (e.g., ACL policies) can be
overridden at the VPN network access level.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "some"
Section: 7.6.1, draft text:
The L2NM inherits the 'member-link-list' structure from the L2SM (including
indication of OAM 802.3ah support [IEEE-802-3ah].
Warning: Unpaired symbol: ')' seems to be missing
Section: 7.6.4, draft text:
An ACL can be based on source MAC address, source MAC address mask, destination
MAC address , and destination MAC address mask.
Warning: Put a space after the comma, but not before the comma.
Suggested change: ","
Section: 9, draft text:
Some of the readable data nodes in the "ietf-l2vpn-ntw" YANG module may be
considered sensitive or vulnerable in some network environments.
Warning: If the text is a generality, 'of the' is not necessary.
Suggested change: "Some"
Section: A.1, draft text:
This section provides an example to illustrate how the L2NM can be used to
mange BGP-based VPLS.
Warning: Did you mean manage?
Suggested change: "manage"
Regards,
Rob
_______________________________________________
OPSAWG mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/opsawg
