Re: [Pce] Early code point allocation for draft-ietf-pce-segment-routing-policy-cp

[tom petch <[e-mail]>]

From: Pce <[e-mail]> on behalf of tom petch <[e-mail]>
Sent: 20 February 2021 12:33

From: Pce <[e-mail]> on behalf of tom petch <[e-mail]>
Sent: 19 February 2021 12:30

From: [e-mail] <[e-mail]>
Sent: 18 February 2021 10:35

Hi Tom,

Thank you for your valuable feedback.

<tp>

The more I look, the less I like it.

This I-D asks for an error code for missing mandatory TLV, a category which PCE has defined  as Error-Type 6 and is referenced  by such as pce-vn-association.

Why does this I-D put missing mandatory TLV in a different Error-Type?

I will raise some more issues next week.

<tp2>
Next week and I like this I-D even less.

The I-D says repeatedly that only one instance of a particular TLV is allowed and others must be ignored.  Where the TLV is optional, this may make sense but where it is mandatory and forms part of the identifier then I think this wrong.  The sender is in effect saying I am confused about which association I am referring to (as indicated by including more than one).  That should be an error IMHO.

In passing I find the error messages poor.  I was looking at 
draft-ietf-pce-associated-bidirectional-lsp
and thinking how clear and easy to understand those error messages are, which I would not say of this I-D; the authors should look and learn.

A more significant problem is the identification of policy.  I assume that when this I-D talks of policy then it means SR policy.  (Sometimes it says so, sometimes it does not - needs fixing.)  This I-D says that a policy identifier consists of color, endpoint and optionally name.  

This raises a number of issues.  I raised earlier the point that an optional label as part of an identifier is a good candidate for erroneous implementations.  In fact it is more challenging than that.  The topic of optional keys came up in the development of YANG, were considered and rejected.  I forget the reasoning, whether they led to logical impossibilities or were too error-prone to implement or what, but they are not allowed.  So you cannot model in YANG anything with an optional key which is what this I-D seems to propose.

Separately, this I-D seems to contradict 
draft-ietf-spring-segment-routing-policy
which is clear that an SR policy is identified by the tuple <headend, color, endpoint> which is not what this I-D says (and which, in passing,  can be modelled in YANG).  Yes this PCE I-D mentions the context of headend but does not say where that should be inferred from to form the unique SR Policy identifier.

As you may infer, I do not see this I-D as fit for an early IANA allocation.

Tom Petch


Some of the issues you point out are easy to address and we've already
requested the authors to revise the I-D accordingly. To fully resolve
your concern, could you please point any other specific parts where you
feel you have to "interpret the words the way you think they should have
been"? If you even have some text to suggest, that could smoothly ease
the update.

<tp>
At a first glance,

s.7.1
RFC8697 names three  columns for the registry; those names do not appear here.

The new association type is given a different identifier in different places.  The preferred identifier needs to be nailed down since it is going into IANA forthwith and will be confusing to change thereafter.

s.7.2
This requests new error values  under Association Error  and specifies a Type of 29.  RFC8697 specifies a Type of 26 (29 is Path Computation Failure).

'Conflicting SRAG TLV' I find rather vague; conflicting with what?  Likewise 'Multiple SRPAG from one LSP'

s.7.3
This document defines five new TLVs
That is TBD3, TBD4, TBD11, TBD5 and ....

RFC8697 specifies the names of the fields in the registry,  Those names are not used here.

s.3.2
'as of the time of writing' will change its meaning as the I-D progresses; date needed

s.4.1
'is only meant to be used'
MUST NOT, SHOULD NOT, .....?

'Policy Identifiers uniquely identify..
Policy Identifiers consist of Color.. Endpoint, optionally the policy hame.
So if one is Color red, Endpoint NY no policy name
and then one is requested for
Color red, Endpoint NY, policy name standby
that is a different triplet and so valid.  Mmm. I can see that being mis-implemented

s.4.2

'is meant to strictly correspond'
MUST, SHOULD, ?

s.5
This document specifies four new TLVs...
These five TLVs .....

These five TLVs encode the Policy Identifiers, SR Policy name, Candidate path identifiers, candidate path name and Candidate path preference..
That is five TLV. Wrong! That is four TLV and something completely different.


When any of the mandatory TLVs
Only one TLV is listed as mandatory SRPOLICY-CPATH-ID.

s.5
At most only one .. can be carried
and then goes on to describe the carriage of  more than one;
'Only one ... SHOULD be present in a ... (whatever identifier you fix on) message.  If more than one is present, only the first is processed and subsequent ones are silently discarded.

A Normative Reference to an unadopted I-D that expires next week is not a good look:-)

Like I said, the word that came to my mind was 'sloppy':-(

Tom Petch

Thanks,

Dhruv & Julien


On 17/02/2021 12:46, tom petch wrote:
> <snip>
> <tp>
> I am sure that IANA will cope because they always do, but it will be by reading between the lines, applying intelligence to what the authors may have meant, and so on.  Editorially this is a poor I-D (as yet), and that quality verges on the technical aspects.
>
> Thus 7.3 says the I-D defines five new TLV and lists four; this also occurs in the body of the I-D.  A reader might also notice the absence of TBD2 and wonder.
>
> Or the new Association.  Thus needs an identifier.  Trouble is, the I-D uses (at least) three different ones; this looseness of terminology can lead to problems down the line.  (MPLS I see as a classic in how not to specify IANA registries and I see this heading the same way).
>
> Likewise the identifiers used in s.7 do not match those in current use, a good way of storing up future trouble.
>
> Is the specification adequate?  Only if you do not take it literally and interpret the words the way you think they should have been.
>
> Tom Petch
>


_______________________________________________
Pce mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/pce

_______________________________________________
Pce mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/pce