On June 10, 2020 at 10:32:09 AM, Robert Wilton wrote:
Rob: Hi! Thanks for the review. I’m replying for the authors as I think that your questions/suggestions should be easy to address. Please see below. Thanks! Alvaro. > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > I found parts of this document hard to understand, but I'm not familiar with > the specifics of the protocols. > > This discuss is in the vein of "I think that folks might struggle to > implement this correctly/consistently". In particular I had some > questions/concerns about section 5 which, if clarified, would probably help > this document. > > In Section 5: > > The ASLA sub-TLV is an optional sub-TLV and can appear multiple times > in the OSPFv2 Extended Link TLV and OSPFv3 Router-Link TLV. The ASLA > sub-TLV MUST be used for advertisement of the link attributes listed > at the end on this section if these are advertised inside OSPFv2 > Extended Link TLV and OSPFv3 Router-Link TLV. It has the following > format: > > I think that it would be useful to clarify when/why the ASLA sub-TLV can be > included multiple times. I.e. when different applications want to control > different link attributes. That sounds like a simple addition. > Standard Application Identifier Bits are defined/sent starting with > Bit 0. Undefined bits which are transmitted MUST be transmitted as 0 > and MUST be ignored on receipt. Bits that are not transmitted MUST > be treated as if they are set to 0 on receipt. Bits that are not > supported by an implementation MUST be ignored on receipt. > > It was not clear to me what it means if the SABM (or UDABM) fields are > entirely empty. This paragraph states that they are treated as if they are > 0, but sections 8 and 11 imply that if the field is omitted then it acts as > if all applications are allowed. Section 12.2 implies that if the field is > omitted then it is as if all applications are allowed unless there there is > another ASLA with the given application bit set, in which case it is treated > as being a 0 again. I think that this document would be helped if the > specific behaviour was defined in section 5, retaining the > justification/clarification in the subsequent sections. Empty is different than omitted. Omitted (the length of the field is 0) means: "these attributed apply to all applications, so I'm not even going to bother setting the bits". Empty (the field is present, but all the bits are set to 0) means that the sender is saying that "no application applies to this set of attributes". I can't think of a circumstance when a sender would do this, as it is basically an empty announcement: it doesn't apply to anything....but it is also not wrong and would simply not be used. OTOH, there is a case where the sender can set a bit (or more) for an application it supports (maybe a new one) -- if the receiver doesn't support that application it will then ignore the bit, and it will look as if nothing was set: none of the supported applications (at the receiver) match. Again, the receiver will simply not use the information. > It is also not entirely clear to me exactly how the bits are encoded on the > wire. My assumption is that if bit 0 is set, then this would sent the > highest bit of the first byte. E.g. 0x80? Is that correct? If not, then I > think that the document needs more text, if so, then an example of the > encoding may still aid readability. Correct. We should have added a figure similar to what is in the ISIS draft: 0 1 2 3 4 5 6 7 ... +-+-+-+-+-+-+-+-+... |R|S|F| ... +-+-+-+-+-+-+-+-+... > User Defined Application Identifier Bits have no relationship to > Standard Application Identifier Bits and are not managed by IANA or > any other standards body. It is recommended that bits are used > starting with Bit 0 so as to minimize the number of octets required > to advertise all UDAs. > > Doesn't this need more constraints to ensure easy interop (i.e. bits > default to 0). Otherwise, it would seem that anyone is allowed to put any > value in this field that they like that could harm interop, or otherwise it > might be tricky to compare a 4 byte UDABM to an 8 byte UDABM? Good point! Even if user-defined, I think that text similar to standard applications could be added: Undefined bits which are transmitted MUST be transmitted as 0 and MUST be ignored on receipt. Bits that are not transmitted MUST be treated as if they are set to 0 on receipt. Bits that are not supported by an implementation MUST be ignored on receipt. > This document defines the initial set of link attributes that MUST > use the ASLA sub-TLV if advertised in the OSPFv2 Extended Link TLV or > in the OSPFv3 Router-Link TLV. Documents which define new link > attributes MUST state whether the new attributes support application > specific values and as such MUST be advertised in an ASLA sub-TLV. > The link attributes that MUST be advertised in ASLA sub-TLVs are: > > I think that I get what this means, but I find the last two sentences > slightly jarring given than the ASLA TLV is optional. Perhaps predicate > both of these constraints with "(if supproted)". E.g., something like, > > Documents which define new link > attributes MUST state whether the new attributes support application > specific values and as such MUST be advertised in an ASLA sub-TLV (if > supported). The link attributes that MUST be advertised in ASLA sub-TLVs (if > supported) are: Sure... The ASLA sub-TLV is optional from the protocol point of view: it is not expected in every OSPF advertisement. IOW, only implementations supporting this specification would even know about it. Given that the MUSTs are within the context of this document, then I think that "if supported" is not needed... _______________________________________________ Lsr mailing list [email protected] https://www.ietf.org/mailman/listinfo/lsr
