Re: [pim] AD Review draft-ietf-pim-null-register-packing-11

["Ananya Gopal (ananygop)" <[e-mail]>]

Hi Alvaro and Stig,

Thank you for the helpful and careful review. Please find my responses below. We would need another draft once I know your thoughts about [RC10] and specially, [RC18].

I have updated
draft-ietf-pim-null-register-packing-12 as per your suggestions.

[RC1] The "PIM Packed Null-Register message" is mentioned before it is described.
s/in a single PIM Packed Null-Register message/in a single PIM message
[AG]Changed as suggested.

[RC2] Please write in third person, not frst person.
s/We will refer to/This document refers to
[AG]Changed as suggested.

[RC3] Is it a "format" or a "message"?  I understand that the message is formatted differently (it is new!), but the use of "format" to (apparently) mean "message" is confusing.
[AG]Changed as suggested. The draft uses “message format” and “packet format” depending upon the context.

[RC4]
  The DR periodically sends PIM Null-Registers to keep the state of existing
  multicast sources active on the RP
[AG]Changed as suggested.

[RC5.1]COPP:  Is there a reference for that?  A vendor-independent reference would be ideal, but I don't think that this document needs to mention COPP to justify the enhancement.
[RC5.2] The section numbers are not needed.
[RC5.3] s/these packets anyway do not contain encapsulated data/these packets do not contain encapsulated data
[AG] I have removed the COPP justification and changed RC5.2 and RC5.3 as suggested.

[RC6] In this section, please only specify the new capability bit...the description of the operation should be in just one place.  That description is in §5.
Because the interoperability with routers that don't support this extension is mentioned in the Abstract and Introduction, please add a new section to explicitly talk about that (it can be just a few sentences long), perhaps as a sub-section of the Operational Considerations.
[AG] This section now only talks about the Capability bit. Also, operational considerations now also has a section 6.2 “Interoperability between different versions”

[RC7] The common PIM header was updated by rfc8736, so the discussion above about the Reserved field and the figure below are out of date.
[RC7.2] Suggestions about changing the field definitions
AG> All three figures now have the new header and field definitions per RFC8736. Also, the packet field definitions have also been changed.

[RC8]
155      Capability bit (flag bit 7) used to indicate support for the
156      Packed Null-Register Capability
[RC9] PIM Packed Null-Register message format: [RC9]  s/count/Count field
[RC9]the updated common header definitions come from rfc8736.
[RC9] Type and SubType are not different values -- rfc8736.
AG> Changed as suggested.

[RC10] The length of the message is not explicit in the PIM message, but it is inferred from the IP layer.  What should the receiver do if the count indicates a number of records that would result in a message with a length different from that is expected?
AG> I think that the count field is not really required in the packet format - we would parse and obtain each record after the header, until the length of the message (inferred from the IP layer). Its presence leads to unnecessary checks and processing. Also, we don’t need the reserved field as well, because we can use the Flag Bits for special purposes when the Type is PIM Packed Null-Register Type and/or PIM Packed Register-Stop.
       If it is not too late at this stage, and if we can proceed without these two fields, I can update the draft accordingly. I have the proposed draft ready as well.

[RC11]
200 4.  PIM Packed Register-Stop message format
 The comments from the last section apply here too.
AG> made necessary changes. Also, would like to propose the removal of the Count and Reserved fields from the PIM Packed Register-Stop message. for the reasons mentioned in RC10.

[RC12] Protocol Operation
[major]  Is receiving the  P-bit set in one Register-Stop message enough for the
DR to use the new messages for all future communication...OR...should the P-bit
first be set for specific (S,G)/(*,G) pairs before the DR can use the new
messages for them?  IOW, is the P-bit supposed to indicate per (S,G)/(*,G)
support or per RP?
AG> The P-bit indicates support per RP. The RP must set the P-bit when the configuration is enabled. The DR may choose to send packed or regular Null-Registers based on its configuration.

[major] If setting the P-bit, does it have to be set in all messages or can it be unset after the DR starts using the new messages?  The text above talks about "the corresponding Register-Stop messages", so it is not clear.
AG>  The RP must set the P-bit when the configuration is enabled. I have specified it in the draft as well in Protocol operation.
[major] When it is ok for the DR to not use PIM Packed Null-Register messages?  IOW, why is this action recommended and not required?
AG> The configuration may not be enabled on the DR, while it is ON on the RP. In that case, the DR will continue sending regular Null-Registers.
[major] Once the DR starts using the new messages, does it have to always use them?  Can a mixture of packed and unpacked messages be used?
AG>  We are allowed to use a mixture, but most implementations would probably find it easier to stick to the new format as long as the RP is capable. There is no harm in using a mixture.
Since we identify the PIM packet based on the new type, we should be good with the processing if the box has the capability to do so. And thus setting of the P-bit.
[major] When it is ok for the RP to not use PIM Packed Register-Stop messages?  IOW, why is this action recommended and not required?
I assume that even if supported, the RP may decide (by configuration??) that if doesn't want to use the new messages.  If this is the justification, then it seems to me that a better Normative keyword would be "MAY" (instead of SHOULD).
AG>
The configuration may not be enabled on the RP, while it is ON on the DR. In that case, after receiving DR's PIM Null-Register message, RP sends a normal Register-Stop without any capability information. DR then sends PIM Null-Registers in the unpacked format.
[major] Once the RP starts using the new messages, does it have to always use them?  Can a mixture of packed and unpacked messages be used?
AG>  We are allowed to use a mixture, but most implementations would probably find it easier to stick to the new format as long as the RP is capable. There is no harm in using a mixture.
Since we identify the PIM packet based on the new type, we should be good with the processing if the box has the capability to do so. And thus the P-bit.

[RC13]  Operational Considerations
[major] The specification in §5 says that "An RP supporting this specification MUST set the P-bit...".  That is an absolute requirement that doesn't account for configuration.
Perhaps you want add this exception to the specification above: When enabled/configured, the RP MUST set the P-bit...  (or something like that)
AG> Changed as suggested in Protocol Operation.


[RC14]   [major] s/by all PIM anycast RP members [RFC4610] in the RP set for the RP address./by all the routers in the Anycast-RP set ([rfc4610]).
AG> Changed as suggested.

[RC15] If the DR receives the P-bit from an RP, should it assume that the condition is met, or is there a way to verify?
AG> If the P-bit is set, then we can safely assume that the RP is capable since it is a reserved flag bit for a special purpose.

[RC16] "should be enabled"  Is Normative language needed?   What is the impact of a partial deployment?
AG>  Yes, normative language is needed. If routing changes, you might have been sending registers to a packed register capable router, and now you start sending to one that is not capable. In that case, the register stops would be ignored until we fall back to not using packed registers. I can add this as the next sentence to the draft(version 13) to explain why it is “SHOULD”.

[RC17] PIM RP router version downgrade: This seems to be an operational consideration.  It might be good to make this a sub-section of §6.
AG> Changed as suggested, moved to Operations Consideration (Section 6.3)

[RC18]
306   Consider a PIM RP router that supports PIM Packed Null-Registers and
307   PIM Packed Register-Stops.  When this router downgrades to a software
308   version which does not support PIM Packed Null-Registers and PIM
309   Packed Register-Stops, the DR that sends the PIM Packed Null-Register
310   message will not get a PIM Register-Stop message back from the RP.
311   In such scenarios the DR can send an unpacked PIM Null-Register and
312   check the PIM Register-Stop to see if the capability bit (P-bit) for
313   PIM Packed Null-Register is set or not.  If it is not set then the DR
314   will continue sending unpacked PIM Null-Register messages.
[major] "In such scenarios the DR can..."
How does the DR detect this situation?  Is there a timer (rfc7761 ??) that can alert the DR of the lack of support?
[AG] We think this needs to be discussed more. But some options are:

  1.  If more than N consecutive packet register messages have been sent without getting register stop?.. N can be 5 for example.
  2.  Or if we revert to data registers due to no null registers received, start a timer. If no register packed register stop or register stop with capability is seen within N seconds, assume RP does no longer support packed registers. and this timer could be configurable, with N’s default value as 60 seconds.
I can add a section for Configurable Parameters, and also add this to Operational Considerations in the next draft as a separate section (Section 6.3)


[RC18]  IANA Considerations
[major] This section should contain a specific request to IANA ("IANA is asked to assign..."), along with the specific names of the registries, and of the new bits/messages (!).
355      This document requires the assignment of Capability bit (P-bit),
356      flag bit 7 in the PIM Register-Stop message.
[major] A value can be suggested, not requested.
358      This document requires the assignment of 2 new PIM message types
359      for the PIM Packed Null-Register and PIM Packed Register-Stop.
[RC19] Please include a short table that can be used my IANA to fill out the registry information.  Note that both requests come from the same registry.
[RC20]This reference should be Normative.
AG> changed as suggested

[End of Response]

Please let me know your thoughts, and I will upload the new version asap upon receiving the next round of review.

Thanks,
Ananya

From: Stig Venaas <[e-mail]>
Date: Tuesday, January 24, 2023 at 2:56 PM
To: Alvaro Retana <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, Mike McBride <[e-mail]>
Subject: Re: [pim] AD Review draft-ietf-pim-null-register-packing-11
Hi Alvaro

I asked one of the author's recently and was told they would respond
soon. So hopefully soon now...

Regards,
Stig

On Thu, Jan 19, 2023 at 6:45 AM Alvaro Retana <[e-mail]> wrote:
>
> Ping!
>
> On December 20, 2022 at 5:55:07 PM, Alvaro Retana ([e-mail]) wrote:
>
>
> Dear authors:
>
> Thank you for the work on this document.  In general, it looks a lot better compared to when I read it last (-05) [1].  However, I am repeating some of the comments I made back then -- see inline.
>
> Thanks!
>
> Alvaro.
>
>
> [1] https://mailarchive.ietf.org/arch/msg/pim/NdNMqSwjwU9pvN0GGD2oXkpIuNI/
>
>
>
> [Line numbers from idnits.]
>
> ...
> 15 Abstract
> ...
> 25   This document defines a standard to send multiple Multicast source
> 26   and group information in a single PIM Packed Null-Register message.
> 27   We will refer to the new packed formats as the PIM Packed Null-
> 28   Register format and PIM Packed Register-Stop format throughout the
> 29   document.  This document also discusses interoperability between the
> 30   PIM routers which do not understand the PIM Packed Null-Register
> 31   format and routers which do understand it.
>
> [minor] The "PIM Packed Null-Register message" is mentioned before it is described.
>
> s/in a single PIM Packed Null-Register message/in a single PIM message
>
>
> [style nit] Please write in third person, not frst person.
>
> s/We will refer to/This document refers to
>
>
> [minor] Is it a "format" or a "message"?  I understand that the message is formatted differently (it is new!), but the use of "format" to (apparently) mean "message" is confusing.
>
> s/PIM Packed Null-Register format and PIM Packed Register-Stop format/PIM Packed Null-Register message and PIM Packed Register-Stop message
>
>
>
> ...
> 85 1.  Introduction
>
> 87   PIM Null-Registers are sent by the DR periodically for Multicast
> 88   streams to keep the states active on the RP, as long as the multicast
> 89   source is alive.  As the number of multicast sources increases, the
> 90   number of PIM Null-Register messages that are sent also increases.
> 91   This results in more PIM packet processing at the RP and the DR.
>
> [] Suggestion:
>
> s/
>   PIM Null-Registers are sent by the DR periodically for Multicast streams to
>   keep the states active on the RP, as long as the multicast source is alive.
> /
>   The DR periodically sends PIM Null-Registers to keep the state of existing
>   multicast sources active on the RP.
>
>
>
> 93   The control plane policing (COPP), monitors the packets that are
> 94   processed by the control plane.  The high rate at which Null-
> 95   Registers are received at the RP can lead to COPP drops of Multicast
> 96   PIM Null-Register messages.  This draft proposes a method to
> 97   efficiently pack multiple PIM Null-Registers [RFC7761] (Section 4.4)
> 98   and Register-Stops [RFC7761](Section 3.2) into a single message as
> 99   these packets anyway do not contain encapsulated data.
>
> [minor] COPP:  Is there a reference for that?  A vendor-independent reference would be ideal, but I don't think that this document needs to mention COPP to justify the enhancement.
>
>
> [nit] The section numbers are not needed.
>
>
> [nit] s/these packets anyway do not contain encapsulated data/these packets do not contain encapsulated data
>
>
>
> ...
> 118 2.  Packed Null-Register Capability
>
> 120   A router (DR) can decide to pack multiple Null-Register messages
> 121   based on the capability received from the RP as part of the PIM
> 122   Register-Stop.  This ensures compatibility with routers that do not
> 123   support processing of the new format.  The capability information can
> 124   be indicated by the RP via the PIM Register-Stop message sent to the
> 125   DR.  Thus a DR will switch to the new format only when it learns that
> 126   the RP is capable of handling the PIM Packed Null-Register messages