Hi Peter, Please see inline ...
I think that there is probably just one point of confusion/ambiguity that needs to be resolved. > -----Original Message----- > From: Peter Psenak <ppse...@cisco.com> > Sent: 11 June 2020 11:30 > To: Rob Wilton (rwilton) <rwil...@cisco.com>; Alvaro Retana > <aretana.i...@gmail.com> > Cc: Acee Lindem (acee) <a...@cisco.com>; draft-ietf-ospf-te-link-attr- > re...@ietf.org; lsr-cha...@ietf.org; lsr@ietf.org; Yingzhen Qu > <yingzhen...@futurewei.com>; The IESG <i...@ietf.org> > Subject: Re: Robert Wilton's Discuss on draft-ietf-ospf-te-link-attr- > reuse-14: (with DISCUSS) > > Hi Rob, > > > thanks for your comments, please see inline (##PP) > > On 10/06/2020 19:18, Rob Wilton (rwilton) wrote: > > Hi Alvaro, > > > > > >> -----Original Message----- > >> From: Alvaro Retana <aretana.i...@gmail.com> > >> Sent: 10 June 2020 16:49 > >> To: Rob Wilton (rwilton) <rwil...@cisco.com> > >> Cc: Acee Lindem (acee) <a...@cisco.com>; draft-ietf-ospf-te-link-attr- > >> re...@ietf.org; lsr-cha...@ietf.org; lsr@ietf.org; Yingzhen Qu > >> <yingzhen...@futurewei.com>; The IESG <i...@ietf.org> > >> Subject: Re: Robert Wilton's Discuss on draft-ietf-ospf-te-link-attr- > >> reuse-14: (with DISCUSS) > >> > >> 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. > > [RW] > > > > Yes, hopefully. > > > >> > >> > >> 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. > > ##PP > Updated as: > > > The ASLA sub-TLV is an optional sub-TLV of OSPFv2 Extended Link TLV and > OSPFv3 Router-Link TLV. Multiple ASLA sub-TLVs can be present in its > parent TLV when different applications want to control different link > attributes or when different value of the same attribute needs to be > advertised by multiple applications. > [RW] That looks fine. > >> > >> > >> > >>> 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". > > [RW] > > > > I don’t particularly like this as the solution, since if feels > inconsistent to me. I.e. if you don’t advertise a bit always treat it as > zero, unless you don’t say anything at all, in which case treat it as 1. > I would have preferred for it to have an explicit flag bit in the ASLA TLV > to indicate that all applications are supported ... more below (see *). > > > >> > >> > >> 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. > > [RW] > > > > I agree. Arguably, the document could state that at least one bit in > the SABM or UDABM flags SHOULD be set. > > > > > > > >> > >> 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. > > [RW] > > > > This is okay with me. > > > > But it is this text in section 12.2 that seems to define > different/complicated semantics: > > > > 12.2. Use of Zero Length Application Identifier Bit Masks > > > > If link attributes are advertised associated with zero length > > Application Identifier Bit Masks for both standard applications and > > user defined applications, then any Standard Application and/or any > > User Defined Application is permitted to use that set of link > > attributes so long as there is not another set of attributes > > advertised on that same link which is associated with a non-zero > > length Application Identifier Bit Mask with a matching Application > > Identifier Bit set. If support for a new application is introduced > > on any node in a network in the presence of such advertisements, > > these advertisements are permitted to be used by the new > application. > > If this is not what is intended, then existing advertisements MUST > be > > readvertised with an explicit set of applications specified before a > > new application is introduced. > > > > I read this as: > > > > If a sender only advertises 0 for SABM and UDABM length fields then the > associated ASLA link attributes apply to all applications. > > > > However, if a different router advertises 0 for SABM and UDABM length > fields in an ASLA TLV but includes a second ASLA TLV with an SABM or UDABM > bit (possibly with one or more link attributes), then the attributes > associated with the first ASLA TLV, that match the application bit in the > second ASLA TLV, are now disabled rather than being enabled. I think that > I can understand why this is done, but it also seems ... complicated. > > ##PP > What is complicated? The logic is actually quite simple - more specific > advertisement wins above a wildcard advertisement. [RW] Okay, if that is what you are trying to achieve then I am okay with that, but that was not how I interpreted the text in 12.2, and I suspect that this is the root of why I perceive the algorithm is complex :-). My interpretation (which I think is valid) was thus: I.e. by advertising say a single attribute (e.g. "Unidirectional Link Delay") in one ASLA TLV for a specific application changes how the receiver interprets all other attributes e.g. say "Unidirectional Delay Variation" in the wildcard advertisement (due to the following text in 12.2): If link attributes are advertised associated with zero length Application Identifier Bit Masks for both standard applications and user defined applications, then any Standard Application and/or any User Defined Application is permitted to use that set of link attributes so long as there is not another set of attributes advertised on that same link which is associated with a non-zero length Application Identifier Bit Mask with a matching Application Identifier Bit set. In particular the "so long as ..." clause seems to change how the default attributes are interpreted. Probably this paragraph could be improved to make its intent more clear? I.e. the attributes values in the default ASLA apply for all applications, unless that are overridden by application specific definitions of particular attributes, and all other default attribute value still apply. > > > > > I would have preferred to see something like the following, stated in > section 5: > > - A given link attribute should only appear in a single ASLA TLV, and is > ignored if it appears in a second ASLA TLV (already stated) > > ##PP > This is already present in the draft: > > "If the same attribute is advertised in more than single ASLA sub-TLVs > with the application listed in the Application Bit Masks, the > application SHOULD use the first instance of advertisement and ignore > any subsequent advertisements of that attribute." [RW] Yes, sorry that is what I meant by (already stated). > > > > - The link attributes in an ASLA TLV can either: > > (i) apply to all applications (preferable indicated with an explicit > flag (*), or otherwise by SABM and UDABM length fields both = 0), or > > (ii) only applies to the specified bit list of applications. > > ##PP > Above is exactly what is proposed. I do not see a need for a special > flag though. The only additional logic is that more specific > advertisement with app bit set can overwrite the less specific > advertisement (zero length SABM). [RW] I'm okay with this, as long as it is clearly described. > > > > > I.e. I would prefer the first part of 12.2 text is dropped, which really > means that if the link attributes are being explicitly advertised for one > application, then they should be be explicitly listed for all applications > that support them. But perhaps this doesn't work well in practice. > > ##PP > there is a good reason for leaving the "Zero Length Application > Identifier Bit Masks" support there - if there is no need for an app > specific values, using the wildcard advertisement is simple and efficient. [RW] Having defaults is fine, I think that it was my different interpretation (or misinterpretation) of your 12.2 text that was throwing me off track. > > > > > However, I appreciate that this suggestion is perhaps beyond my remit. > Hence, keeping the existing behaviour is okay, abeit more complicated that > I like, but I do think that it would aid the document clarity if the > behaviour was clearly specified in section 5. > > ##PP > sure, I have moved the text from 12.2 to 5. [RW] Thanks. As above, I also think that this text needs to be refined/clarified. > > > > > > > > >> > >> > >> > >>> 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| ... > >> +-+-+-+-+-+-+-+-+... > >> > > [RW] > > > > Yes, having reviewed the ISIS draft after the OSPF draft, I found the > equivalent section in the ISIS draft easier to understand. Hence, adding > the diagram would be good. > > ##PP > Added the drawing of bits. [RW] Thanks. > > > > > > >> > >> > >> > >>> 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. > >> > > [RW] > > > > I agree. > > ##PP > 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." [RW] Looks good. > > > > > >> > >> > >>> 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... > > [RW] > > > > I don't feel too strongly on this point, so if you wish to leave it as > is that is okay with me ... although on further reading it is worth > nothing that both the link attribute and ASLA TLV are optional. An > alternative way of wording this could be: > > > > Documents which define new link attributes MUST state whether the new > attributes support application > > specific values and as such are advertised in an ASLA sub-TLV. The > standard link attributes that are advertised in ASLA sub-TLVs are: ... > > ##PP > changed to the above. [RW] Thanks, Rob > > > > > Thanks, > > Rob > > > > > > _______________________________________________ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
