Hi Bruno, You bring up some very good points in your review. I am not a co-author, but please check inline for a few responses.
On Mon, Jul 1, 2024 at 3:03 PM <[email protected]> wrote: > Support. > > This is a very useful feature for operators. > > Thank you for writing it. > > On the good side (I guess) I read the draft. On the bad side (I guess) I > have some comments, and a significant one regarding the choice to allow > implementations to restrict their support to a single tag. > > Thank you very much for the inclusion of the yang data model (as a > first-class citizen in this feature/doc) > > Please find below some comments > > §2 32-Bit Administrative Tag Sub-TLV > > "At least one administrative tag must be advertised." > > May be :s/must/MUST ? > > > Do we want to specify that the order of the tags, within the Sub-TLV, is > irrelevant? > If so, may be the terms "First" and "Last" in Figure 1 are misleading? > KT> Perhaps call them 1 ... N or something like that? > If not, in term of redistribution, do we want to specify that the order > should be kept? (I'm not familiar with OSPF (redistribution) so please > disregards if this is just irrelevant) > > §3 Administrative Tag Applicability > > What about SRv6 locators and SRv6 End SID? > Again, I 'm not familiar with OSPF, but if we believe Admin Tags are a > good thing (and I do) they would likely be a good thing for SRv6. > KT> This is a great catch! The new sub-TLV is also applicable for the OSPFv3 SRv6 Locator TLV of the OSPFv3 SRv6 Locator LSA. We need code point allocation for it under https://www.iana.org/assignments/ospfv3-parameters/ospfv3-parameters.xhtml#srv6-locator-lsa-sub-tlvs > > > §4 Protocol Operation > > "An OSPF router supporting this specification MUST be able to advertise > and interpret at least one 32-bit tag for all type of prefixes." > > Ah... here we are.... > I would prefer that implementation support the any number of tags. Just > like for BGP community (simple, large, extended, wide... all types of BGP > communities have no restriction to a single community). > Especially since I've experienced this limitation with IS-IS already, and > this is painful. Essentially, once you have chosen 1 usage for tag, you > can't use tags for another usage which is strongly limiting the feature. > This is also a big contradictory with the Abstract as the definition of > this new sub-TLV is also to allow for multiple tags "Previously, OSPFv2 and > OSPFv3 were relegated to a single tag". > KT> To me, the feature is first and foremost about the ability to associate tags with intra/inter area prefixes in OSPF. The part about multiple tags comes thereafter. > And this is all the more painful when merging two networks which have a > different usage for the tag (e.g. one for "selective prefix prioritization" > during FIB rewrite and one for selective prefix redistribution between > level/domains. > > I don't really expect to be heard on this point, but when doing feature > for operators -and admin tags is typically for network operations- it would > be good to listen to network operators rather than making it easy to > implement for vendors. (because those days, most implementors will > typically only implement the minimal feature set to claim compliance, and > with a LS IGP, even a good implementation will be limited by the weakest > implementation in the flooding domain). > KT> Are you recommending a SHOULD instead of MAY for support for more than one tag? Or if it is MUST then the next thing is to specify the number of tags ... And how do we decide what that limit is? Or are you suggesting an unlimited number of tags? > > So given this added arbitrary limitation, tags order matter (cf above). > And now we do need implementation to enforce ordering during configuration, > origination, reception, redistribution). Is this really going to be more > simple for implementation. (except for the basic implementation only > handling the first tag) > > "The maximum tags that an implementation supports is a local matter > depending upon supported applications using prefix tags." > I'm not sure about this. In a LS IGP, we typically need consistency within > the flooding domain (although this may depends on the application). > Please add a discussion about this in section 6 "Management Considerations" > > > "When tags are advertised for AS External or NSSA LSA prefixes, the > existing tag in the OSPFv2 and OSPFv3 AS-External-LSA and NSSA-LSA > encodings SHOULD be utilized for the first tag." > Do you expect/mandate implementations to support admin-tags in _addition_ > to existing tag. IOW, implementations will need to support at least 2 tags, > one from "existing" tags, and one from the new tag? If not, sorry but I > fail to see the benefit of this new document, as the new tag could only be > used to advertise the same tag value. Bringing no new value. > KT> Yes, both will be needed for sure. The older tag is a field in the fixed form LSA and so it has to be set with the first tag - I don't see another way. > > "The semantics of the tag order are implementation-dependent." > I object. > If you allow implementation to be restricted to the support of a single > tag (or N tag), order matters. > Plus semantic cannot be implementation-dependent as this would lead to > interop issues. You may say the order has no semantic, but not that the > semantic is implementation-dependent. > > > "Whether or not tag A precedes or succeeds tag B SHOULD not change the > meaning of the tag set." > This seems to contradict the above sentence. > To me, "semantic" and "meaning" is essentially the same concept. > https://dictionary.cambridge.org/dictionary/english/semantic > > > > "The number of tags supported MAY limit the number of tags that are > propagated." > Probably :s/MAY/may > If not :s/MAY/will > > > "When propagating multiple tags between areas as previously described, the > order of the the tags SHOULD be preserved so that implementations > supporting fewer tags will have a consistent view across areas." > We need a MUST. Exactly for the reason provided. > (note that all those complexities comes from allowing implementation to > only support a sub-set of the sub-TLV. I.e., we are adding lots of > complexity to support partial is not weak/bad implementations. I don't > think that this is a good trade-off. Especially since we had already > defined a sub-TLV to carry a single tag. So those basic implementations > could just refrain from implementing this new spec. And voila, same > feature set "single tag support". > > > §4.1 > > > "When multiple LSAs contribute to an OSPF route, it is possible that these > LSAs will all have different tags. In this situation, the OSPF router MUST > associate the tags from one of the LSAs contributing a path and, if the > implementation supports multiple tags, MAY associate tags from multiple > contributing LSAs up to the maximum number of tags supported. It is > RECOMMENDED that tags from LSAs are added to the path in ascending order of > LSA originator Router-ID." > > I feel that the first tag (supported/seen by all implementations) MUST > comes from the contributing path. Only sub-sequent tags should be ordered. > KT> I believe the part about contributing paths is implied since the non-contributing paths cannot contribute tags. Perhaps this clarity can be improved by the authors. Thanks, Ketan > Is there a specific reason for this order? (which may be weakly > deterministic for the operator). Why not ordering based on the tag value, > which is chosen by the operator and for which the operator would decide a > meaningful order? (e.g. most important tags has a small value) > > > §5 > "BGP-LS [RFC9552] introduced the support for advertising administrative > tags associated with prefixes using the BGP-LS IGP Route Tag TLV (TLV 1153) > that is used to carry the OSPF Administrative Tags specified in this > document." > > Who am I to comment on English readability when the editor is Acee? 😉 > At least for non-native speaker, I feel that the readability of this > sentence could be improved. > > §6 Management Considerations > > I think this section should discuss the operational issues created by the > choice to allow lower-grade implementation to only support 1 or N tag(s). > And possibly the possible issues brought by implementation supporting a > single tag from both "existing tags" and those new tags. > > 7. YANG Data Model > > Thank you for including this section. > I don't speak yang hence can't review. But I'll nonetheless provide one > comment (hi ultracrepidarian) > > It's not clear to me whether the model allow the enforcement of ordering > of tag configuration. Especially for the first one. > While since some implementation will only support the first (N) one, this > seems relevant. > > > §8 > > Thank you for the comprehensive security section. > I'm wondering whether some security guy would not ask for adding > consideration about privacy as tag/OSP info are typically not encrypted and > tag may be used to carry privacy related information(or a least information > which may be sensitive. E.g. this loopback/router is located in country X; > not to be trusted). > > §10 > " the ISIS specification" > May be "IS-IS" (as per RFC 5130 and RFC editor Abbreviations List) > > Regards, > --Bruno > > -----Original Message----- > From: Christian Hopps <[email protected]> > Sent: Monday, June 17, 2024 8:42 PM > To: [email protected] > Cc: [email protected]; [email protected]; > [email protected] > Subject: [Lsr] WG Last Call for draft-ietf-lsr-ospf-admin-tags > > > This begins a 2 week WG Last Call, ending Monday July 1st, 2024, for: > > https://datatracker.ietf.org/doc/draft-ietf-lsr-ospf-admin-tags > > Authors, > > Please indicate to the list, your knowledge of any IPR related to this > work. > > Thanks, > Chris. > > ____________________________________________________________________________________________________________ > Ce message et ses pieces jointes peuvent contenir des informations > confidentielles ou privilegiees et ne doivent donc > pas etre diffuses, exploites ou copies sans autorisation. Si vous avez > recu ce message par erreur, veuillez le signaler > a l'expediteur et le detruire ainsi que les pieces jointes. Les messages > electroniques etant susceptibles d'alteration, > Orange decline toute responsabilite si ce message a ete altere, deforme ou > falsifie. Merci. > > This message and its attachments may contain confidential or privileged > information that may be protected by law; > they should not be distributed, used or copied without authorisation. > If you have received this email in error, please notify the sender and > delete this message and its attachments. > As emails may be altered, Orange is not liable for messages that have been > modified, changed or falsified. > Thank you. > _______________________________________________ > Lsr mailing list -- [email protected] > To unsubscribe send an email to [email protected] >
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
