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

[Stig Venaas <[e-mail]>]

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