Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

[Stig Venaas <[e-mail]>]

Hi

It looks like most of the issues I found have been resolved now. But
most importantly, the IANA Considerations still say that there are
none. This needs to be fixed!

I see a couple of places where it says PIM signaling message or PIM
signaling packet. I think it should say PIM join/prune message, or PIM
join/prune packet.,

Regards,
Stig

On Tue, Sep 24, 2019 at 10:51 AM Stig Venaas <[e-mail]> wrote:
>
> Hi
>
> This is good work and I generally support the document. There are
> however some issues that need to be addressed. I also think it
> would be good to get this reviewed in the pim WG, perhaps you
> can ask for reviews on the pim list.
>
> I mostly have editorial comments that are easy to address. But at
> a high level I think some more text about pim is needed.
>
> PIM relies on hello messages to know what capabilities a router
> has, e.g. whether it support Join attributes, whether it support
> BIDIR etc. I think you need to point out what capabilities are
> assumed to be present. Obviously it is assumed that Join
> attributes are supported. It may also be good to point out that
> there is no J/P suppression as the J/P is only sent to the target,
> and not to other routers. Also point out that only J/P messages
> are used, and that there is no assert processing.
>
>
> Below are the editorial comments. Please also check the "nits"
> tool. It complains about missing references, and it cannot find
> the "Authors' Addresses" section. The heading is missing.
>
> Also Section 7 must be updated. There is an IANA action, but
> it says there are none!
>
> The abstract is rather long. Suggest removing some of the text that
> explains the general operation of BIER.
>
> Some of the references might need some work. In section 2 it
> says "RFC 2119 [RFC2119]". Should it be just "[RFC2119]"? Also
> the reference is missing.  In 2.1 it says "[I-D. rfc8279]" which
> also needs to be fixed. Most of the other references look fine.
> In 3.1 there seems to be a reference to 8279 without brackets though.
>
> In 2.1, BFR definition. Missing space after ".".
>
> BFIR definition: It says "insert the BM into the packet", but I think it
> might be better to say that it performs BIER encapsulation. The term BM
> is not defined. It says "plain" instead of "plane".
>
> BFER defintion: Do we have to say that BFER is a BFR? In that case, we
> should also say that BFIR is a BFR. It says "plain" instead of "plane".
>
> IBBR and EBBR, might it be good to include "signaling" in the name, like
> Ingress Signaling BIER Router and Egress Signaling BIER Router? I want it
> to be very clear that ingress and egress are not related to data.
>
> In "Figure 1" I think "bfir" and "bfer" should be upper case.
>
> In 3.1:
> "weather" should be "whether".
> "located on" should probably be "located in".
>
> In 3.1 there is this paragraph that I think need some changes.
>
>    After discovering the EBBR and its BFR-ID (flooded via IGP BIER
>    extension), the IBBR will construct a PIM Join Attribute encoded as
>
> It says that BFR-ID is flooded via IGP BIER extension. That isn't
> necessarily true, do we need to explain how the BFR-ID is discovered?
>
>    TLVs into the Source Address field of the PIM Join Message as per
>
> Isn't it just one TLV? I think you can skip saying that it is a TLV
> in the source address field. Just say that it includes a new PIM
> Join Attribute in the Join/Prune message. Also note that we should
> really say Join/Prune, not just Join.
>
>    [RFC5384] and include it in PIM signaling message. Two new "BIER
>
> I think it is better to be clear and say PIM Join/Prune message.
>
>    IBBR" attributes are define and explained in upcoming section. The
> s/define/defined
>
>    PIM Join Attribute is used on EBBR to obtain necessary bier
> s/bier/BIER
>    information to build its multicast states. In addition the IBBR will
>    change the PIM signaling packet source IP address to its BIER prefix
>    address (standard PIM procedure). It will also keep the destination
>    address as the well known multicast IP address. It then will
>    construct the BIER header. The signaling packet, in this case the PIM
>    join/prune packet, is encapsulated in the BIER header and transported
>    through BIER domain to EBBR.
>
> The language at the end here makes it sound like the PIM J/P is being
> forwarded and that the J/P is being modified. But J/P messages are sent
> hop-by-hop. Each router originates a new J/P. For instance, a router
> may receive a J/P for multiple sources. These sources may have different
> RPF neighbors on the receiving router and hence be split into separate
> join messages. This could happen on the IBBR as well.
>
> The lasts paragraph in section 3 says:
>    The IBBR will track all the PIM interfaces on the attached PIM domain
>    which are interested in a certain (S,G). It creates multicast states
>    for arriving (S,G)s from PIM domain, with incoming interface as BIER
>    "tunnel" interface and outgoing interface as the PIM domain
>    interface(s) on which PIM Join(s) were received on.
>
> The IBBR is a PIM router and what you are describing is standard PIM
> behavior, so I think this is a bit redundant. It might be goot to
> stress that OIFs are adding according to standard PIM behavior. The only
> special here is the RPF interface.
>
> Section 3.1.4:
> In heading Pim/PIM
> bier/BIER
>
> It says "PIM Join Attribute [RFC5384] is used." I think it might be good
> to say that "a new PIM Join Attribute is used". And then also this
> sentence "The PIM Join Attribute format is as follow:" should perhaps be
> The new PIM Join Attribute format is defined as follows:".
>
> s/Ipv4/IPv4
> s/Ipv6/IPv6
>
> In the format I think it might be good to not just show "BIER info", but
> show the formatting of prefix, SD and BFR-ID explicitly. Also, it should
> state clearly that these are the prefix, SD and BFR-id of the IBBR.
>
> In 3.1.4.1 why not say PIM Join/Prune packet instead of PIM signaling?
>
> In 3.3.:
>    After receiving the BIER packet and determining this packet is a
>    signaling packet, EBBR will remove the BIER header from PIM packet.
>    The Received PIM join/prune Signaling packet is processed as if it
>    were received from neighbors on a virtual interface, (i.e. as if the
>    pim adjacency was presents, regardless of the fact there is no
>    adjacency)
> Wouldn't the router remove the BIER header simply because the BFR-id
> in the BIER header matches its own BFR-id? There are some grammar issues
> and a missing period in this paragraph.
>
> In this paragraph:
>    With same token the EBBR creates a multicast state with incoming
>    interface as same interface that PIM join packet was forwarded and
>    outgoing interfaces of BIER tunnel to BFER identified with BFIR-id
>    and its corresponding Sub-Domain obtained from the BIER header or via
>    PIM Join Attributes added to the PIM signaling packet by the IBBR.
>
> I'm not sure what "With same token" means here. Also, it not should say
> that PIM join packet was forwarded. I would say that state is created
> according to the PIM specification, and just describe the BIER OIF state.
> The specific OIF state may be implementation specific though. Perhaps this
> is sufficiently described in the paragraph that comes right after? It is
> also well described in 4.1.
>
> In section 5: s/LEAFs/leaves? Should it be EBBRs?
>
> In section 6:
> s/vrf/VRF
> "it is determine" should be "it is determined".
> Replace "PIM signaling message" with "PIM Join/Prune message"?
>
> Section 8:
> The 4601 reference should be to RFC 7761.
>
> Appendix A:
> The header seems to not be formatted correctly.
> "This section" should be "This appendix"?
>
> In A.1: "is consist" should be "consists".
>
> In Figure 2: s/bier/BIER
> Also in the text in A.2.2.
>
> IN A.3.1
> s/Bier/BIER
> s/bier/BIER
> s/tlv/TLV
>
> Stig