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

["Adrian Farrel" <[e-mail]>]

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:[e-mail]]
> Sent: 10 April 2012 19:32
> To: [e-mail]
> Cc: [e-mail]; [e-mail]; Yiqun Cai
> Subject: Re: [pim] AD review of draft-ietf-pim-pop-count
> 
> 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.
> 
> On 4/9/2012 10:58 AM, Stig Venaas wrote:
> > On 4/3/2012 1:38 PM, Adrian Farrel wrote:
> >> 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.
> >
> > Thanks for the detailed review. Your comments look good. I will start
> > working on an update.
> >
> > Stig
> >
> >>
> >> 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 [e-mail] and his
> >> coordinates to Microsoft.
> 
> Done
> 
> >> ---
> >>
> >> The RFC Editor likes the document to begin with an Introduction, so
> >> you could move Section 1 into 2.1 and renumber.
> 
> Done
> 
> >> ---
> >>
> >> 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.
> 
> 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.
> 
> >> ---
> >>
> >> Can you s/this draft/this document/ throughout since, on publication,
> >> this will not be a draft any more.
> 
> Done
> 
> >> ---
> >>
> >> 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 pretty much used your text here, and acknowledged you.
> 
> >> ---
> >>
> >> I think you need to expand "oif-list" on first use.
> 
> Done
> 
> >> ---
> >>
> >> 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.
> 
> Yes. I'm also calling it option instead of TLV as that is the
> terminology usually used in the docs.
> 
> >> ---
> >>
> >> 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 :-)
> 
> 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.
> 
> >> 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.
> 
> 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.
> 
> >> END
> >>
> >>
> >> ---
> >>
> >> Section 4
> >>
> >> Suggest
> >>
> >> s/Pop-Count attribute/Pop-Count Join Attribute/
> >> s/to process PIM Join Attribute/to process a PIM Join Attribute/
> 
> Done
> 
> >> ---
> >>
> >> Section 4
> >>
> >> Again, to leave this to IANA...
> >>
> >> OLD
> >> Attr Type: 2.
> >> NEW
> >> Attr Type: TBD2
> >> END
> 
> Done
> 
> >> 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.
> 
> Done
> 
> >> ---
> >>
> >> 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.
> 
> You're absolutely right. Done.
> 
> >> ---
> >>
> >> 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?
> 
> Tried to make this clearer.
> >> ---
> >>
> >> Section 4.2 has some ugly XML bleedthrough
> >>
> >> <figure>
> >> <preamble></preamble>
> >> <artwork><![CDATA[
> 
> Done
> 
> >> ---
> >>
> >> Section 5 has some lower case 2119-type words. Can you check to see
> >> whether you mean upper case.
> 
> Done. Some changes.
> 
> >> ---
> >>
> >> Section 6 is good. It might be nice to note at the top that the
> >> section is
> >> non-normative.
> 
> Done
> 
> >> 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.
> 
> Done
> 
> >> ---
> >>
> >> 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.
> 
> I rewrote the security considerations, trying to discuss this.
> 
> Stig
> 
> >> _______________________________________________
> >> pim mailing list
> >> [e-mail]
> >> https://www.ietf.org/mailman/listinfo/pim
> >
> > _______________________________________________
> > pim mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/pim