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

[Stig Venaas <[e-mail]>]

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.
>
> 128   Conversely, a DR that does not support the packed format can continue
> 129   generating the PIM Null-Register as defined in [RFC7761]
> 130   (Section 4.4).  To exchange the capability information in the
> 131   Register-Stop message, the "Reserved" field can be used to indicate
> 132   this capability in those Register-Stop messages.  One bit of the
> 133   Reserved field is used to indicate the "packing" capability (P bit).
> 134   The rest of the bits in the "Reserved" field will be retained for
> 135   future use.
>
> [major] 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.
>
> Suggestion>
>    This section allocates a bit in the PIM Register-Stop message
>    Flag Bits field for the RP to indicate its ability to receive
>    PIM Packed Null-Register messages (Section 3), and send PIM
>    Packed Register-Stop messages (Section 4).
>
>
> [major] The common PIM header was updated by rfc8736, so the discussion above about the Reserved field and the figure below are out of date.
>
> The new common header is this:
>
>     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
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |PIM Ver| Type  |   Flag Bits   |           Checksum            |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> ...so the discussion should be about the allocation of the Flag Bits.
>
>
>
> ...
> 149   PIM Version, Type, Checksum, Group Address, Source Address:
>
> 151      Same as [RFC7761] (Section 4.9.4)
>
> [major] Suggestion:
>
>    The fields in the PIM Register-Stop message are defined in Section
>    4.9.4 of [RFC7761], and the common header in [RFC8736].
>
>
> 153   P:
>
> 155      Capability bit (flag bit 7) used to indicate support for the
> 156      Packed Null-Register Capability
>
> [minor] Suggestion:
>
>    P: When set, it indicates the ability of the RP to receive
>       PIM Packed Null-Register messages, and send PIM Packed
>       Register-Stop messages.
>
>
> 158 3.  PIM Packed Null-Register message format
>
> 160   PIM Packed Null-Register message format includes a count to indicate
> 161   the number of Null-Register records in the message.
>
> [nit] s/count/Count field
>
>
> 163       0                   1                   2                   3
> 164       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
> 165      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 166      |PIM Ver| Type  |Subtype|  FB   |           Checksum            |
> 167      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 168      |   Count       |              Reserved                         |
> 169      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 170      |     Group Address[1]   (Encoded-Group format)                 |
> 171      |     Source Address[1]  (Encoded-Unicast format)               |
> 172      .                                                               .
> 173      .                                                               .
> 174      .                                                               .
> 175      .                                                               .
> 176      .     Group Address[N]                                          .
> 177      |     Source Address[N]                                         |
> 178      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> 180              Figure 2: PIM Packed Null-Register message format
>
> 182   PIM Version, Reserved, Checksum:
>
> 184      Same as [RFC7761] (Section 4.9.3)
>
> [major] Again, the updated common header definitions come from rfc8736.
>
>
> 186   Type, SubType:
>
> 188      The new packed Null-Register Type and SubType values TBD.
> 189      [RFC8736]
>
> [major] Type and SubType are not different values -- rfc8736.
>
>
> 191   Count:
>
> 193      The number of packed Null-Register records.  A record consists of
> 194      a Group Address and Source Address pair.
>
> [major] 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?
>
>
>
> ...
> 200 4.  PIM Packed Register-Stop message format
>
> [] The comments from the last section apply here too.
>
>
>
> ...
> 241 5.  Protocol operation
>
> 243   The following combinations exist -
>
> [] The different combinations don't need to be mentioned.  Instead, the specification for case 1 should be enough to cover all of them.
>
> 245   1.  DR and RP both support the PIM Packed Null-Register and PIM
> 246       Packed Register-Stop formats:
>
> 248       *  As specified in [RFC7761], the DR sends PIM Register messages
> 249          towards the RP when a new source is detected.
>
> 251       *  An RP supporting this specification MUST set the P-bit in the
> 252          corresponding Register-Stop messages.
>
> [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?
>
>
> [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.
>
> The text in §8 seems to indicate that the P-bit should always be set.  ??
>
>
> 254       *  When a Register-Stop message with the P-bit set is received,
> 255          the DR SHOULD send PIM Packed Null-Register messages
> 256          (Section 3) to the RP instead of multiple Register messages
> 257          with the N-bit set [RFC7761].
>
> [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?
>
> I assume that even if supported, the DR 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).
>
>
> [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?
>
>
> 259       *  The RP, after receiving a PIM Packed Null-Register message
> 260          SHOULD start sending PIM Packed Register-Stop messages
> 261          (Section 4) to the corresponding DR instead of individual
> 262          Register-Stop messages.
>
> [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).
>
>
> [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?
>
>
>
> ...
> 289 6.  Operational Considerations
>
> 291   In case the network manager disables the packed capability at the RP,
> 292   the router should not advertise the capability.  However, an
> 293   implementation MAY choose to still parse any packed registers if they
> 294   are received.  This may be particularly useful in the transitional
> 295   period after the network manager disables it.
>
> [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)
>
>
> 297 7.  PIM Anycast RP Considerations
>
> [] This seems to be an operational consideration.  It might be good to make this a sub-section of §6.
>
>
> 299   The PIM Packed Null-Register format should be enabled only if it is
> 300   supported by all PIM Anycast RP [RFC4610] members in the RP set for
> 301   the RP address.  This consideration applies to PIM Anycast RP with
> 302   MSDP [RFC3446] as well.
>
> [major] See the comments above about the (unqualified) requirement in §5 to set the P-bit.
>
>
> [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]).
>
>
> [major] 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?
>
>
> [major] "should be enabled"  Is Normative language needed?   What is the impact of a partial deployment?
>
>
> 304 8.  PIM RP router version downgrade
>
> [] This seems to be an operational consideration.  It might be good to make this a sub-section of §6.
>
>
> 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?
>
>
>
> ...
> 324 10.  Security Considerations
>
> [] Most of these considerations are the same as the ones already covered in rfc7761.  There's no need to repeat them here.  Also, additional requirements (for example, "standard methods to prevent spoofing") should not be suggested as they would better belong in the base specification.
>
> Suggestion>
>
>    The Security Considerations from [RFC7761] apply to this document.  In
>    particular, the effect of forging a PIM Packed Null-Register or
>    Register-Stop message would be amplified to all the records included and
>    not a single source/group pair.
>
>
>
> ...
> 345   There is another case where a spoofed Register-Stop can be sent to
> 346   make it appear that is from the RP, and that the RP supports this new
> 347   packed capability when it does not.  This can cause Null-Registers to
> 348   be sent to an RP that doesnt support this packed format.  But
> 349   standard methods to prevent spoofing should take care of this case.
> 350   For example, uRPF can be used to filter out packets coming from the
> 351   outside from addresses that belong to routers inside.
>
> [] This last paragraph points at a different effect of a forged packet: indicating support where it doesn't exist.
>
> Suggestion>
>
>    By forging a PIM Register-Stop message and setting the P-bit,
>    an attacker can trigger the use of PIM Packed Null-Register
>    messages by a DR thus creating unnecessary churn in the network.
>
> Is that the impact?
>
>
> 353 11.  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.
>
> [major] 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.
>
>
>
> ...
> 395 13.2.  Informative References
>
> 397   [RFC3446]  Kim, D., Meyer, D., Kilmer, H., and D. Farinacci, "Anycast
> 398              Rendevous Point (RP) mechanism using Protocol Independent
> 399              Multicast (PIM) and Multicast Source Discovery Protocol
> 400              (MSDP)", RFC 3446, DOI 10.17487/RFC3446, January 2003,
> 401              <https://www.rfc-editor.org/info/rfc3446>.
>
> [major] This reference should be Normative.
>
> [EoR -11]
>
> _______________________________________________
> pim mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pim