Last Call Review of draft-ietf-mpls-mldp-in-band-wildcard-encoding-02
review-ietf-mpls-mldp-in-band-wildcard-encoding-02-genart-lc-davies-2014-10-31-2-00

Hi, Ijsbrand/Ice,



Thanks for the response.It seems there isn't much that can be done about 


the error handling.  I shall escalate this problem as a generic check 


that we need to make sure that unused extension systems defined in base 


protocols have adequate error handling/reporting for when the 


extensibility gets used.






Are you intending to generate a new version before the IESG review?  I 


see this is now scheduled for 30 November, and it would be useful to 


know whether there will be a new version to check out for the IESG 


review report.




Cheers,
Elwyn

On 04/11/14 22:03, IJsbrand Wijnands wrote:



Hi Elwyn,




Summary: Almost ready.  There are a couple of clarifications around
how IPv4 and IPv6 trees can or cannot be merged on a single MP-LSP
that would be advantageous.  Also the error handling in the parent
RFCs (6388, 6826) is a bit sketchy resulting in messy handling if
an LSR that does not support the wildcard encoding is accidentally
sent the TLVs from this draft.  I am not sure if this can be
cleaned up (and maybe there is no thought to be a problem - I am
not sufficiently expert in LDP signalling).




I don’t think that is a problem, the wildcard encoding i.e. ‘all
zero’s’, is an invalid value in implementation that don’t support it.
So these routers would just drop the label mapping when this
happens.




Minor issues: IPv4 and IPv6 Multicast: I think it would be useful
to add a short discussion of the fact that there are both IPv4 and
IPv6 multicast trees.  Presumably an MP-LSP can only carry one or
the other at a time, but I am unclear about whether this is the
case. Also a note in s3.1 that it is either the IPv4 or IPv6
address fields that are relevant and they are all zeroes in either
case would clarify the usage further.




We are not changing anything about how an LSP is used, either for
IPv4 or IPv6. We’re only allowing *all* sources within a IPv4 or IPv6
group to be forwarded down the LSP. I don’t think we should add
anything here since we are not changing the default behaviour.






I understand that.  To a novice reading the document it would be useful 


to add a sentence after the first three paras of s3.2, something like:



   The wildcard scheme allows multicast traffic for multiple sources or
   multiple trees for either IPv4 or IPv6 traffic, but not both, to
   share a single MP-LSP according to the type of TLV used.







RFC 5918 Typed Wildcard:  Is there anything special that has to be
done if the Typed Wildcard is used?




No, a typed wildcard does not encode any mLDP Source or Group address
fields, so this draft does not apply.




OK





s3.3:  Is it possible to specify the error behaviour more
concretely (i.e., what might  happen) in case an unadapted Ingress
LSR gets a wildcard TLV?  It appears that RFC 6826 is rather
underspecified as regards error handling - but I am not an expert
on this area - it appears that the MP-LSP would get set up but to
no good purpose which seems inappropriate.  Could an error status
not be returned?




For the Opaque in-band TLV's that mLDP FEC’s carry there is no error
signalling defined in the base RFC. If you receive an opaque type for
an in-band TLV and you don’t support it, you’ll just drop the label
mapping. I agree this would have been useful to add a LDP capability
in the base RPF 6826. Since this draft is just a minor update to the
functionality as defined in 6826, it does make sense to do it now.







Pity.  Do I understand correctly that the egress LSR will be none the 


wiser that its request has been effectively silently ignored?  There 


doesn't seem to be much that can be done at this stage.






*** NOT SPECIFIC TO THIS DRAFT: There is a generic point here that when 


extensible capabilities are put in place in protocols but not actually 


used in the base protocol, we need to be sure that adequate error 


signalling is put in place so that a later (mis)use of the extensible 


capabilities can be detected and reported appropriately.  This is the 


second instance I have seen in the recent past where there was 


inadequate error reporting capability to  handle an extension scheme.





Nits/editorial comments: s2, Definition of 'in-band signalling':
Should also allow for (S,*) - and possibly (*,*), I believe.




Yes, I’ll change this.





s3.4: s/PIM ASM/PIM-ASM/




Ok,

Thanks for your review,

Ice.