Hi, Jie.
Thanks for the response.
Some notes in line below.
Regards,
Elwyn
On 10/02/2015 03:06, Dongjie (Jimmy) wrote:
Hi Elwyn,
Thanks a lot for your comments. We will fix the nits with a new revision. And
please see my replies inline.
Best regards,
Jie
-----Original Message-----
From: Elwyn Davies [mailto:elw...@dial.pipex.com]
Sent: Monday, February 09, 2015 7:16 AM
To: General area reviewing team
Cc: draft-ietf-teas-rsvp-te-li-lb....@tools.ietf.org
Subject: Gen-art LC review of draft-ietf-teas-rsvp-te-li-lb-03
I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART,
please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
Please resolve these comments along with any other Last Call comments you
may receive.
Document:
Reviewer: draft-ietf-teas-rsvp-te-li-lb-03.txt
Review Date: 2015/02/08
IETF LC End Date: 2015/02/18
IESG Telechat date: (if known) -
Summary: Almost ready with a number of nits. As somebody who has very
sketchy knowledge of RSVP-TE and GMPLS, I found the discussion of
specification of the identity of the loopback node in an ERO to be very
confusing. The phrase "strictly identify" doesn't really express what is going
on to somebody who is not deeply familiar with the topic. An example of the
relevant parts of a PATH message might help if that could be managed. What I
think is an unfortunate typo ("mode" instead of "node" in s3.2) sent me off on a
wildgoose chase looking for specifications of loopback modes. I am still not
absolutely certain that this *is* a typo!
Major issues:
None
Minor issues:
None.
Nits/editorial comments:
General: For the avoidance of doubt for IANA, it would be desirable to identify
the five "TBA" values as TBA-1 to TBA-5 wherever they occur.
General: s/in ADMIN_STATUS/in the ADMIN_STATUS/ (10 cases)
Abstract: s/as control plane/for the control plane/
s1, para 1 and last para: s/in Multiprotocol/in the Multiprotocol/
s1,para 2: s/as control plane/for the control plane, s/e.g./e.g.,/
s1, last para: /For MPLS-TP network/For a network supporting MPLS-TP/
s1, last para: Although the formal definitions of LI and LB are given in the 2
RFCs referenced, it would help with the understanding of this draft to add short
explanation, such as:
ADD:
An LSP that is locked, using LI, is prevented from carrying user data traffic.
The
LB function can only be applied to an LSP that has been previously locked.
s2.1: s?the lock/unlock?the lock/unlock status?,
s2.1: RFC 3471 calls ADMIN_STATUS "Administrative Status". RFC3473 uses
ADMIN_STATUS but the two terms are never formally linked together. I
suggest:
s/in ADMIN_STATUS/in the Administrative Status (ADMIN_STATUS) /
s3.1, para 1: Technically, the A bit isn't defined in this draft: Suggest:
s/bit defined above/bit used as specified above/
s3.2, para 2: Need to expand acronym: ERO
Thanks. The above nits will be fixed in a new revision.
s3.2, para 2:
OLD:
The ingress MUST ensure that the desired loopback mode is strictly
identified in the ERO.
Am I right that "mode" is a typo? Should it be "node" rather than "mode? I
couldn't see anything about "loopback modes" in RFC 5860 or RFC 6371. After
much head scratching I came to the conclusion that "node"
was the right answer... If not then please explain where modes are defined.
I guess we may not call it a typo. The above sentence was added during the WG
review, the intention was to strictly identify the entity at which the loopback
is desired, which can be either a node or an interface of a node.
If we replace "mode" with "entity", do you think it can be clearer?
Continuing on the basis that the word s/b "node"... I think this would
be clearer as follows:
NEW
The ingress node MUST ensure that the node where loopback is intended to
occur is marked as a strict hop (i.e., not a loose hop) in the ERO.
If you agree with the above change, in your suggestion we can also substitute "node" with
"entity".
That would help. How about:
NEW (mk 2):
The ingress node MUST ensure that the entity (node or interface), at which
loopback
is intended to occur, is marked as a strict hop (i.e., not a loose hop) in the
ERO
type subobject.
s3.2, para 3:
the node MUST check that the desired loopback is strictly
identified by verifying that the L bit is set to 0 in both the ERO
Hop Attributes subobject and the prior subobject. The prior
subobject MUST also be checked to ensure that it provides strict
identification.
What would the prior subobject likely be? Would it be possible to give an
example of what the RSVP message would contain in the way of objects and
subobjects? This would help with understanding this section. As with the
previous comment, the phrase 'strictly identified' is not very clear. Maybe:
OLD:
desired loopback is strictly identified
NEW:
node at which loopback is desired is a strict hop in the explicit route
OLD:
it provides strict identification
NEW:
it identifies a strict hop in the explicit route
The prior subobject of "ERO Hop Attributes subobject" would also be ERO subobject which
identifies the target entity of loopback. As specified in the sentence below, the type value of the
prior subobject ensures the prior subobject would be an IP prefix or an interface ID, which can
identify the node or interface at which loopback is desired. Note that the current description
requires both the "ERO Hop Attributes subobject" and the prior subobject have the L bit
set to 0 to ensure that the two subobjects together strictly specify a loopback request on an
specific entity (node or interface).
We can rephrase to make it clearer.
I have looked into this issue a bit more deeply, and realize that my
problem is not really with this draft but, at least partly, with
draft-ietf-teas-lsp-attribute-ro-01. Having gone back to RFC3209, I
understand that the subobjects in an ERO as defined there are all
specifiers for type of 'entity' that could be waypoints on the explicit
route. There is no allowance for other types of subobject (such as the
attribute subobjects being introduced by
draft-ietf-teas-lsp-attribute-ro-01). As introduced in RFC 3209, both
EROs and RROs contained only entity type subobjects, but RFC 5420
changed this for *RROs only* by introducing RRO Attributes subobjects
which could be inserted between the type subobjects. Thus as
standardized currently, EROs aren't allowed any other sort of subobject
and there is no equivalent to the "Presence Rules" defined for RROs in
RFC 5420 section 7.3.1.
draft-ietf-teas-lsp-attribute-ro-01 updates the presence rules for RROs
to allow for the RRO Hop Attributes subobjects but the necessary
presence rules that would control where the ERO HOP Attributes could be
inserted are not properly defined in
draft-ietf-teas-lsp-attribute-ro-01. Para 1 of s2.3 of
draft-ietf-teas-lsp-attribute-ro-01 seems to indicate that the HOP
Attributes would be placed after all the type subobjects which doesn't
seem right. I would have thought that the hop attributes needed to be
interleaved between the type subobjects - and I think that is what the
existing text in the lock draft is trying to say.
So I think the draft-ietf-teas-lsp-attribute-ro-01 needs to be clarified
- a specific section on Presence Rules as in RFC 5420 would help.
The text about checking that the L bit in the lock draft is confusing,
because the L bit is *required* by draft-ietf-teas-lsp-attribute-ro-01
to be 0 in *all* Hop Attributes subobjects whether they are associated
with loose or strict hops and need not be mentioned. The L bit is only
set in the type subobject if it is a loose hop so the check should be
that the type subobject has its L bit clear. It should also be made
clearer in the lock draft that there can be multiple Hop Attribute
subobjects after the type subobject.
I hope this helps.
s3.2, para 3:
Currently, the type value MUST be verified to be
less than 32, and for type values 1 and 2 the prefix length MUST be
32 and 128 respectively.
It would be better to use the names of the types (1 = IPv4, 2 = IPv6) that would
make it clearer why the lengths are as they are specified.
Is it possible to explain why the type has to be less than this apparently
arbitrary
limit of 32? And hence what allocation constraints ought to be applied in the
future - this might need a change in the IANA allocation rules that should be
specified in this document.
The limit of 32 was to ensure the subobject type can identify a specific node
or interface. Maybe we can specify the required types (node, interface)
explicitly to make it clear.
That would help.
It has just occurred to me why you want to say that the prefix lengths
are 32/128 bits - so that the IP address refers to exactly one node.
However this is possibly confusing because RFC 3209 *requires* the
values to be 32/128 respectively and the subobjects do carry just a host
address and not a domain prefix as might be implied by lower values.
I noted having looked again at the registry [1], and it appears that
nobody has yet defined an ERO type for 4 octet AS numbers which have now
(finally) been standardized. This might mean that 32 was not the right
boundary. Perhaps this means that you need to define an update to the
allocation rules for the registry so that values below (say) 32 are
required to identify exactly one physical node and not an abstract node
with potentially many physical nodes.
[1]
http://www.iana.org/assignments/rsvp-parameters/rsvp-parameters.xml#rsvp-parameters-25
s5: In line with "modern" policy, the discussion in para 3 of s5 of
draft-ietf-teas-lsp-attribute-ro-01 could be thought of a "Privacy
Considerations" rather than strictly Security Considerations. Please consider
separating out a privacy discussion and referencing the discussion in
draft-ietf-teas-lsp-attribute-ro-01 (which should in turn talk about Privacy
Considerations).
OK
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art
