[Idr] Review of draft-ietf-idr-error-handling-18

["Alvaro Retana (aretana)" <[e-mail]>]

Alia:

Hi!

As part of my AD training, I have done a review of this draft.  I have some comments, none of which should delay the publication.

  1.  This document is marked as updating many RFCs.  It would be very nice if there was a section that summarized the updates — pointing to the details is ok.  Section 3 does a good job in detailing the changes to rfc4271, but there are other updates elsewhere (section 7.7, for example).  What I’m suggesting is a section that basically says: "rfc4271 is updated by changing the handling of path attribute errors (Section 3), the ORIGIN (Section 7.1), etc.”  Maybe it’s just me, but I like to be able to find the updates in one place (at least a summary).
  2.  Nit:  s/revised/updated     Related to the last point, when looking through a document for updates, searching for the work “update” makes it easier..  The authors chose the word “revised”, which is a perfectly good word, but it is not the “official” term used when an RFC is changed.   Yes, it’s a nit..
  3.  Section 3 talks about updates to rfc4271, and it includes this text in bullet h: "When multiple attribute errors exist in an UPDATE message, if the same approach (either "session reset", "treat-as-withdraw” or "attribute discard") is specified for the handling of these malformed attributes, then the specified approach MUST be used.  Otherwise the approach with the strongest action MUST be used.”  Given that multiple RFCs are updated, it seems to me like this consideration is applicable not just to rfc4271 (even though it is the base), so it might be a good idea to include the same (or at least similar) text in Section 2 (where the approaches are defined and discussed).
  4.  Section 6 (Operational Considerations) includes several rfc2119 keywords.  I read the thread with Brian Haberman about clarifying what is expected from the operator and what the implementation should do; I agree that this section should be clarified so that the operational recommendations are clear and separate from the actions that an implementation can take.  For example: “.. such facilities MUST include logging an error listing the NLRI involved, and containing the entire malformed UPDATE message when such an attribute is detected.  The malformed UPDATE message SHOULD be analyzed, and the root cause SHOULD be investigated.”  Logging is definitely the job of the implementation..while other actions such as investigating clearly would fall on the operator..
  5.  Security Considerations (Section 10).  It is true that malformed optional transitive attributes should not cause remote session resets.  Are remote withdrawals a new threat?  Following the logic in the introduction, an update with a malformed optional transitive attribute may not be properly checked for several hops..but once the error is detected then the NLRI may use Treat-as-withdraw..  Assuming an AS-level granularity in checking, this won’t cause loops or major disruptions, but nonetheless it can result in not everyone receiving the information it should..

Thanks!

Alvaro.