Hi, Eric. On Fri, 2014-04-11 at 16:07 -0400, Eric Osborne wrote: > Hi Elwyn- > > A few questions and comments inline with EO#. > > > > Specifics: > > On-the-wire format: > > - The encoding of the TLV Length field is not specified (unsigned binary > > integer in network bit order). > > - The encoding of the individual TLV length and Type fields in s2 should > > also be specified (unsigned binary integer in network bit order). > > - The value of TLV Length should be more precisely specified. Suggest: > > The TLV Length is the sum of the lengths of all TLVs in the message, > > where the length of a TLV is the sum of the lengths of the three > > TLV fields, i.e., the the length of the value field + 4. > > > EO# > Your comments are around the on-wire format for PSC TLVs. It seems > weird to specify the encoding *only* for TLVs and not for the rest of > PSC. So the first thing I thought of was adding a section that says > "all of PSC is network byte order". This seems like overkill, though, > as no other MPLS-TP RFCs discuss byte order. Would any reasonable > implementer get everything else right in all other protocols and then > get it wrong in PSC TLVs?
The other parts of the PSC don't involve integers and so aren't affected. The usual solution is to add a single statement to the effect that all integer fields are encoded as unsigned integers in network bit order. Assuming things is always dangerous in standards. > > > > > Malformed messages check: > > - Check values of fields prior to TLV Length are consistent with s4.2 of > > RFC 6378. > > - Check overall length of message matches value in TLV Length + 12. > > - Check Sum of lengths of TLVs matches value in TLV Length. > > - Check all TLV types received are recognized. > > > > EO# Same kind of question - is this necessary? It seems like an > implementation issue. I'd hate to see an implementation that lets > other errors pass by uncaught because psc-updates didn't tell the > implementer what other sorts of errors to look for. > > For both of these questions I looked at a few big RFCs as examples - > 2328, 5340, 2460, 6427, 4271 - and got very little out of them. > > 4271 just says "All multi-octet fields are in network byte order." > None of the rest say anything about network bit/byte ordering or the > error checking an implementation must have in order to be compliant. > > And of course rfc6378 itself doesn't cover this much either, for any > of its own fixed fields. > > It took me longer to go do that research than it would have to just > write the section you want, so I'm not really wedded to leaving that > sort of stuff out. It just seems like overkill to put it only in PSC > TLVs when it's nowhere else. > > Is this level of detail really necessary? Doubtless that is a matter of opinion, but in my experience of code inspection and functional specifications/standards, providing a checklist while we are thinking about it can help to avoid the sort of security doom that OpenSSL is coping with at the moment. > > > > > Behaviour in the event of receiving a malformed message: > > - There has been discussion of appropriate behaviour on the MPLS mailing > > list. > > > > Behaviour in the event of receiving a well-formed but inappropriate TLV > > in a message: > > - This needs to be specified. (might be mode specific) > > EO# ACK - these two need to be specified as per discussion on the list. > > > > > Nits/Editorial Comments: > > General: s/i.e. /i.e., /g, s/e.g. /e.g., /g > > > > Abstract/s1: Make the count of changes consistent (four/five currently). > > Might be better just to say 'a number of changes' and leave the reader > > to count. > > > > s1: Since the number of items has grown to five and maybe six (depending > > on how the above changes are inserted), it would helpful to link the > > categorization to the actual section numbers.) > > > > s2: It would be good to have the conventional picture here - just grab > > the one from draft-mpls-tp-psc-itu. > > > > s2, Value field: Better to say that this has the number of octets > > specified in the length field rather than talking about multiples of 4 > > again. The discussion of padding seems superfluous. > > > > > EO# It seems odd that you want me to take our four words around > padding while adding a section on implementation-level error > checking... You are not obliged to follow the comment. Anything here is designed to make it easier for people to follow what is going on after memories have faded... and we all have our views of how this is best achieved. Regards Elwyn > > > > > > > > eric > > > s4, next to last para: Should 'A remote No Request message SHALL be > > ignored' be > > 'A remote NR message SHALL be ignored' for consistency? > > > > s5,para 8: s/In both cases the request which was driving/In both cases the > > request that was driving/ > > > > s6: It might be worth pointing out that extensions of PSC (like tp-psc-itu) > > may > > introcuce additional capabilities and state that it is up to these > > sepcifications to > > say how capability mismatch in this areas is/are handled. > > > > > > > > > > > > _______________________________________________ > > Gen-art mailing list > > [email protected] > > https://www.ietf.org/mailman/listinfo/gen-art > > > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
