Re: [mpls] Rtgdir last call review of draft-ietf-mpls-sfl-control-03

[Stewart Bryant <[e-mail]>]

Thank you again Stig

Please see inline

Stewart

> On 2023-08-10 00:28, Stig Venaas via Datatracker wrote:
> Reviewer: Stig Venaas
> Review result: Has Nits
> The only somewhat substantial comment I have is regarding this:
> In section 3.1:
>   Editors Note - We need to revisit the RFC6374 errors and the protocol
>   to see if we need some more error codes.
> Has this been done? This note should be removed before publication.
> The remaining comments are all editorial.

I have just looked at these and I think that there are a number of errors that might be of use to an operator.

The protocol will work without them so the proposals are backwards compatible,

I propose we add

    0x13: Error - Unsupported Control Code.  Indicates that the
    operation failed because the Control Code requested an
    operation that is not available.

    0x14: Error - Unknown Batch.  Indicates that the batch specified
    in the SFL-Refresh or SFL-Withdraw Query was unknown.

    0x15: Error - Administrative Block.  Indicates that the
    operation failed because it has been administratively
    disallowed.

    0x16: Error - Resource Unavailable.  Indicates that the
    operation failed because node resources were not available.

    0x17: Error - Resource Released.  Indicates that the operation
    failed because the specified SFLs were administratively released.

    0x18: Error - Invalid Message.  Indicates that the operation
    failed because the received query message was malformed.

    0x19: Error - Protocol Error.  Indicates that the operation
    failed because a protocol error was found in the received query
    message.

13 is direct from RFC6374
14 is new but a useful diagnostic


Chairs/ADs please confirm that you are happy with the additional error messages. 



> In abstract:
>   extend the lifetime of such labels.  It is not the only control
>   protocol that might be used to support SFL, but it has the benefit of
>   being able to be used without modifying of the existing MPLS control
>                                      ^^^^^^^^^^^^
>   protocols.  The existence of this design is not intended to restrict
> Should say “modifying the existing” or “modification of the”
> Same in section 1.

I used the second option which was clearly the language originally intended.

> I don’t think there should be normative language in the abstract. It has MUST and SHOULD.

The last para of the abstract was a spurious cut and paste- I think from when I recreated the 
.md from a text version of the draft. I have removed the para.

As an aside the style guide seems to be silent on the use of RFC2119 language in the abstract 
but thanks of rpicking this up I would have missed the bigger error in the draft.

> In section 3.1. Missing period at the end.
>   0x3: SFL Withdraw-Ack. This indicates that the responder has
>   received the Withdraw message and will withdraw the SFLs

Fixed

>   1 (Request (R))  Indicates to the Querier that this member of the
>                  SFL batch is requested.  Where a value is specified
>                  in the request, but the Responder is unable honour
>                                                         ^^^^^^^
>                  that request, no SFL is allocated and the
>                  corresponding A flag MUST be cleared.
> Should be “Is unable to honour”

Fixed

> In 3.2.1:
>   If the Responder is unable to allocate all of the requested SFLs it
>   MUST respond with a response code of SFL-Unable.  The Querier MUST
>   determine whether the allocated SFLs were adequate for its purposes
>   and MUST send a withdraw if there are not adequate.  A Querier MUST
>                              ^^^^^^^^^^
>   NOT attempt to hoard labels in the hope that the residual labels
>   needed may become available in the future.
> “if they are not adequate”?

I have changed this to:

The Querier MUST
determine whether the allocated SFLs were adequate for its purposes
and MUST send a withdraw if insufficient SFLs were allocated.

> In section 4:
> 4.  Return Path
>   Where the LSP (or other MPLS construct) is multi-point to point, or
>   multi-point to multi- point the RFC6374 Address TLV MUST be included
>   in Query packet, even if the response is requested in-band, since
>   this is needed to provide the necessary return address for this
>   request.
> Remove space “multi- point”.

Fixed

Attached the new text and the diff

I will upload when the embargo lifts or can upload earlier if Andew is happy to approve.

Best regards

Stewart