Subject: [mpls] More comments from another Reviewer//Re: MPLS-RT review of draft-bryant-mpls-sfl-control

From: Zhenghaomian <[email-masked]>
Date: Fri, 3 Jan 2020 02:44:08 +0000

Hi, All,

Happy new year to you all.

I am another MPLS-RT reviewer for this document, and would like to follow up in this thread, by starting from reviewing the latest draft-bryant-mpls-sfl-control-05. Generally I think this is a complete work with self-contained solution, and I assume the importance of the document has been evaluated by the WG chairs and experts. Some comments are listed below as a ‘MPLS beginner’☺

1.       Section 1 Introduction
"well managed" should be "well-managed"; -> this also applies for section 7 security considerations.
 "withdrawn" should be "withdraw", -> this also applies for the abstract text.

2. The RFC6374 looks slightly different than other RFC citations, as the '[]' is missing; please check the formatting.

3. section 3.1 SFL Control Message
I am curious on the bits allocation "26-bit Session Identifier" and "6-bit SFL Batch", which is not typical (my intuitive is 24+8), and there is no reason in the explanation text below the TLV. We need to confirm 1) the allocation is intended to be 26+6; 2) add something to the TLV explanation, for example:

OLD:
SFL Batch      Used where the SFLs for this Session Identifier
               managed across multiple SFL Control Messages.  A given
               set of SFLs MUST be retained in the same batch.
NEW:
SFL Batch (6 bits) Used where the SFLs for this Session Identifier
                  managed across multiple SFL Control Messages.  A given
                  set of SFLs MUST be retained in the same batch.

4. section 3.1 SFL Control Message
Regarding the flags, my understanding is that the intention is to use the first bit to indicate query/respond, and pad the other 3 bits as 0. The processing of '0' bits is understood as "MUST be set to 0 by sender and ignored by receiver". Is it correct? If yes, the following statement may be easier:
OLD:
Flags: The format of the Flags field is shown below.

                              +-+-+-+-+
                              |R|0|0|0|
                              +-+-+-+-+

                        SFL Control Message Flag

   R: Query/Response indicator.  Set to 0 for a Query and 1 for a
      Response.

   0: Set to zero by the Sender and ignored by the Receiver.

NEW:
Flags (4 bits): The first bit is set to 0 for a query and 1 for a
response; the next 3 bits MUST be set to 0 by the Sender and ignored
by the Receiver.

5. section 3.1 SFL Control Message
Regarding the 'Control Code', do we allocate specific value for the query/response actions? For example, we can have request (control code = 0), refresh (control code = 1) and withdraw (control code =2) for a query. Currently there is no such allocations.

6. section 3.2.1.  Request/Grant
In the first paragraph, when you say 'It sets ...' and 'It chooses ...', does the “it” refer the Querier?
This problem confused me some times in later sections as well.

7. section 4. Return Path
'mulit-point' -> 'multi-point'
The usage of comma and bracket confused me, does it mean the following?
Where the MPLS LSP (or other construct) is a 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 it is
necessary to provide the necessary return address for this request.

Thank you.

Best wishes,
Haomian

发件人: mpls [mailto:[email-masked]] 代表 Stewart Bryant
发送时间: 2020年1月3日 0:28
收件人: Andrew G. Malis <[email-masked]>
抄送: [email-masked]; mpls-chairs <[email-masked]>; mpls <[email-masked]>
主题: Re: [mpls] MPLS-RT review of draft-bryant-mpls-sfl-control

Fixed in the master I will use for the next release.

- Stewart

On 2 Jan 2020, at 14:47, Andrew G. Malis <[email-masked]<mailto:[email-masked]>> wrote:

Stewart,

Looking at the diff from the previous version, I just noticed a typo that I missed earlier. In the first paragraph of the introduction, "withdrawn" should be "withdraw". I also see that you added an Oxford comma, which is fine by me! :-)

Cheers,
Andy

On Thu, Jan 2, 2020 at 9:40 AM Andrew G. Malis <[email-masked]<mailto:[email-masked]>> wrote:
Stewart,

Glad to be of assistance.

Cheers,
Andy

On Thu, Jan 2, 2020 at 9:09 AM Stewart Bryant <[email-masked]<mailto:[email-masked]>> wrote:

Hi Andy

Thank you for the review.

> On 30 Dec 2019, at 19:31, Andrew G. Malis <[email-masked]<mailto:[email-masked]>> wrote:
>
> All,
>
> I’ve been selected as an MPLS-RT reviewer for draft-bryant-mpls-sfl-control,
> which is currently a candidate for MPLS WG adoption.
>
> In general I believe that the draft is ready for WG adoption. However, I have a few minor comments which may be addressed either before or after WG adoption.
>
> 1. The Security Considerations section says "It is assumed that this protocol is run in a well managed MPLS network with strict access controls preventing unwanted parties from generating MPLS OAM packets." While this is true of most (all?) MPLS networks, this assumption should also be stated in the abstract or Introduction as well.

I have added a line to the Introduction.

I generally work on the assumption that the purpose of the Abstract is to help the potential reader decide if they want to read the document (or more likely to help a search engine match the document against a query) and I am unconvinced as to value of the qualification text in deciding whether to read the rest of the document.

>
> 2. Section 8 contains the line "Force references to appear with mkd [RFC3032] [RFC5036]". However, both references appear elsewhere as well, so this line can be removed.

The earlier references are in text that I included as a figure to force the layout style I wanted, and the markdown compiler ignores references in figures. The compiling process excluded unused references so they have to be forced. I have changed the text to

RFC Editor please remove this note which is used to force the following references to appear {{RFC3032}} {{RFC5036}}

>
> 3. I would move "I-D.ietf-mpls-sfl-framework" to the Normative References, as understanding it is necessary to understand this draft.
>

The framework is informational and so the reference would become a down-ref. I will leave it to the chairs and ADs on how they want to proceed with this. It is not unusual for a framework to be required reading and yet informational.

> 4. The text in Section 2 is out of date. The current wording is:
>
> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all capitals, as shown here.
>
> RFC 8174 should also be added as a normative reference.

Done

>
> 5. When this draft was last updated, Stewart included the following in an email message to the MPLS WG:
>
> "Next to do is authors of draft-bryant-mpls-sfl-control to check that
> this is suitable to be called by another protocol to act on its behalf.
> When we are satisfied that that is that case and any consequentially
> necessary amendments have been make to the other drafts we will
> request adoption of draft-bryant-mpls-sfl-control and then WGLC on all
> three."
>
> There's been no follow-up indication that this analysis has occurred, or further revision to the draft. If it has, the authors should indicate as such on the list. If it hasn't, then this will serve as a reminder to the authors.

George and I discussed this and concluded that the design was satisfactory.

I certainly indicated this to the chairs but I cannot remember if I posted that to the list. If any reader of this note (which will go to the MPLS list) has any technical concerns WRT this point, please raise them and we will work with you to address the issue.

New version (05) has been uploaded.

Best regards

Stewart

> Thanks,
> Andy

---

From: Loa Andersson <[email-masked]>
Date: Fri, 3 Jan 2020 12:12:42 +0800

Haomian,

Thanks! The author will evaluate and address comment. I have small
comment on your comments inline.

On 03/01/2020 10:44, Zhenghaomian wrote:
> I am another MPLS-RT reviewer for this document, and would like to 
> follow up in this thread, by starting from reviewing the latest 
> draft-bryant-mpls-sfl-control-05. Generally I think this is a complete 
> work with self-contained solution, and I assume the importance of the 
> document has been evaluated by the WG chairs and experts. Some comments 
> are listed below as a ‘MPLS beginner’J
> 
> 1.Section 1 Introduction
> 
> 2. The RFC6374 looks slightly different than other RFC citations, as the 
> '[]' is missing; please check the formatting.
> 
> I am curious on the bits allocation "26-bit Session Identifier" and 
> "6-bit SFL Batch", which is not typical (my intuitive is 24+8), and 
> there is no reason in the explanation text below the TLV. We need to 
> confirm 1) the allocation is intended to be 26+6; 2) add something to 
> the TLV explanation, for example:
> 
> OLD:
> 
> Regarding the flags, my understanding is that the intention is to use 
> the first bit to indicate query/respond, and pad the other 3 bits as 0. 
> The processing of '0' bits is understood as "MUST be set to 0 by sender 
> and ignored by receiver". Is it correct? If yes, the following statement 
> may be easier:
> 
> OLD:
> 

The use of "first bit" and "next bits" could be confusing since it may
vary by context, e.g. order on the wire and descriptive text. I think
it would be better to use LSB and MSB.

BTW - I quite like the figure, if for no other reason that it give a
name for the R-bit.

To the authors: Query/Response indicator is fine in this context, but is
it possible that it might be confused with other Query/Response
indicators? Do we want to talk about "SFL Query/Response indicator" as
the general term?

/Loa
> Regarding the 'Control Code', do we allocate specific value for the 
> query/response actions? For example, we can have request (control code = 
> 0), refresh (control code = 1) and withdraw (control code =2) for a 
> query. Currently there is no such allocations.
> 
> *发件人:*mpls [mailto:[email-masked]] *代表 *Stewart Bryant
> *发送时间:*2020年1月3日0:28
> *收件人:*Andrew G. Malis <[email-masked]>
> *抄送:[email-masked]; mpls-chairs 
> <[email-masked]>; mpls <[email-masked]>
> *主题:*Re: [mpls] MPLS-RT review of draft-bryant-mpls-sfl-control
> 
>     Looking at the diff from the previous version, I just noticed a typo
>     that I missed earlier. In the first paragraph of the introduction,
>     "withdrawn" should be "withdraw". I also see that you added an
>     Oxford comma, which is fine by me! :-)
> 
>     Cheers,
> 
>     Andy
> 
>         On Thu, Jan 2, 2020 at 9:09 AM Stewart Bryant
>         <[email-masked] <mailto:[email-masked]>> wrote:
> 
> 
>             Hi Andy
> 
>             > On 30 Dec 2019, at 19:31, Andrew G. Malis <[email-masked] <mailto:[email-masked]>> wrote:
>             > 
>             > All,
>             > 
>             I generally work on the assumption that the purpose of the
>             Abstract is to help the potential reader decide if they want
>             to read the document (or more likely to help a search engine
>             match the document against a query) and I am unconvinced as
>             to value of the qualification text in deciding whether to
>             read the rest of the document.
> 
> 
>             > 
>             The earlier references are in text that I included as a
>             figure to force the layout style I wanted, and the markdown
>             compiler ignores references in figures. The compiling
>             process excluded unused references so they have to be
>             forced. I have changed the text to
> 
>             RFC Editor please remove this note which is used to force
>             the following references to appear {{RFC3032}} {{RFC5036}}
> 
>             > 
>             The framework is informational and so the reference would
>             become a down-ref. I will leave it to the chairs and ADs on
>             how they want to proceed with this. It is not unusual for a
>             framework to be required reading and yet informational.
> 
> 
>             I certainly indicated this to the chairs but I cannot
>             remember if I posted that to the list. If any reader of this
>             note (which will go to the MPLS list) has any technical
>             concerns WRT this point, please raise them and we will work
>             with you to address the issue.
> 
> _______________________________________________
> mpls mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/mpls
> 

-- 

Loa Andersson                        email: [email-masked]
Senior MPLS Expert
Bronze Dragon Consulting             phone: +46 739 81 21 64

---

From: Zhenghaomian <[email-masked]>
Date: Fri, 3 Jan 2020 06:11:15 +0000

Hi Loa,

Thank you for the good catch! 

I tried to integrate your comments into the TLV, as follow: 
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
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |Version|R|Flags|  Control Code |        Message Length         |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Then I think the authors can specify the R bit and the flags padding in descriptive text. 

Best wishes,
Haomian

-----邮件原件-----
发件人: Loa Andersson [mailto:[email-masked]] 
发送时间: 2020年1月3日 12:13
收件人: Zhenghaomian <[email-masked]>; Stewart Bryant <[email-masked]>; Andrew G. Malis <[email-masked]>
抄送: [email-masked]; mpls-chairs <[email-masked]>; mpls <[email-masked]>
主题: Re: [mpls] More comments from another Reviewer//Re: MPLS-RT review of draft-bryant-mpls-sfl-control

Haomian,

Thanks! The author will evaluate and address comment. I have small comment on your comments inline.

On 03/01/2020 10:44, Zhenghaomian wrote:
> document has been evaluated by the WG chairs and experts. Some 
> comments are listed below as a ‘MPLS beginner’J
> 
> 1.Section 1 Introduction
> 
> 2. The RFC6374 looks slightly different than other RFC citations, as 
> the '[]' is missing; please check the formatting.
> 
> The processing of '0' bits is understood as "MUST be set to 0 by 
> sender and ignored by receiver". Is it correct? If yes, the following 
> statement may be easier:
> 
> OLD:
> 

The use of "first bit" and "next bits" could be confusing since it may vary by context, e.g. order on the wire and descriptive text. I think it would be better to use LSB and MSB.

BTW - I quite like the figure, if for no other reason that it give a name for the R-bit.

To the authors: Query/Response indicator is fine in this context, but is it possible that it might be confused with other Query/Response indicators? Do we want to talk about "SFL Query/Response indicator" as the general term?

/Loa
> query/response actions? For example, we can have request (control code 
> = 0), refresh (control code = 1) and withdraw (control code =2) for a 

--

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 3 Jan 2020 16:58:07 +0000

> On 3 Jan 2020, at 02:44, Zhenghaomian <[email-masked]> wrote:
> 
> Hi, All,
>  
> I am another MPLS-RT reviewer for this document, and would like to follow up in this thread, by starting from reviewing the latest draft-bryant-mpls-sfl-control-05. Generally I think this is a complete work with self-contained solution, and I assume the importance of the document has been evaluated by the WG chairs and experts. Some comments are listed below as a ‘MPLS beginner’J
>  

Withdrawn correction done.

I have put the hyphens in, but I am not sure whether they are required. 

> 2. The RFC6374 looks slightly different than other RFC citations, as the '[]' is missing; please check the formatting.

Thanks for picking that I. I regenerated the .mkd from a .txt file and missed that reference. More importantly the RFC was not in the reference list at the end of the document.

It is now fixed in -06.

> I am curious on the bits allocation "26-bit Session Identifier" and "6-bit SFL Batch", which is not typical (my intuitive is 24+8), and there is no reason in the explanation text below the TLV. We…

The choice of the 26 bit size is inherited from RFC 6374 and needs to be the same.

I have added in the (6-bits) because it is unusual, but it is obvious from the figure and we do not specify the size of any of the other fields.

The use of 6 bits was a compromise. Strictly the protocol would work without it, but at the cost of extra messages and extra state. Allowing the use of a 6-bit batch identifier is convenient in that it reuses the DS field in the RFC6374 structure. If we had made it larger we would have needed to use an additional longword in the protocol.

A batch size of 64 seemed a good engineering compromise when we drew up the design.

Done

> response; the next 3 bits MUST be set to 0 by the Sender and ignored

Remember that this is inherited from RFC 6374 and I have tried to use a similar presentation of information.

I have tidied the text a little to mirror the RFC 6374 layout.

I really do not think it is necessary to say 4-bits.

It now looks like this - not sure it it is back to where it started:

                           +-+-+-+-+
                           |R|0|0|0|
                           +-+-+-+-+

                    SFL Control Message Flags.

The meanings of the flag bits are:

    R: Query/Response indicator.  Set to 0 for a Query and set
       to 1 for a Response.

Note that this then style matches the text that follows.

> Regarding the 'Control Code', do we allocate specific value for the query/response actions? For example, we can have request (control code = 0), refresh (control code = 1) and withdraw (control cod…

Thank you for noting that. That is the most important review comment. I will address it in a separate email.

> This problem confused me some times in later sections as well.

I have worked on this a bit and changed most of the “It” to Querier or Responder as appropriate.

> necessary to provide the necessary return address for this request.

I have clarified this.

Thank you for the review

Best wishes

Stewart

>  
> Thank you.
>  
> Best wishes,
> Haomian
>  
>  
> 发件人: mpls [mailto:[email-masked] <mailto:[email-masked]>] 代表 Stewart Bryant
> 发送时间: 2020年1月3日 0:28
> 收件人: Andrew G. Malis <[email-masked] <mailto:[email-masked]>>
> 抄送: [email-masked] <mailto:[email-masked]>; mpls-chairs <[email-masked] <mailto:[email-masked]>>; mpls <[email-masked] <mailto:[email-masked]>>
> On 2 Jan 2020, at 14:47, Andrew G. Malis <[email-masked] <mailto:[email-masked]>> wrote:
>  
> Stewart,
>  
> On Thu, Jan 2, 2020 at 9:40 AM Andrew G. Malis <[email-masked] <mailto:[email-masked]>> wrote:
> Stewart,
>  
> On Thu, Jan 2, 2020 at 9:09 AM Stewart Bryant <[email-masked] <mailto:[email-masked]>> wrote:
> 
> Hi Andy
>

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 3 Jan 2020 17:38:25 +0000

> indicators? Do we want to talk about "SFL Query/Response indicator" as

This has strong RFC6374 heritage so I was trying to maintain as much commonality as possible. You might deduce from some of the errors that the text started out using RFC6374 mkd as a template.

I propose to we leave it unchanged.

Best regards

Stewart

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 3 Jan 2020 18:29:07 +0000

> Regarding the 'Control Code', do we allocate specific value for the query/response actions? For example, we can have request (control code = 0), refresh (control code = 1) and withdraw (control cod…

Thanks for picking this up. It is quite significant since without these values you cannot implement the protocol.

I brushed over this about five years ago when I quickly pulled the first draft together and so far no one (including the authors) noticed. The implications is that no one has implemented the protocol. Obviously I think it will work, but you cannot be sure with a signalling protocol, and as the WG knows I have been a bit nervous on that point. I wonder if we should downgrade it from PS to Experimental?

The codes need to go into a registry and we have two choices, we create a new registry for it or we recognise the string ties to RFC6374 and add the values to its registry (presumably updating the RFC and renaming the registry).

It is tempting to put the values into RFC6374 because of the strong heritage but I made a mistake like that before with the ACH code points and it caused all sorts of problems, so I will clone the RFC 6374 IANA text and use it to create a new registry. However it makes me wonder if we should be trying to get some commonality in the ACH

In doing that I note that we originally proposed 0x60. I think we meant the next one after 0x59 which is 0x5A, so unless anyone objects I suggest we request 0x5A rather than leave the gap in the registry.

I will compile and upload the latest version, but need to take a much loser look at the error codes and I think we can do a better job than I have done so far.

This will need at least one more revision, but so long as it is on track the chairs could adopt it and allow it to proceed under WG control.

Best regards

Stewart

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 3 Jan 2020 19:29:57 +0000

Version 6 has been uploaded.

There are quite a lot of changes as per the commentary.

I will need another pass to work on the error conditions.

Please note my question on whether we should proceed as PS or EXP?

Best regards

Stewart