Subject: [Pals] Shepherds review of draft-ietf-pals-status-reduction-00

From: Stewart Bryant <[email-masked]>
Date: Thu, 9 Jun 2016 19:54:55 +0100

Hi Authors

I have done a shepherd's review of this draft and have a few issues
that need to be looked. It would be timely if you could fix these during the
IPR poll I am about to start so that we can have cleaner text to
take to the WG for last call.

Please see below.

- Stewart

  Abstract

    This document describes a method for generating an aggregated
    pseudowire status message on Multi-Protocol Label Switching (MPLS)
    network Label Switched Path (LSP).

    The method for transmitting the pseudowire (PW) status information is
    not new, however this protocol extension allows a Service Provider
    (SP) to reliably monitor the individual PW status while not
    overwhelming the network of multiple periodic status messages. This
SB> s/of/with/

    is achieved by sending a single cumulative summary status
    verification message for all the PWs grouped in the same LSP.
SB> grouped in or grouped on?

============

      * Session ID

        A non-zero, locally selected session number that is not preserved
        if the local PE restarts.

        In order to get a locally unique session ID, the recommended
        choice is to perform a CRC-16 giving as input the following data

        |Y|Y|M|M|D|D|H|H|M|M|S|S|L|L|L|

        Where:  YY: are the decimal two last digit of the current year
        MM: are the decimal two digit of the current month DD: are the
        decimal two digit of the current day HHMMSSLLL: are the decimal
        digits of the current time expressed in (hour, minutes, seconds,
        milliseconds)

SB> What happens if there is a hash collision?
============
  
      * Refresh Timer.

        A non zero unsigned 16 bit integer value greater or equal to 10,
        in milliseconds, that indicates the desired refresh interval. The
        default value of 30000 is RECOMENDED.

SB> Do you need to specify a min of 10ms, or would it be adequate to
SB> to recommend this lower value?

===========

5. PW status refresh reduction Control Messages

    There can only be one control message construct per PW status refresh
    reduction Message. If the U bit is set, and a PE receiving the PW
    status refresh reduction Message does not understand the control
    message, the control message MUST be silently ignored. However the
    control message sequence number MUST still be acknowledged by sending
    a null message back with the appropriate value in the Last Message
    Received Field.

SB> The above is a little odd. On the one hand you are saying that
SB> that the message is not understood, but on the other hand that you
SB> expect the node to execute an element of the protocol procedure.
SB>
SB> Presumably it is a particular message type that is not understood?

==============

5.0.1. Notification message

    The most common use of the Notification Message is to acknowledge the
    reception of a message by indicating the received message sequence
    number in the "Last Received Sequence Number" field. The notification
    message is encoded as follows:

     0                   1                   2                   3
     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |         Checksum              |    Message Sequence Number    |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |  Last Received Seq Number     |  Type=0x01    |U   Flags      |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |                      Notification Code                        |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

    The message type is set to 0x01, and the U bit is treated as
    described in the above section.

SB> It might be better to give the explicit section number.
==========

5.0.2. PW Configuration Message

    The PW status refresh reduction TLVs are informational TLVs, that
    allow the remote PE to verify certain provisioning information. This
    message contain a series of sub-TLVs in no particular order, that
    contain PW and LSP configuration information. The message has no
    preset length limit, however its total length will be limited by the
    transport network Maximum Transmit Unit (MTU).

SB> How do you know the MTU?

========

5.0.2.2. PW ID configured List

    The number of PW Path IDs in the TLV will be inferred by the length
    of the TLV up to a maximum of 8.  The procedure for processing this
    TLV will be described in a section below.

SB> It would be kind to the reader to give the section number.

===========

  5.0.2.3. PW ID unconfigured List

    This OPTIONAL TLV contains a list of the PWs that have been
    unprovisioned on the LSP. Note that it is a fatal session error to
    send the same PW address in both the configured list TLV , and the
    unconfigured list TLV in the same configuration message.

SB> What action is taken on a fatal session error?

=========

6. PW provisioning verification procedure

    This procedure and the advertisement of the PW configuration message
    are OPTIONAL.

    A PE that desires to use the PW configuration message to verify the
    configuration of PWs on a particular LSP, should advertise its PW
    configuration to the remote PE on LSPs that have active keepalive
    sessions.  When a PE receives PW configuration information using this
    protocol and it not supporting or not willing to use the information,
    it MUST acknowledge all the PW configuration messages with a
    notification of "PW configuration not supported".

SB> How does a PE acknowledge a protocol it does not support?

===========

       -iii. An Alarm MAY be raised to a network management system.

SB> Shouldn't that be SHOULD be raised?

===========
  6.1. PW ID List advertising and processing

    If a new PW is added to a particular LSP, the PE MUST place the
    configuration verification of this PW on hold for a period of at
    least 10 seconds. This is necessary to prevent false positive events
    of mis-configuration due to the ends of the PW being slightly out of
    sync.

SB> Prevent or minimise?

================

7. Security Considerations

    Section to be completed in a later version of the document.

SB> Sorry author's we can't that this to WG LC with this security
section!

8. IANA Considerations

8.1. PW Status Refresh Reduction Message Types

    IANA needs to set up a registry of "PW status refresh reduction
    Control Messages".

SB> This is presumably a registry contained in the PW Name Spaces

===========

8.2. PW Configuration Message Sub-TLVs

    IANA needs to set up a registry of "PW status refresh reduction
    Configuration Message Sub-TLVs".

SB> This is presumably a registry contained in the PW Name Spaces

SB> Actually do you need to create a sub registry to hold all three
SB> of these new registries. I would think you did to keep them
SB> together.

=============

8.3. PW Status Refresh Reduction Notification Codes

    IANA needs to set up a registry of "PW status refresh reduction
    Notification Codes".

SB> This is presumably a registry contained in the PW Name Spaces
==========

    These are 32-bit values. Type value 1 through 7
    are defined in this document.

SB> Actually 0 through 7 are defined aren't they?
=========

SB> What is the interaction between this protocol and PW protection?

===========================
===========================

---

From: Luca Martini <[email-masked]>
Date: Fri, 17 Jun 2016 10:40:25 -0600

>    overwhelming the network of multiple periodic status messages. This
ok.
> SB> grouped in or grouped on?
pw are inside an LSP . I think it's in.

ok, I'll take care of this by just adding 1 to the number until it is
locally unique.
( unless you have a better suggestion ? )
I will add the following:

If the calculation results in an already existing session ID, a unique
session ID can be generated by adding 1 to the result until the session
ID is unique.
Any other method to generate a locally unique session ID is also acceptable.

I'm not clear on your question. We are recommending a default, the value
can be set to anything.
anything below 10ms would be a bit silly.
not quite. this is standard LDP like behavior. The idea is that a new
implementation sending something additional that is not yet implemented
in an old implementation should just work.
say for example that someone starts sending some new extra information,
and some peers are yet to be upgraded. We want that to just be ignored.
We have to update the sequence number otherwise the message queue would
just stop.

I hate explicit section numbers , as they always end up wrong ... Is the
section not Above ?
As you remember we have errata on other documents on this point....

The physical link is attached to the PE , so the PE does know the MTU of
the link. Remember that this is only going to the neighbor, the path
only has one hop.

> SB> It would be kind to the reader to give the section number.
same comment - anytime i have done this sections numbers got out of
order ...
It is explained in section "5.0.1    Notification message"
However, this text should explain which error code to send .....
the following text will be added:
If the this error occurs , an error notification message is returned
with the error code of "PW Configuration TLV conflict" and the session
is terminated by entering STARTUP state.

> SB> How does a PE acknowledge a protocol it does not support?
ok, i see that the text is not clear. the notification is meant to
indicate that this procedure is not supported.
I will change the top text to be :
The advertisement of the PW configuration message is OPTIONAL.

This really depends on local PE implementation and policy. It is not
necessary for the protocol implementation.
I can think of examples where an alarm is not desirable. For example of
a PW is just being configured  manually, and the remote end is not yet
added.
I would rather leave this to a local implementation preference then
force it with a SHOULD.

Yes minimize is a better term. Will fix. I will also move the number to
30s as it seem s a bit more appropriate.
oops i missed this .....
I will write it in.
The security considerations of [RFC6478] are adequate for the proposed
mechanism since the operating environment is almost identical to to the
one where this protocol would be deployed. It should also be noted that
since this protocol is defined  to be deployed between two adjacent PEs
connected by a physical link, it is not possible to misdirect or inject
traffic without compromising the PW transport link itself, all these
situations are covered in the security considerations of [RFC6478].
> SB> This is presumably a registry contained in the PW Name Spaces
yes , the names has PW in it , i would assume that IANA would put it
there .Do i need to add something to the IANA instructions ?

Since this is really a trivial IANA organization issue, let's discuss
this in person, and I will add the necessary IANA instructions as you
seem fit.

yes -fixed.
> SB> What is the interaction between this protocol and PW protection?
There is no real interaction other then this can be another source of
information to set the PW state.

 =========================== ===========================
Luca
>
>
> _______________________________________________
> Pals mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pals

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 17 Jun 2016 18:56:09 +0100

Hi Luca

We pretty much agree on everything. Please see inline for some notes and 
a couple of typos.

> pw are inside an LSP . I think it's in.

Genuinely not sure - will leave it to the RFC editor.

> Any other method to generate a locally unique session ID is also acceptable.

That would work, or you could say any use any method you like so long as you
search for  value that is unique amongst the currently known Session IDs.

Is there any reason no to preserve the number? Surely it make n/w mgt 
easier?

> anything below 10ms would be a bit silly.

With hindsight, yes you are right, leave it.

> We have to update the sequence number otherwise the message queue would
OK, I understand.

> As you remember we have errata on other documents on this point....
Ah, but not if you automatically generate them as I (and RFCed) do :)

The problem can arise if you add a section in and forget about the implicit
pointer.

I will not press the point.

> the link. Remember that this is only going to the neighbor, the path
OK.
> same comment - anytime i have done this sections numbers got out of
Same reply :)
> is terminated by entering STARTUP state.
Good.
> The advertisement of the PW configuration message is OPTIONAL.
Thanks
> I would rather leave this to a local implementation preference then
OK, maybe you need to put in a little text along the lines you just said 
to me.
If you want to leave it OK, but I anticipate a bunch of AD/Dir comments.

> mechanism since the operating environment is almost identical to to the
s/to to/to /
> since this protocol is defined  to be deployed between two adjacent PEs
defined or designed?
> there .Do i need to add something to the IANA instructions ?
It is common to specify the namespace. IANA are always nice to people, so
it is always good to make the instruction clear to help them out.

> this in person, and I will add the necessary IANA instructions as you
OK.
> information to set the PW state.
OK

If you do the edits we look good to go.

Stewart

---

From: Luca Martini <[email-masked]>
Date: Mon, 20 Jun 2016 11:36:13 -0600

Stewart,

Answers in line - thanks for the prompt response.
Luca

On 06/17/16 11:56, Stewart Bryant wrote:
> Hi Luca
>
> We pretty much agree on everything. Please see inline for some notes
> and a couple of typos.
>
>
I agree.
> Is there any reason no to preserve the number? Surely it make n/w mgt
I did not see a reason to force any implementation to preserve the
number. It certainly is not necessary for inter-operation of PEs.
Since the number is only unique to a PE , I would expect it to not be
used to identify links on it's own.

> I will not press the point.
yes, I am not sure why ... but in the past it was the rfc editor that
caused the issue ... ( AD sending a delete section message , and section
numbers not getting updated in the text)

> OK, maybe you need to put in a little text along the lines you just
> said to me.
ok- I will change to SHOULD
>> mechanism since the operating environment is almost identical to to the
fixed.
>> since this protocol is defined  to be deployed between two adjacent PEs
Designed.
Also reading the security section on those references , they are exactly
what is needed here. So i'm pretty happy that this section is adequate.

> it is always good to make the instruction clear to help them out.
ok will add the reference to "PW Name Spaces".

---

From: Stewart Bryant <[email-masked]>
Date: Tue, 28 Jun 2016 12:06:36 +0100

======
> used to identify links on it's own.

You are correct in terms of interoperability. I am not sure what view 
the OPs folks
will take.

Why don't we leave it as non-preserving then and see what they say.

Stewart