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

[Greg Shepherd <[e-mail]>]

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