[RTG-DIR] RtgDir QA review: draft-ietf-ospf-segment-routing-extensions-12

[Stig Venaas <[e-mail]>]

Hello,

I have been selected as the Routing Directorate reviewer for this
draft. This is just an early QA review.

The draft is in good shape, but I did find some minor issues and nits.
It is fairly readable, but it could be improved in a few places.


Minor issues:

In 3.1:
The SR-Algorithm TLV is some places called a Sub-TLV. It might be
good to be consistent.

This is not clear in 3.1:
   The SR-Algorithm Sub-TLV is optional. It MAY only be advertised once
   in the Router Information Opaque LSA.
Is this trying to say that it MUST NOT be advertised more than
once? With the current wording this is not obviously that strict.

I see some text regarding multiple SR-Algorithm sub-TLVs, but it also
looks like one can have multiple algorithms in one sub-TLV. At least
from the diagram. But I don't see any discussion about this. Is it OK
to add multiple? When can it be done, what does it mean? What if
routers don't support the exact same set of algorithms?

The term "lowest flooding scope" is used a couple of places. I think I
know what it means, but it might be good to point it out. Also, I'm
used to seeing the term "smallest" rather than "lowest". I'm assuming
they mean the same.

In 3.2 there is this bullet point:
   The receiving router must adhere to the order in which the ranges
   are advertised when calculating a SID/label from a SID index.

You probably should use MUST here.

Section 4:
In section 4 there is a range for advertising a range of prefixes.
But it looks like it contains a single prefix length and it says
the length is the length of the prefix. While it says range size
is the number of prefixes. I don't understand from the text what
really prefix length and range size means and how this should be
used.

I understand this is IPv4 only since OSPFv2, but rather than just
saying IPv4 is 0, maybe refer to an IANA AF registry? This might
be helpful if you want to use the same sub-TLV in OSPFv3 and
use the same code for parsing etc. IANA has 1 for IPv4 though.

Section 5:
Is it intentional that the flags start in position 1 rather than
0?

I see that the NP flag should be ignored when M is set. Then I
see this text:
   As the Mapping Server does not specify the originator of a prefix
   advertisement, it is not possible to determine PHP behavior solely
   based on the Mapping Server advertisement.  However, PHP behavior may
   safely be done in following cases:
This seems not very precise. Could you say exactly what the behavior
should be, rather than saying "behavior may be done"?

Section 6:
It might be good to make clear that other flag positions are
reserved, set to 0 and ignored... Perhaps also point out that
weight is in the range 0-255.

I see this sentence:
      If the SID/Label Sub-TLV appears in the SID/Label Binding Sub-TLV
      more than once, instances other than the first will be ignored and

Should it say MUST be ignored?

Section 6.2 it says:
   All ERO Sub-TLVs must immediately follow the SID/Label Sub-TLV.
   All Backup ERO Sub-TLVs must immediately follow the last ERO Sub-TLV.

Should these be normative MUSTs?

In 6.2.1:
It would be good for all of these to specify that other flags are
reserved.


Nits:
The intro should perhaps mention LAN adjacency and binding SIDs?

2nd paragraph of section 2 is confusing. It sounds like
the Opaque LSAs in 7684 were defined for SID in particular,
but it is a generic mechanism. Perhaps SID was the
motivation though?

Section 6.1:
It says:
   The ERO Metric Sub-TLV advertises the cost of an ERO path.  It is
   used to compare the cost of a given source/destination path.  A
   router SHOULD advertise the ERO Metric Sub-TLV in an advertised ERO
   TLV.

Is the ERO TLV the ERO Sub-TLVs defined in 6.2? It would be good to
point that out.

In 8.4.2:
   Broadcast, NBMA or or hybrid
Extra "or".

Section 9:
There are no new registries and most of the TLVs are already
allocated? It seems there are a few new ones where it should
probably say TBD, or say something about being suggested values.
That was done in earlier sections. I see in some places it says
"are allocated" here, while it says "suggested" in the definition
of the TLV.

Section 10:
It says there are responses from 2 implementers, but I see 3.

Section 11:
Are these really all the potential security issues?

I'm on vacation the next 2 weeks, so I may not reply to any
emails during that period.

Regards,
Stig