Subject: [Gen-art] Gen-ART Last Call review of draft-ietf-ospf-prefix-link-attr-10.txt

From: Suresh Krishnan <[email-masked]>
Date: Fri, 14 Aug 2015 03:41:08 +0000

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 General Area director. Document editors and WG chairs
should treat these comments just like any other last call comments.

Document: draft-ietf-ospf-prefix-link-attr-10.txt
Reviewer: Suresh Krishnan
Review Date: 2015-08-13
IETF LC End Date: 2015-08-13

Summary: The draft is almost ready for publication as Proposed Standard 
but there are some minor issues that need to be addressed.

* Section 2

* In the packet format described in Figure "OSPFv2 Extended Prefix 
Opaque LSA" the numbers 9, 10 and 11 are shown in a field. I think it 
would be better if these are replaced by the text "LS type" as it is the 
actual field. This will provide consistency with the rest of the figure. 
The following text can then describe the allowed LS types as 9,10, and 11.

s/differential/differentiate/

* Padding of TLVs. I am assuming that the TLVs are padded using zero 
octets. If so, please state it explicitly.

* Section 2.1.

* In the packet format the "Address Prefix" field is marked as variable 
length when it actually *is not*. It is always encoded as 32 bits long, 
right? If so please the "(variable)" designation needs to be removed
for the Address Prefix.

* Shouldn't there be some IANA instructions for further extensions to 
the Flags field in the OSPFv2 Extended Prefix TLV? Or is this field not 
expected to be extended?

* IANA Considerations

* The Opaque LSA Options types used by this document (7 & 8) seem to be 
wrongly entered into the IANA registry and are pointing to 
[draft-ietf-ospf-segment-routing-extensions] instead of this draft. It 
is probably worth taking this up during the IANA check.

Thanks
Suresh

---

From: Peter Psenak <[email-masked]>
Date: Mon, 17 Aug 2015 16:12:08 +0200

Hi Suresh,

please see inline:

> The following text can then describe the allowed LS types as 9,10, and 11.

"9, 10 and 11" is used by RFC 4970, we took it from there. Do you still 
want us to change?

> octets. If so, please state it explicitly.

again, we took the wording from RFC 4970.

RFC2370/RFC5250 that specifies the base Opaque LSA, does not specify 
what values should be used for padding. I believe we should not mandate 
it in the ospf-prefix-link-attr specification.

> right? If so please the "(variable)" designation needs to be removed

Address Prefix is a variable length field - it is an encoding of the 
prefix itself as an even multiple of 32-bit words, padding with zero 
bits as necessary. The encoding as such supports IPv4 and IPv6 
addresses, based on the "AF" field, so it can be extended for IPv6 if 
needed later.

> the Flags field in the OSPFv2 Extended Prefix TLV? Or is this field not

Flags field can be extended. Agree that it is a good idea to add it to 
IANA section.

> is probably worth taking this up during the IANA check.

they point to the old OSPF SR draft which was split later and from which 
the ospf-prefix-link-attr was carved. Once the RFC is published IANA 
registry will be updated accordingly.

thanks,
Peter

---

From: Suresh Krishnan <[email-masked]>
Date: Mon, 17 Aug 2015 22:13:59 +0000

Hi Peter,
   Thanks for your quick response. Please find comments inline.

> "9, 10 and 11" is used by RFC 4970, we took it from there. Do you still

I leave it to you discretion, but I found it very strange.

> it in the ospf-prefix-link-attr specification.

Sure. No problems.

> addresses, based on the "AF" field, so it can be extended for IPv6 if

This draft is applicable only for OSPFv2 right? If you think this could 
be extensible to IPv6, then you probably need additional text about 
future extensibility in the description of the Address Prefix field 
which states "The prefix itself encoded as a 32-bit value". I don't mind 
either way but the figure and the text need to be consistent.

> Flags field can be extended. Agree that it is a good idea to add it to

Sounds good.

> registry will be updated accordingly.

Great. I put this in there so that somebody can check whether this 
happens during IANA processing of the draft. Glad that you are on it.

Thanks
Suresh