Re: [OSPF] Genart last call review of draft-ietf-ospf-segment-routing-extensions-19

Hi Dan,

please see inline:

On 05/10/17 13:05 , Dan Romascanu wrote:
> Reviewer: Dan Romascanu
> Review result: Ready with Issues
>
> 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
>
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>
> Document: draft-ietf-ospf-segment-routing-extensions-19
> Reviewer: Dan Romascanu
> Review Date: 2017-10-05
> IETF LC End Date: 2017-10-13
> IESG Telechat date: Not scheduled for a telechat
>
> Summary:
>
> A useful and well-written document. It requires previous reading and
> understanding of OSPF, SPRING and other routing work. It is Ready for
> publication. I found some unclear minor issues. I recommend to address them
> before approval and publication.
>
> Major issues:
>
> Minor issues:
>
> 1. I am wondering why, at this stage of progress of the document, the type
> values are still 'TBD, suggested value x'. Is there any other document defining
> this?

good point, fixed it.

>
> 2. Section 3.1 - are there other algorithms planned to be added in the future?
> If yes, do we need a registry? If no, what is this field an octet?

yes, we know about this. The question is where do we want to put this 
registry - draft-ietf-spring-segment-routing defines Prefix-SID 
Algorithms 3.1.1. Both IGP SR drafts defines the same algorithms. This 
looks redundant. We need to keep it either in 
draft-ietf-spring-segment-routing and define a registry there and remove 
the definitions from IGP SR drafts and simply point to them to 
draft-ietf-spring-segment-routing. An alternative is to remove the 
section 3.1.1 from teh draft-ietf-spring-segment-routing and keep the SR 
Algorithm definition in IGP SR drafts and define the registry in one of 
them.

I need to talk to authors of draft-ietf-spring-segment-routing to 
resolve this.

>
> 3. It would be useful to mention that the Length fields are expressed in
> Octets. Also please clarify if padding is applied or not.

I fixed those cases where the "octets" was missing in the Length 
specification.

For the padding, all OSPF TLVs are always padded to 4-octet alignment. I 
don't believe we need to specify that here again.

>
> 4. Section 3.3:
>
> 'The originating router MUST NOT advertise overlapping ranges.'
>
> How are conflicts resolved at receiver?

SRLB sub-TLV is not used by routers running ISIS. The advertisement is 
only there for the benefit of external entities such as controllers so 
they can determine what labels are available for assignment. We do not 
define controllers behavior in our drafts.

>
> 5. I like Section 9 - Implementation Status - which I found rather useful. Is
> there any chance to keep a trimmed down version of it, with synthetic
> information on the lines of 'at the time the document was discussed a survey
> was run, it showed that there were x implementation, y were implementing the
> full specification, z were included in released production software ....'

I'm not sure, typically such section is removed prior to publication. I 
have no problem keeping it if that is allowed.

>
> 6. Section 10 - beyond recommending the counting and logging of the mal-formed
> TLVs and sub-TLVs, should not supplementary security recommendations be made?
> for example - throttling mechanisms to preempt DoS attacks.

added following statement:

"Logging of malformed TLVs and Sub-TLVs should be rate-limited to 
prevent a Denial of Service (DoS) attack (distributed or otherwise) from 
overloading the OSPF control plane. "

thanks,
Peter
>
> Nits/editorial comments:
>
>
> _______________________________________________
> OSPF mailing list
> OSPF@ietf.org
> https://www.ietf.org/mailman/listinfo/ospf
> .
>