[Gen-art] Genart telechat review of draft-ietf-bess-evpn-vpws-11

[Roni Even]

Reviewer: Roni Even
Review result: Ready

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

.

Document: draft-ietf-bess-evpn-vpws-??
Reviewer: Roni Even
Review Date: 2017-04-08
IETF LC End Date: 2017-03-10
IESG Telechat date: 2017-04-13

Summary:

Major issues:

Minor issues:

Nits/editorial comments: 

In section 1 second paragraph "[RFC7432] provides the ability " looks
like the reference is not a link to RFC7432.



___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Gen-ART Last Call review of draft-ietf-isis-mi-bis-02

[Les Ginsberg (ginsberg)]

Orit -

Thanx for the review.
Responses inline.

> -Original Message-
> From: Orit Levin [mailto:or...@microsoft.com]
> Sent: Thursday, April 06, 2017 8:27 PM
> To: gen-art@ietf.org
> Cc: i...@ietf.org; draft-ietf-isis-mi-bis@tools.ietf.org
> Subject: Gen-ART Last Call review of draft-ietf-isis-mi-bis-02
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for
> the IETF Chair.  Please treat these comments just like any other last call
> comments.
> 
> For more information, please see the FAQ at
> 
> .
> 
> Document: draft-ietf-isis-mi-bis-02
> Reviewer: Orit Levin
> Review Date: 2017-04-06
> IETF LC End Date: 2017-04-07
> IESG Telechat date: 2017-04-13
> 
> Summary: This draft is "ready with issues" for publication.
> 
> Major issues: None.
> 
> Minor issues:
> 
> 1. Add text explaining the reason (or reasons) for replacing the original RFC
> 6822 from 2012.
> Reason: It is a "bis" draft and there is no mention about it in the text.

[Les:] Note that the latest revision of the draft correctly identifies the 
draft as obsoleting RFC 6822. Previous versions had incorrectly identified this 
as an update to RFC 6822.
This is then the new Standard for the IS-IS MI support.

There are two classes of future readers of this document:

a)Readers who are unfamiliar with RFC 6822. For them what changed between RFC 
6822 and this document is irrelevant. 

b)Readers who are familiar with RFC 6822. For them it is useful to know what 
changed - which is described in Appendix A.

In order not to distract readers of type "a" - as well as to provide an 
"uninterrupted" description of the normative behavior I believe placement of 
the change description in an Appendix improves the readability of the document.

Does this make sense to you?

> 2. In Abstract, state clearly that this standard introduces the support for
> instances vs. other already existing concepts also listed in the Abstract 
> (i.e.,
> circuits, adjacencies,  topologies, etc.).

[Les:] The Abstract currently says:

"This draft describes a mechanism that allows a single router to share
   one or more circuits among multiple Intermediate System To
   Intermediate System (IS-IS) routing protocol instances."

Previous to this extension, a router could have multiple instances of the IS-IS 
protocol, but multiple instance could not be run over the same interface. 
So we are not introducing "instances", but we are introducing the ability to 
enable multiple instances on the same interface.

> Reason: The wording is not clear about what is the new feature vs. what are
> the new benefits vs. what was the original baseline 

>3. Throughout the
> document, use "standard instance" instead of "IID = 0" or "IID #0".
> Reason: Expressions "standard instance", "IID = 0" and "IID #0" are used
> interchangeably throughout the document. It seems that they all refer to the
> same thing - the implementation of the original protocol without the concept
> of instances. Please, correct me if I am wrong.

[Les:]  I don't think this is possible without seriously compromising the 
document. For example:

Section 2.1

" IID #0 is reserved for the standard instance supported by legacy
   systems. "

Changing this to  " Standard instance is reserved for the standard instance ..."

Is clearly nonsensical.

Later in Section 2.1

"When the IID = 0, the list of supported ITIDs MUST NOT be present."

What is being discussed here is what is the correct behavior when an MI-capable 
router sends a PDU associated IID #0 and includes the new IID TLV. 
Replacing this with "When the standard instance..." loses the important point 
that the value of the IID in the IID TLV in this case is "0".

Hope this helps clarify things.

> 4. In section 2 par 3, change "support" and "operates" to "MUST support" to
> use requirements language.

[Les:] I am on the fence as regards this change. Section 2 is an introduction 
to the following sub-sections - which define the normative behavior. But the 
introduction itself is not defining normative behavior - it is providing a 
context in which the protocol extensions defined in the following sub-sections 
can be understood. 

I am more inclined to change the "MAY" used later in the same paragraph you 
mention to "may" so it is consistent with the rest of this section.

???


> 5. In section 2 par 2, change "may" to either "can" or "MAY" to clarify the
> intent.

[Les:] Did you mean Section 2.1 para 2?
If so I agree to the change.

> 6. In section 2.1 par 3, clarify whether IID #0 is ever being used on the 
> wire.

[Les:] There are numerous places in the document where the legal use of IID  #0 
is discussed. I do not understand how a reader would conclude that IID #0 is 
never sent on the wire.

> Explain the concept of the "standard interface" (see previous comment)

[Les:] There is no mention of 

Re: [Gen-art] Genart last call review of draft-ietf-rtgwg-yang-key-chain-17

[Acee Lindem (acee)]

Hi Matt,

Thanks for the review.

On 4/7/17, 1:58 PM, "Matthew Miller" 
wrote:

>Reviewer: Matthew Miller
>Review result: Almost Ready
>
>I am the assigned Gen-ART reviewer for this draft. The General Area
>Review Team (Gen-ART) reviews all IETF documents being processed
>by the IESG for the IETF Chair.  Please treat these comments just
>like any other last call comments.
>
>For more information, please see the FAQ at
>
>.
>
>Document: draft-ietf-rtgwg-yang-key-chain-17
>Reviewer: Matthew A. Miller
>Review Date: 2017-04-07
>IETF LC End Date: 2017-04-07
>IESG Telechat date: 2017-04-13
>
>Summary:
>
>This document is almost ready to be published as a Proposed Standard,
>once the issues noted herein are resolved.
>
>Major issues:
>
>NONE
>
>Minor issues:
>
>* Forgive me for my limited knowledge of YANG, but is there a reason
>key-strings are only representable as either a YANG string or
>hex-string type, and not the YANG binary type?

Let me ask why I¹d want to use this type? I can get all the entropy I want
with a hex string and implementations are familiar with this
representation. I¹m not really fond of the obscure base64 representation
used by the YANG type and if one consults Benoit¹s search tool, the type
is not widely used. http://www.yangcatalog.org/yang-search/yang-search.php

>
>* This document does not provide much guidance around AES key wrap
>other than it can be used and the KEK is provided
>out-of-band/-context.
>For instance, AES key-wrapped key-strings probably require using
>"hexidecimal-string".

Yes - I¹ll add that.

>  Also, assuming I'm reading the model
>correctly,
>it appears this feature applies to the whole chain, which I think is
>worth calling out.

In YANG, if you support a feature, it is for the entire model.
The per-chain boolean indicates whether or not it is applies to all the
keys in that particular key-chain. I will clarify this.
>
>* This document warns against using the "clear-text" algorithm, which
>the
>reader is lead to understand is for legacy implementation reasons.
>However, is there not a similar concern with cryptographically weak
>algorithms, such as md5 and (arguably) sha1?

I can add something for MD5.
>
>Nits/editorial comments:
>
>* In Section 3.2. "Key Chain Model Features", the word "of" is
>missing
>between "configuration" and "an" in the phrase "support configuration
>an
>acceptance tolerance".

Good catch. I will fix.
>
>Non-nits:
>
>* I note that idnits is calling out some odd spacing issues, but I
>think
>they are safe to ignore.

Though the line numbers don¹t match the draft, I was able to fix these.

Thanks,
Acee 
>

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art