Subject: [pim] AD review of draft-ietf-pim-pop-count

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 3 Apr 2012 21:38:00 +0100

Hi,

Sorry I sat on this I-D a while doing my review. The review is my
usual pass over the document to iron out nits that I think might 
show up in IETF last call, Directorate reviews, or during IESG 
evaluation. By catching the issues at this stage we can save the
effort of multiple reviewers, and achieve a smoother passage 
through the publication process.

All of my comments can be discussed.

As it stands, I think a new revision would help address these point
so I will move the I-D into "New Revision Required" state, and when
I see an update I will advance the document to IETF last call. Please
be sure that the new revision passes idnits.

Thanks,
Adrian

---

Please update Yiqun's email address to [email-masked] and his
coordinates to Microsoft.

---

The RFC Editor likes the document to begin with an Introduction, so
you could move Section 1 into 2.1 and renumber.

---

I think that the RFC Editor likes all figures to be captioned and given
a number (e.g. Figure 1 : A Sample Foobar). Further, they like all
figures to be explicitly referenced form the text.

That sounds a bore to do, but if you have the energy it might be worth
it.

---

Can you s/this draft/this document/ throughout since, on publication,
this will not be a draft any more.

---

We will get push-back from the IESG to "describe the experiment".

I think this can be handled relatively easily in the Introduction with
some text such as:

   This is a new proposal for an extension to PIM, and it is not
   completely understood what impact collecting information using PIM
   would have on the operation of PIM. Many PIM features (including the
   core protocol) were first introduced as Experimental RFCs, and it
   seems appropriate to advance this work as Experimental. Reports of
   implementation and deployment across whole distribution trees or
   within sub-trees (see Section 7) will enable an assessment of the
   desirability and stability of this feature. The PIM working group
   will then consider whether to move this work to the Standards Track.

I am only showing example text here. I am not attached to the content,
but I am trying to cover the bases in a way that the IESG is likely to
request. Please feel free to use other text if it works better for you.

---

I think you need to expand "oif-list" on first use.

---

Section 3

   If a PIM router supports this draft, it must send the Pop-
   Count-Supported TLV.

I think this might be better written as:

   A PIM router indicates that it supports this document by sending the
   Pop-Count-Supported TLV in a PIM Hello message.

---

Section 3

   The format of the TLV is defined to be:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |          OptionType           |         OptionLength          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                          OptionValue                          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   OptionType = 29, OptionLength = 4, there is no OptionValue semantics
   defined at this time but will be included for expandability and be
   defined in future revisions of this draft.  The format will look
   like:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |             29                |              4                |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                      Unallocated Flags                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   Unallocated Flags:   for now should be sent as 0 and ignored on
      receipt.

I am ambivalent about defining the flags field. Does PIM require that
an options TLV is always the same length? If so, you are doing the
right thing, but if not, you could equally come back later and define
that, if the length is non-zero, the first four bytes are flags. But
this is Experimental, so I don't object. Mainly I am curious :-)

But anyway, I don't think you need the TLV format twice, and you need
to allow IANA to make the allocation (although you can suggest a
value in the IANA section). So you could rewrite as:

   The format follows the format of all Hello Option TLVs [RFC4601]:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |     OptionType = TBD1         |      OptionLength = 4         |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                      Unallocated Flags                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   Unallocated Flags:   Unused flags SHOULD be sent as 0 and ignored on
                        receipt. This document defines no flags.

Then, in Section 8...

OLD
   A new PIM Hello Option type, 29, has been assigned.  See [HELLO] for
   details.
NEW
   IANA is requested to define a new PIM Hello Option type in the
   registry at [HELLO].

      Pop-Count Support   TBD1

   A value of 29 is suggested.
END

---

Section 4

Suggest

s/Pop-Count attribute/Pop-Count Join Attribute/
s/to process PIM Join Attribute/to process a PIM Join Attribute/

---

Section 4

Again, to leave this to IANA...

OLD
   Attr Type:   2.
NEW                                                                  
   Attr Type:   TBD2
END

And then, in Section 8

OLD
   A new PIM Join Attribute type needs to be assigned. 2 is proposed in
   this draft.
NEW
   IANA is requested to allocate a new PIM Join Attribute type.

      Pop-Count Join Attribute   TBD2

   A value of 2 is suggested.

---

Section 4                                       

      Unallocated Flags:   The flags which are currently not defined.
         If a new flag is defined and sent by a new implementation, an
         old implementation should preserve the bit settings.  This
         means that if a bit was set in a PIM Join message from any of
         the downstream routers, then it MUST also be set in any PIM
         Join sent upstream.

Am I being too pedantic? Presumably clear bits MUST also be kept clear.
How about:
This means that a router MUST preserve the settings of all Unallocated
Flags in PIM Join messages received from downstream routers in any PIM
Join sent upstream.

---

Section 4

      P flag:   This flag remains set if all downstream routers support
         this specification.  That is, they are PIM pop-count capable.
         This allows one to tell if the entire sub-tree is completely
         accounting capable.

I'm not clear about this. Is the assumption that a router will clear 
this flag if it knows that a downstream router is not PIM pop-count
capable? (I presume that the downstream router would not clear the
flag because it doesn't know about this I-D.) Can you clarify by saying
who has the responsibility to clear the flag?

---

Section 4.2 has some ugly XML bleedthrough

               <figure>
               <preamble></preamble>
               <artwork><![CDATA[

---

Section 5 has some lower case 2119-type words. Can you check to see
whether you mean upper case.

---

Section 6 is good. It might be nice to note at the top that the section is
non-normative.

However, the final paragraph (with its recommendation and use of "should
not") seems to be important enough to promote to Section 5 and to use
2119 language.

---

Section 9

I wonder...

Does collecting this information put any additional burden on the
routers and the network? If so, might it be something that a router 
should rate limit to prevent DoS?

Does access to the information provide assistance to an attacker or 
even just violate an operator's confidentiality? If so, there should
be a warning against sniffing attackers if the information is considered
sensitive (I presume encrypting it is not possible).

Now, you may say "so what?" but I think we should make a habit of 
raising flags so that operators are aware and thoughtful.

---

From: Heidi Ou <[email-masked]>
Date: Wed, 04 Apr 2012 11:55:16 -0700

Adrian,

Thanks for your review. We will address your comments in a new version.

- Heidi

On 4/3/12 1:38 PM, "Adrian Farrel" <[email-masked]> wrote:

> _______________________________________________
> pim mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pim

---

From: Stig Venaas <[email-masked]>
Date: Mon, 09 Apr 2012 10:58:42 -0700

> All of my comments can be discussed.

Thanks for the detailed review. Your comments look good. I will start
working on an update.

Stig

---

From: Greg Shepherd <[email-masked]>
Date: Mon, 9 Apr 2012 11:08:17 -0700

Thanks Stig!

-G

---

From: Stig Venaas <[email-masked]>
Date: Tue, 10 Apr 2012 11:32:11 -0700

Hi

I've been editing the document and hopefully resolved most of your
concerns. Please see my comments below.

Unless you see any obvious issues based on my comments, I can
go ahead and submit a new version.

Note that these changes are fairly big, I made a few additional ones,
so I feel we also need to give the WG some time to review them. And
of course also for you and the other authors to review them.

Thanks a lot, I think this improved the document significantly. I
also found some issues myself that I tried to take care of.

>> coordinates to Microsoft.

Done

>> you could move Section 1 into 2.1 and renumber.

Done

>> That sounds a bore to do, but if you have the energy it might be worth

Sorry, I didn't have the energy. I don't really think it is needed. And
it can be done later if it really seems to be needed.

>> this will not be a draft any more.

Done

>> request. Please feel free to use other text if it works better for you.

I pretty much used your text here, and acknowledged you.

>> I think you need to expand "oif-list" on first use.

Done

>> Pop-Count-Supported TLV in a PIM Hello message.

Yes. I'm also calling it option instead of TLV as that is the
terminology usually used in the docs.

>> The format of the TLV is defined to be:

I've added some text where I define it without the options, but also say
that implementations MUST accept an Hello with non-zero length and
ignore the contents. This way we should be able to add flags later if
needed.

>> The format follows the format of all Hello Option TLVs [RFC4601]:

I did something like this. But note that 29 has already been
temporarily assigned for this purpose. I'm noting that in the
new text.

>> s/to process PIM Join Attribute/to process a PIM Join Attribute/

Done

>> Again, to leave this to IANA...

Done

>> A value of 2 is suggested.

Done

>> Flags in PIM Join messages received from downstream routers in any PIM

You're absolutely right. Done.

>> who has the responsibility to clear the flag?

Tried to make this clearer.
>> Section 4.2 has some ugly XML bleedthrough

Done

>> whether you mean upper case.

Done. Some changes.

>> Section 6 is good. It might be nice to note at the top that the

Done

>> not") seems to be important enough to promote to Section 5 and to use

Done

>> raising flags so that operators are aware and thoughtful.

I rewrote the security considerations, trying to discuss this.

Stig

---

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 10 Apr 2012 22:24:49 +0100

Excellent job, Stig, thanks.

I'd be OK with this, but you are right that the WG should have a quick look to
make sure they are happy.

Probably best to post it and let Mike run it from there.

Cheers,
Adrian

> -----Original Message-----
> From: Stig Venaas [mailto:[email-masked]]
> Sent: 10 April 2012 19:32
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; Yiqun Cai
> Subject: Re: [pim] AD review of draft-ietf-pim-pop-count
> 
> Hi
>

---

From: Stig Venaas <[email-masked]>
Date: Tue, 10 Apr 2012 15:13:51 -0700

> Probably best to post it and let Mike run it from there.

Based on Adrian's comments I've made several changes to the pop-count
draft. I also found some other issues I've tried to resolve.

Apart from changing wording, whether to use 2119 keywords etc., the only
real protocol change IMO, is that the hello option no longer contains
an option value with reserved bits. Instead I've added language that
if a value is present, the hello option must be processes as if there
is no value present.

The new version is at
http://www.ietf.org/internet-drafts/draft-ietf-pim-pop-count-06.txt

and a diff is available at
http://tools.ietf.org/rfcdiff?url2=draft-ietf-pim-pop-count-06

Adrian, please check if this resolves your issues.

I hope other authors and people in the WG can help review this to make
sure the changes are fine.

I leave to Mike to ensure that the document is appropriately reviewed
by the WG.

Stig

---

From: Stig Venaas <[email-masked]>
Date: Tue, 17 Apr 2012 10:43:07 -0700

>> Probably best to post it and let Mike run it from there.

Adrian, Mike and rest of WG, hope you can have a look and let everyone
know if you have any concerns.

Stig

---

From: Mike McBride <[email-masked]>
Date: Mon, 23 Apr 2012 11:03:50 -0700

Stig,

Thanks for the work. I have no concerns with your changes. Since there
have been no comments in the last two weeks I feel we can proceed.
Someone on this list will yell if they disagree.

mike

---

From: Greg Shepherd <[email-masked]>
Date: Mon, 23 Apr 2012 11:29:10 -0700

Yes, thank you very much, Stig!

Greg

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 25 Apr 2012 22:50:40 +0100

Thanks,

I'll take this forward.

Adrian

> From: Mike McBride [mailto:[email-masked]]
> Sent: 23 April 2012 19:04
> To: Stig Venaas
> Cc: [email-masked]; [email-masked]; [email-masked]