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

[Nabeel Cocker <[e-mail]>]

Shepherds Review:

Authors, I started on the review of the document and have made my comments
in the attached google document:

https://docs.google.com/document/d/1exSjh3ZFRl1LfWSmfv8BI_XzRewmsBPksOUKio4Rkvk/edit?usp=sharing

My comments are within that doc.  The document is in pretty good shape, it
has addressed all the comments I have seen from the mailing list.

The main feedback is really on a few editorial/grammar/spellings...

If you prefer, I can download the google doc and send that as an attachment
as well.

thanks
Nabeel

On Tue, Sep 15, 2020 at 2:57 PM Mankamana Mishra (mankamis) <mankamis=
[e-mail]> wrote:

> Yes, I think its good to move document to next step
>
>
>
> *From: *Greg Shepherd <[e-mail]>
> *Date: *Tuesday, September 15, 2020 at 10:39 AM
> *To: *"Bidgoli, Hooman (Nokia - CA/Ottawa)" <[e-mail]>
> *Cc: *Stig Venaas <[e-mail]>, "Mankamana Mishra (mankamis)" <
> [e-mail]>, BIER WG <[e-mail]>, Ijsbrand Wijnands <
> [e-mail]>, "[e-mail]" <[e-mail]>, "Jeffrey
> (Zhaohui) Zhang" <[e-mail]>, Antoni Przygienda <[e-mail]>
> *Subject: *Re: [Bier] WG LC on
> https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/
>
>
>
> Thanks! If all the authors agree then let’s release the Doc Shepherd for
> the write-up.
>
>
>
> - Shep
>
> Sent from my iPhone
>
>
>
> On Sep 15, 2020, at 08:52, Bidgoli, Hooman (Nokia - CA/Ottawa) <
> [e-mail]> wrote:
>
> Hi
>
>
>
> Yes the agreed changes are there, again these were mostly cleanup rather
> then any significant procedural changes.
>
>
>
> I think they were included in the previous last call.
>
>
>
> Thanks
>
> Hooman
>
>
>
> *From:* Greg Shepherd <[e-mail]>
> *Sent:* Monday, September 14, 2020 4:30 PM
> *To:* Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]>
> *Cc:* Stig Venaas <[e-mail]>; Mankamana Mishra (mankamis) <
> [e-mail]>; BIER WG <[e-mail]>; Ijsbrand Wijnands <
> [e-mail]>; [e-mail]; Jeffrey (Zhaohui) Zhang <
> [e-mail]>; Antoni Przygienda <[e-mail]>
> *Subject:* Re: [Bier] WG LC on
> https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/
>
>
>
> It appears that issues have been addressed as per the list discussion of
> July 28th. The latest rev was posted July 29th, I assume with the agreed
> changes, correct?
>
>
>
> Let's do a quick 1 week WGLC to confirm the latest rev.
>
>
>
> Please respond to this thread by Sept 27th.
>
>
>
> Thanks,
>
> Shep
>
> (chairs)
>
>
>
> On Tue, Jul 28, 2020 at 2:35 PM Bidgoli, Hooman (Nokia - CA/Ottawa) <
> [e-mail]> wrote:
>
> Thanks Stig!
>
> Inline HB>
>
> Regards
>
> Hooman
>
> -----Original Message-----
> From: Stig Venaas <[e-mail]>
> Sent: Tuesday, July 28, 2020 4:56 PM
> To: Mankamana Mishra (mankamis) <[e-mail]>
> Cc: [e-mail]; Bidgoli, Hooman (Nokia - CA/Ottawa) <
> [e-mail]>; BIER WG <[e-mail]>; Ijsbrand Wijnands <
> [e-mail]>; [e-mail]; Jeffrey (Zhaohui) Zhang <
> [e-mail]>; Antoni Przygienda <[e-mail]>
> Subject: Re: [Bier] WG LC on
> https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/
>
> 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!
>
> HB> Done, Add it for the join attribute TLV that you got away from in the
> other draft ;)
>
> 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.,
>
> HB>
> Changed to PIM join/prune in section 3 because this was the PIM domain
> join/prune. "it will generate a PIM join/prune packet toward its attached
> PIM domain."
> Any where else that the pim signaling is left is actually the BIER
> signaling the join/prune through the bier network as such I wanted it to be
> clear that these are signalling packet and not extending the PIM over the
> BIER domain.
>
> :
>
>
> 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
>
> _______________________________________________
> BIER mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/bier
>