Re: [Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-10-03 Thread Peter Psenak

Hi Dhruv,

On 03/10/2019 16:48, Peter Psenak wrote:


(3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
definition and usage of ERLD


##PP
The Introduction section has:

"This capability, referred to as Entropy Readable Label Depth (ERLD) as
defined in[I-D.ietf-mpls-spring-entropy-label]"

Is not that sufficient?



This is good, wondering if this should be a normative reference?


##PP2
I can make that informative.


sorry, I meant to say, I will make it normative.

thanks,
Peter


___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr


Re: [Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-10-03 Thread Peter Psenak

Hi Dhruv,

On 03/10/2019 16:14, Dhruv Dhody wrote:

Hi Peter,

Snipping to open points...


(1) Please use updated requirement language text as per RFC 8174, as you do
have a mix of upper-case and lower-case terms in your I-D.

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
"MAY", and "OPTIONAL" in this document are to be interpreted as
described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
appear in all capitals, as shown here.


##PP
RFC 8174 allows the usage of a mix of upper and lower case. If used in
lower case "they have their normal English meanings", which is the case
in this draft. Do you have any specific concerns in that regard?



DD: You are currently using 2119 requirement language in the front
page. I am suggested to move to 8174.


##PP2
sure, I can do that.




(2) Could you mark that the codepoints mentioned in the draft are early
allocated by IANA? This would make it clear that you are not squatting on them.
I also suggest following change in Section 7 (IANA Considerations) -

OLD:
 This document requests IANA to allocate one flag from the OSPFv2
 Extended Prefix TLV Flags registry:

0x20 - E-Flag (ELC Flag)

 This document requests IANA to allocate one flag from the OSPFv3
 Prefix Options registry:

0x04 - E-Flag (ELC Flag)
NEW:
 IANA is requested to confirm the early allocation of the following
 code point in the OSPFv2 Extended Prefix TLV Flags registry:

0x20 - E-Flag (ELC Flag)

 IANA is requested to confirm the early allocation of the following
 code point in the  the OSPFv3 Prefix Options registry:

0x04 - E-Flag (ELC Flag)
END


##PP
I'm not sure above is necessary, given that the above text would change
eventually to simply say which code points have been allocated.



DD: As a reviewer, when I see code-points in draft, I think this might
be a case of squatting on the code-points and then I need to look up
archive and IANA to make sure. As the document goes for external
reviews I assume this would be the normal reaction, and thus suggest
this update to save up effort for the next set of reviewers. But
totally up to you to make the change or not :)



(3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
definition and usage of ERLD


##PP
The Introduction section has:

"This capability, referred to as Entropy Readable Label Depth (ERLD) as
defined in[I-D.ietf-mpls-spring-entropy-label]"

Is not that sufficient?



This is good, wondering if this should be a normative reference?


##PP2
I can make that informative.

thanks,
Peter


Thanks!
Dhruv




___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr


Re: [Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-10-03 Thread Dhruv Dhody
Hi Peter,

Snipping to open points...

> > (1) Please use updated requirement language text as per RFC 8174, as you do
> > have a mix of upper-case and lower-case terms in your I-D.
> >
> >The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
> >NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
> >"MAY", and "OPTIONAL" in this document are to be interpreted as
> >described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
> >appear in all capitals, as shown here.
>
> ##PP
> RFC 8174 allows the usage of a mix of upper and lower case. If used in
> lower case "they have their normal English meanings", which is the case
> in this draft. Do you have any specific concerns in that regard?
>

DD: You are currently using 2119 requirement language in the front
page. I am suggested to move to 8174.

> >
> > (2) Could you mark that the codepoints mentioned in the draft are early
> > allocated by IANA? This would make it clear that you are not squatting on 
> > them.
> > I also suggest following change in Section 7 (IANA Considerations) -
> >
> > OLD:
> > This document requests IANA to allocate one flag from the OSPFv2
> > Extended Prefix TLV Flags registry:
> >
> >0x20 - E-Flag (ELC Flag)
> >
> > This document requests IANA to allocate one flag from the OSPFv3
> > Prefix Options registry:
> >
> >0x04 - E-Flag (ELC Flag)
> > NEW:
> > IANA is requested to confirm the early allocation of the following
> > code point in the OSPFv2 Extended Prefix TLV Flags registry:
> >
> >0x20 - E-Flag (ELC Flag)
> >
> > IANA is requested to confirm the early allocation of the following
> > code point in the  the OSPFv3 Prefix Options registry:
> >
> >0x04 - E-Flag (ELC Flag)
> > END
>
> ##PP
> I'm not sure above is necessary, given that the above text would change
> eventually to simply say which code points have been allocated.
>

DD: As a reviewer, when I see code-points in draft, I think this might
be a case of squatting on the code-points and then I need to look up
archive and IANA to make sure. As the document goes for external
reviews I assume this would be the normal reaction, and thus suggest
this update to save up effort for the next set of reviewers. But
totally up to you to make the change or not :)


> > (3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
> > definition and usage of ERLD
>
> ##PP
> The Introduction section has:
>
> "This capability, referred to as Entropy Readable Label Depth (ERLD) as
> defined in[I-D.ietf-mpls-spring-entropy-label]"
>
> Is not that sufficient?
>

This is good, wondering if this should be a normative reference?

Thanks!
Dhruv

___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr


Re: [Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-10-03 Thread Peter Psenak

Hi Dhruv,

please see inline (##PP)



On 12/09/2019 12:11, Dhruv Dhody via Datatracker wrote:

Reviewer: Dhruv Dhody
Review result: Has Issues

Subject: RtgDir Early review: draft-ietf-ospf-mpls-elc-09

Hello

I have been selected to do a routing directorate “early” review of this draft.
​https://datatracker.ietf.org/doc/draft-ietf-ospf-mpls-elc/

The routing directorate will, on request from the working group chair, perform
an “early” review of a draft before it is submitted for publication to the
IESG. The early review can be performed at any time during the draft’s lifetime
as a working group document. The purpose of the early review depends on the
stage that the document has reached.

As this document is in working group last call, my focus for the review was to
determine whether the document is ready to be published. Please consider my
comments along with the other working group last call comments.

For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-ospf-mpls-elc-09
Reviewer: Dhruv Dhody
Review Date: 12-09-2019
Intended Status: Standards Track

Summary: I have some minor concerns about this document that I think should be
resolved before it is submitted to the IESG.

The draft is focused and straightforward, the reader needs to be aware of
RFC6790 and draft-ietf-mpls-spring-entropy-label beforehand. I have reviewed
this and the IS-IS I-D together and you will find similar comments for both
I-Ds.

Minor
*

(1) Please use updated requirement language text as per RFC 8174, as you do
have a mix of upper-case and lower-case terms in your I-D.

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
   NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
   "MAY", and "OPTIONAL" in this document are to be interpreted as
   described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
   appear in all capitals, as shown here.


##PP
RFC 8174 allows the usage of a mix of upper and lower case. If used in 
lower case "they have their normal English meanings", which is the case 
in this draft. Do you have any specific concerns in that regard?




(2) Could you mark that the codepoints mentioned in the draft are early
allocated by IANA? This would make it clear that you are not squatting on them.
I also suggest following change in Section 7 (IANA Considerations) -

OLD:
This document requests IANA to allocate one flag from the OSPFv2
Extended Prefix TLV Flags registry:

   0x20 - E-Flag (ELC Flag)

This document requests IANA to allocate one flag from the OSPFv3
Prefix Options registry:

   0x04 - E-Flag (ELC Flag)
NEW:
IANA is requested to confirm the early allocation of the following
code point in the OSPFv2 Extended Prefix TLV Flags registry:

   0x20 - E-Flag (ELC Flag)

IANA is requested to confirm the early allocation of the following
code point in the  the OSPFv3 Prefix Options registry:

   0x04 - E-Flag (ELC Flag)
END


##PP
I'm not sure above is necessary, given that the above text would change 
eventually to simply say which code points have been allocated.




(3) Section 4, I think a reference to RFC 8476 is needed as well to state the
ERLD is advertised as part of Node MSD advertisement as defined in [RFC8476].


##PP
I have updated the normative reference from 
ietf-isis-segment-routing-msd to RFC 8476.



As mentioned in my review of the IS-IS I-D, what happens if one receives ERLD
in the Link MSD advertisement? As per my understanding this is not allowed,
better to add normative text for the case then.


##PP
added a sentence to ignore it in Link MSD Sub-TLV.




(4) Section 8, suggest to also add one sentence for the impact of advertising
incorrect ERLD. If there isn't any, that can also be stated.


##PP added the sentence.



Nits

(1) Suggested ordering of sections - ..ELC/ERLD/BGP-LS/ACK..  [matching between
OSPF/ISIS]


##PP
done



(2) Section 2, add [I-D.ietf-mpls-spring-entropy-label] for terminology
reference


##PP
done



(3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
definition and usage of ERLD


##PP
The Introduction section has:

"This capability, referred to as Entropy Readable Label Depth (ERLD) as 
defined in[I-D.ietf-mpls-spring-entropy-label]"


Is not that sufficient?



(4) Section 6,

The ERLD MSD-type introduced for OSPF in Section 4 is advertised
using the Node MSD TLV (TLV 266) of the BGP-LS Node NLRI Attribute as
defined in section 3 of [I-D.ietf-idr-bgp-ls-segment-routing-ext].

I think you mean draft-ietf-idr-bgp-ls-segment-routing-msd here!


##PP
right, corrected it.



Also, maybe change the title "BGP-LS Extension" as there is no 'extension'
required, ELC/ERLD is BGP-LS would be automatically supported.


##PP
renamed to "Signaling ELC and ERLD in BGP-LS".



(5) Expand MSD on first use.


done.

thanks,
Peter


Thanks!
Dhruv






Re: [Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-09-13 Thread Acee Lindem (acee)
Thanks Dhruv for the review... 

Peter and other authors, 
Please include Dhruv's comments or respond as to why they are being omitted. 

Thanks,
Acee

On 9/12/19, 6:12 AM, "Dhruv Dhody via Datatracker"  wrote:

Reviewer: Dhruv Dhody
Review result: Has Issues

Subject: RtgDir Early review: draft-ietf-ospf-mpls-elc-09

Hello

I have been selected to do a routing directorate “early” review of this 
draft.
​https://datatracker.ietf.org/doc/draft-ietf-ospf-mpls-elc/

The routing directorate will, on request from the working group chair, 
perform
an “early” review of a draft before it is submitted for publication to the
IESG. The early review can be performed at any time during the draft’s 
lifetime
as a working group document. The purpose of the early review depends on the
stage that the document has reached.

As this document is in working group last call, my focus for the review was 
to
determine whether the document is ready to be published. Please consider my
comments along with the other working group last call comments.

For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-ospf-mpls-elc-09
Reviewer: Dhruv Dhody
Review Date: 12-09-2019
Intended Status: Standards Track

Summary: I have some minor concerns about this document that I think should 
be
resolved before it is submitted to the IESG.

The draft is focused and straightforward, the reader needs to be aware of
RFC6790 and draft-ietf-mpls-spring-entropy-label beforehand. I have reviewed
this and the IS-IS I-D together and you will find similar comments for both
I-Ds.

Minor
*

(1) Please use updated requirement language text as per RFC 8174, as you do
have a mix of upper-case and lower-case terms in your I-D.

  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
  NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
  "MAY", and "OPTIONAL" in this document are to be interpreted as
  described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
  appear in all capitals, as shown here.

(2) Could you mark that the codepoints mentioned in the draft are early
allocated by IANA? This would make it clear that you are not squatting on 
them.
I also suggest following change in Section 7 (IANA Considerations) -

OLD:
   This document requests IANA to allocate one flag from the OSPFv2
   Extended Prefix TLV Flags registry:

  0x20 - E-Flag (ELC Flag)

   This document requests IANA to allocate one flag from the OSPFv3
   Prefix Options registry:

  0x04 - E-Flag (ELC Flag)
NEW:
   IANA is requested to confirm the early allocation of the following
   code point in the OSPFv2 Extended Prefix TLV Flags registry:

  0x20 - E-Flag (ELC Flag)

   IANA is requested to confirm the early allocation of the following
   code point in the  the OSPFv3 Prefix Options registry:

  0x04 - E-Flag (ELC Flag)
END

(3) Section 4, I think a reference to RFC 8476 is needed as well to state 
the
ERLD is advertised as part of Node MSD advertisement as defined in 
[RFC8476].
As mentioned in my review of the IS-IS I-D, what happens if one receives 
ERLD
in the Link MSD advertisement? As per my understanding this is not allowed,
better to add normative text for the case then.

(4) Section 8, suggest to also add one sentence for the impact of 
advertising
incorrect ERLD. If there isn't any, that can also be stated.

Nits

(1) Suggested ordering of sections - ..ELC/ERLD/BGP-LS/ACK..  [matching 
between
OSPF/ISIS]

(2) Section 2, add [I-D.ietf-mpls-spring-entropy-label] for terminology
reference

(3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
definition and usage of ERLD

(4) Section 6,

   The ERLD MSD-type introduced for OSPF in Section 4 is advertised
   using the Node MSD TLV (TLV 266) of the BGP-LS Node NLRI Attribute as
   defined in section 3 of [I-D.ietf-idr-bgp-ls-segment-routing-ext].

I think you mean draft-ietf-idr-bgp-ls-segment-routing-msd here!

Also, maybe change the title "BGP-LS Extension" as there is no 'extension'
required, ELC/ERLD is BGP-LS would be automatically supported.

(5) Expand MSD on first use.

Thanks!
Dhruv



___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr


[Lsr] Rtgdir early review of draft-ietf-ospf-mpls-elc-09

2019-09-12 Thread Dhruv Dhody via Datatracker
Reviewer: Dhruv Dhody
Review result: Has Issues

Subject: RtgDir Early review: draft-ietf-ospf-mpls-elc-09

Hello

I have been selected to do a routing directorate “early” review of this draft.
​https://datatracker.ietf.org/doc/draft-ietf-ospf-mpls-elc/

The routing directorate will, on request from the working group chair, perform
an “early” review of a draft before it is submitted for publication to the
IESG. The early review can be performed at any time during the draft’s lifetime
as a working group document. The purpose of the early review depends on the
stage that the document has reached.

As this document is in working group last call, my focus for the review was to
determine whether the document is ready to be published. Please consider my
comments along with the other working group last call comments.

For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-ospf-mpls-elc-09
Reviewer: Dhruv Dhody
Review Date: 12-09-2019
Intended Status: Standards Track

Summary: I have some minor concerns about this document that I think should be
resolved before it is submitted to the IESG.

The draft is focused and straightforward, the reader needs to be aware of
RFC6790 and draft-ietf-mpls-spring-entropy-label beforehand. I have reviewed
this and the IS-IS I-D together and you will find similar comments for both
I-Ds.

Minor
*

(1) Please use updated requirement language text as per RFC 8174, as you do
have a mix of upper-case and lower-case terms in your I-D.

  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
  NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
  "MAY", and "OPTIONAL" in this document are to be interpreted as
  described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
  appear in all capitals, as shown here.

(2) Could you mark that the codepoints mentioned in the draft are early
allocated by IANA? This would make it clear that you are not squatting on them.
I also suggest following change in Section 7 (IANA Considerations) -

OLD:
   This document requests IANA to allocate one flag from the OSPFv2
   Extended Prefix TLV Flags registry:

  0x20 - E-Flag (ELC Flag)

   This document requests IANA to allocate one flag from the OSPFv3
   Prefix Options registry:

  0x04 - E-Flag (ELC Flag)
NEW:
   IANA is requested to confirm the early allocation of the following
   code point in the OSPFv2 Extended Prefix TLV Flags registry:

  0x20 - E-Flag (ELC Flag)

   IANA is requested to confirm the early allocation of the following
   code point in the  the OSPFv3 Prefix Options registry:

  0x04 - E-Flag (ELC Flag)
END

(3) Section 4, I think a reference to RFC 8476 is needed as well to state the
ERLD is advertised as part of Node MSD advertisement as defined in [RFC8476].
As mentioned in my review of the IS-IS I-D, what happens if one receives ERLD
in the Link MSD advertisement? As per my understanding this is not allowed,
better to add normative text for the case then.

(4) Section 8, suggest to also add one sentence for the impact of advertising
incorrect ERLD. If there isn't any, that can also be stated.

Nits

(1) Suggested ordering of sections - ..ELC/ERLD/BGP-LS/ACK..  [matching between
OSPF/ISIS]

(2) Section 2, add [I-D.ietf-mpls-spring-entropy-label] for terminology
reference

(3) Section 3, Add reference to draft-ietf-mpls-spring-entropy-label for the
definition and usage of ERLD

(4) Section 6,

   The ERLD MSD-type introduced for OSPF in Section 4 is advertised
   using the Node MSD TLV (TLV 266) of the BGP-LS Node NLRI Attribute as
   defined in section 3 of [I-D.ietf-idr-bgp-ls-segment-routing-ext].

I think you mean draft-ietf-idr-bgp-ls-segment-routing-msd here!

Also, maybe change the title "BGP-LS Extension" as there is no 'extension'
required, ELC/ERLD is BGP-LS would be automatically supported.

(5) Expand MSD on first use.

Thanks!
Dhruv

___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr