From nobody Wed Feb 15 05:00:07 2023 Return-Path: X-Original-To: rift@ietfa.amsl.com Delivered-To: rift@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 64AC5C17D667; Wed, 15 Feb 2023 05:00:06 -0800 (PST) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.098 X-Spam-Level: X-Spam-Status: No, score=-2.098 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ClPWOxJow8dG; Wed, 15 Feb 2023 05:00:05 -0800 (PST) Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id E4080C17CEA1; Wed, 15 Feb 2023 05:00:04 -0800 (PST) Received: by mail-pf1-x42e.google.com with SMTP id bd35so7595957pfb.6; Wed, 15 Feb 2023 05:00:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date :mime-version:from:from:to:cc:subject:date:message-id:reply-to; bh=nCGjjKX7jHZ46NbB/NRTTzc2tEESgqGfnQkuZNDC1TU=; b=o1Ab8BCDRQRWRyj9KHwtSdUwZcxOAjCQoZ7gmzvsef5h9+jTifYmrzvuwfLrSVMcep QJNhenAh2fL3YzA449Pq9tDnG8exFnV09BwA9qMeST3tic/T8+ymYRnJr3Gr2TiPhJuu lK8w+PeY665Tb64e/R03xKJo/8h1/zxgAEFkidkULvZYbQFTx9hj0wpNXv3GqUlWreAQ CF68UG7Jou0bwR6ykj7zihOljIgcvo7U5wXlCZQrt+pRgkFa/9C1IJxOHS0BQmwazTaV c44Ky6ygHto1hC9pBBhVIV0i/4Srf7ZqsOUa6YTxUkc3+DVdvUWFAoHzy5BxM+QiJDwc Dz0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date :mime-version:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=nCGjjKX7jHZ46NbB/NRTTzc2tEESgqGfnQkuZNDC1TU=; b=IhtLatCywTOUJKAsMsFdstDvuzDzv/5qJdTDSyvqxUakTvBiZgqvkr1QkVKK8LyiGl M1uF4mQeqkI5aTvWkqEiLgAVnI7san59RxM1ASJ2dLvOkE+24k1taziJOzyKw/87AlGh SMyz7iqN3U6wAm59GHDd/5DRgtQXDbesimPAAGnAF8DK+c3370SBD0YtKIcnUrWD/ny0 jFqgrPyq/pIjeyK9fiLfUyZgCJNlNG2KVlwHDSl5StH6BTnaJe1dTcyB7pqrr8vJb+Jq ehpGKxZ71699vkF/r5huERumno1nrtCATbWBUvH6GqqfzUa2VPHwctr2lB63QZXc7gWh 1vSA== X-Gm-Message-State: AO0yUKU3HyHXSGAeZpi8/6XmxE4T0AjQreUifWuWUYPDBnZm2HY/cozz U5h4I3U47RL+nzFIfiDtEjBClQ9GuivVo4x3sl0= X-Google-Smtp-Source: AK7set/oGb1Q3btPI9UhWVc192Pe68HGGV28RIg6n8mgHOplw4p3lU+WMGjIOHyiOueIq9C4o/FWKb8IrrO44shUV1M= X-Received: by 2002:aa7:8f17:0:b0:598:b25d:fdb2 with SMTP id x23-20020aa78f17000000b00598b25dfdb2mr345658pfr.0.1676466003952; Wed, 15 Feb 2023 05:00:03 -0800 (PST) Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Wed, 15 Feb 2023 05:00:02 -0800 From: Alvaro Retana MIME-Version: 1.0 Date: Wed, 15 Feb 2023 05:00:02 -0800 Message-ID: To: Jordan Head Cc: draft-ietf-rift-rift@ietf.org, rift-chairs@ietf.org, "rift@ietf.org" , "EXT-zhang.zheng@zte.com.cn" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Archived-At: Subject: [Rift] AD Review of draft-ietf-rift-rift-16 (4.2.3.3.1) X-BeenThere: rift@ietf.org X-Mailman-Version: 2.1.39 Precedence: list List-Id: Discussion of Routing in Fat Trees List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Feb 2023 13:00:06 -0000 Hi! This message contains =C2=A74.2.3.3.1 and a couple of "detours" -- I pointed those out with separators; for example I dug introduce you_are_sending_too_quickly and the comments below indicate that this way: =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D ... =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D Alvaro. 2448 4.2.3.3.1. =C2=A0Normative Flooding Procedures 2450 =C2=A0 On reception of a TIE with an undefined level value in the pac= ket 2451 =C2=A0 header the node MAY issue a warning and indiscriminately disca= rd the 2452 =C2=A0 packet. =C2=A0Such packets can be useful however to establish = e.g. via 2453 =C2=A0 `instance_name`, `name` and `originator` elements in `LIEPacke= t` 2454 =C2=A0 whether the cabling of the node fulfills expectations, even be= fore 2455 =C2=A0 ZTP procedures determine levels across the topology. [] This paragraph seems out of place. [major] "MAY issue a warning and indiscriminately discard the packet" Given that this is an option, what are the considerations to do so? Is the warning due to the "TIE with an undefined level" or to the fact that the packer was discarded? =C2=A0If the former, can the actions be performed independently of each other; for example, drop the packet but not log a warning? [minor] "indiscriminately discard...packets can be useful" I'm guessing that "indiscriminately" was used to emphasize the action. If the packet can se useful, but not flooded (since we're in this section), maybe the correct action is to ignore (not to discard). =C2=A0?? [minor] "useful however to establish e.g. via `instance_name`, `name` and `originator` elements in `LIEPacket` whether the cabling of the node fulfills expectations" I'm guessing this is not explained anywhere and that it is probably an operational issue to be covered in the applicability document, right? 2457 =C2=A0 This section specifies the precise, normative flooding mechani= sm and 2458 =C2=A0 can be omitted unless the reader is pursuing an implementation= of the 2459 =C2=A0 protocol or looks for a deep understanding of underlying infor= mation 2460 =C2=A0 distribution mechanism. [] This seems to be a better first-paragraph for the section. 2462 =C2=A0 Flooding Procedures are described in terms of a flooding state= of an 2463 =C2=A0 adjacency and resulting operations on it driven by packet arri= vals. 2464 =C2=A0 The FSM itself has basically just a single state and is not we= ll 2465 =C2=A0 suited to represent the behavior. =C2=A0An implementation MUST= either 2466 =C2=A0 implement the given procedures in a verbatim manner or behave = on the 2467 =C2=A0 wire in the same way as the provided normative procedures of t= his 2468 =C2=A0 paragraph. [nit] s/a flooding state/the flooding state [minor] "The FSM itself has basically just a single state and is not well suited to represent the behavior." There's no TIE FSM, right? =C2=A0Then why even mention it? [major] "MUST either implement the given procedures in a verbatim manner or behave on the wire in the same way as the provided normative procedures of this paragraph." Normative requirements with options are not great. =C2=A0s/MUST/is expected= to Also, I don't know what the difference is between the options. 2470 =C2=A0 RIFT does not specify any kind of flood rate limiting since su= ch 2471 =C2=A0 specifications always assume particular points in available 2472 =C2=A0 technology speeds and feeds and those points are shifting at f= aster 2473 =C2=A0 and faster rate (speed of light holding for the moment). [] Judgement about other technology. :-( Suggestion> RIFT does not specify any kind of flood rate limiting. Instead, the packets provide hints... 2475 =C2=A0 To help with adjustement of flooding speeds the encoded packet= s 2476 =C2=A0 provide hints to react accordingly to losses or overruns via 2477 =C2=A0 `you_are_sending_too_quickly` in `LIEPacket` and `Packet Numbe= r` in 2478 =C2=A0 security envelope described in Section 4.4.3. =C2=A0Flooding o= f all 2479 =C2=A0 corresponding topology exchange elements SHOULD be performed a= t 2480 =C2=A0 highest feasible rate whereas the rate of transmission MUST be 2481 =C2=A0 throttled by reacting to packet elements and adequate features= of the 2482 =C2=A0 system such as e.g. queue lengths or congestion indications in= the 2483 =C2=A0 protocol packets. [nit] s/in `LIEPacket`/in the `LIEPacket` [nit] s/in security envelope/in the security envelope =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D [] I went looking for `you_are_sending_too_quickly`: 2952 4.2.3.5. =C2=A0'Flood Only Node TIEs' Bit 2954 =C2=A0 RIFT includes an optional ECN (Explicit Congestion Notificatio= n) 2955 =C2=A0 mechanism to prevent "flooding inrush" on restart or bring-up = with 2956 =C2=A0 many southbound neighbors. =C2=A0A node MAY set on its LIEs th= e 2957 =C2=A0 corresponding `you_are_sending_too_quickly` flag to indicate t= o the 2958 =C2=A0 neighbor that it should temporarily flood node TIEs only to it= and 2959 =C2=A0 slow down the flooding of any other TIEs. =C2=A0It SHOULD only= set it in 2960 =C2=A0 the southbound direction. =C2=A0The receiving node SHOULD acco= mmodate the 2961 =C2=A0 request to lessen the flooding load on the affected node if so= uth of 2962 =C2=A0 the sender and SHOULD ignore the indication if northbound. [nit] s/flood node TIEs only/only flood Node TIEs [major] "It SHOULD only set it in the southbound direction." The "SHOULD only" construct is always confusing. s/ =C2=A0 =C2=A0A node MAY set on its LIEs the corresponding `you_are_sending_= too_quickly` =C2=A0 =C2=A0flag to indicate to the neighbor that it should temporarily fl= ood node TIEs =C2=A0 =C2=A0only to it and slow down the flooding of any other TIEs. =C2= =A0It SHOULD only =C2=A0 =C2=A0set it in the southbound direction. / =C2=A0 =C2=A0A node MAY set the `you_are_sending_too_quickly` flag in its L= IEs to =C2=A0 =C2=A0indicate to southbound neighbors to temporarily only flood Nod= e TIEs =C2=A0 =C2=A0to it and slow down the flooding of any other TIEs. [major] "The receiving node ...SHOULD ignore the indication if northbound." s/northbound/north of the sender When is it ok to not ignore the indication if north of the sender? Why is this action recommended and not required? =C2=A0The schema says that it is "Ignored when received from southbound neighbor.", which sounds to me more like a "MUST". 2964 =C2=A0 Obviously this mechanism is most useful in the southbound dire= ction. 2965 =C2=A0 The distribution of node TIEs guarantees correct behavior of 2966 =C2=A0 algorithms like disaggregation or default route origination. 2967 =C2=A0 Furthermore though, the use of this bit presents an inherent t= rade- 2968 =C2=A0 off between processing load and convergence speed since suppre= ssing 2969 =C2=A0 flooding of northbound prefixes from neighbors permanently wil= l lead 2970 =C2=A0 to traffic loss. [minor] "Obviously this mechanism is most useful in the southbound directio= n." It may not be obvious to everyone... Is there any utility in the other direction? =C2=A0If `you_are_sending_too_quickly` is ignored anyway, it seems like this sentence is not needed. =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D [] I also went looking for `Packet Number`. Where's the Security Envelope in the schema? >From =C2=A74.4.3: 5275 =C2=A0 Packet Number: 5276 =C2=A0 =C2=A0 =C2=A016 bits. =C2=A0An optional, per adjacency, per pa= cket type 5277 =C2=A0 =C2=A0 =C2=A0monotonically increasing number rolling over usin= g sequence number 5278 =C2=A0 =C2=A0 =C2=A0arithmetic defined in Appendix A. =C2=A0A node SH= OULD correctly set the 5279 =C2=A0 =C2=A0 =C2=A0number on subsequent packets or otherwise MUST se= t the value to 5280 =C2=A0 =C2=A0 =C2=A0`undefined_packet_number` as provided in the sche= ma. =C2=A0This number 5281 =C2=A0 =C2=A0 =C2=A0can be used to detect losses and misordering in f= looding for 5282 =C2=A0 =C2=A0 =C2=A0either operational purposes or in implementation = to adjust 5283 =C2=A0 =C2=A0 =C2=A0flooding behavior to current link or buffer quali= ty. =C2=A0This number 5284 =C2=A0 =C2=A0 =C2=A0MUST NOT be used to discard or validate the corre= ctness of 5285 =C2=A0 =C2=A0 =C2=A0packets. =C2=A0Packet numbers are incremented on = each interface and 5286 =C2=A0 =C2=A0 =C2=A0within that for each type of packet independently= . =C2=A0This allows to 5287 =C2=A0 =C2=A0 =C2=A0parallelize packet generation and processing for = different types 5288 =C2=A0 =C2=A0 =C2=A0within an implementation if so desired. [nit] s/using sequence number arithmetic/using the sequence number arithmet= ic [major] "SHOULD correctly set the number on subsequent packets" This is not a good Normative statement -- among other things, by recommending it you're opening the door for it to be ok for the number to not be "correctly set", not to mention the definition of "correct", etc. Given the references to Appendix A, it is Normative (right?), so perhaps the right approach is to simply drop the sentence. Suggestion> s/ =C2=A0 =C2=A0An optional, per adjacency, per packet type monotonically incr= easing =C2=A0 =C2=A0number rolling over using sequence number arithmetic defined i= n =C2=A0 =C2=A0Appendix A. =C2=A0A node SHOULD correctly set the number on su= bsequent =C2=A0 =C2=A0packets or otherwise MUST set the value to `undefined_packet_n= umber` =C2=A0 =C2=A0as provided in the schema. / =C2=A0 =C2=A0An optional, per adjacency, per packet type number set using t= he =C2=A0 =C2=A0sequence number arithmetic defined in Appendix A. =C2=A0If the= arithmetic =C2=A0 =C2=A0in Appendix A is not used the node MUST set the value to =C2=A0 =C2=A0`undefined_packet_number`. =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D [] I also looked at Appendix A. 6968 Appendix A. =C2=A0Sequence Number Binary Arithmetic 6970 =C2=A0 The only reasonably reference to a cleaner than [RFC1982] sequ= ence 6971 =C2=A0 number solution is given in [Wikipedia]. =C2=A0It basically co= nverts the 6972 =C2=A0 problem into two complement's arithmetic. =C2=A0Assuming a str= aight two 6973 =C2=A0 complement's subtractions on the bit-width of the sequence num= ber the 6974 =C2=A0 corresponding >: and =3D: relations are defined as: [] I'm not going to pretend to understand the details, but it seems that you're saying that RFC1982 is not great and that what is on Wikipedia is better -- Wikipedia claims the same thing. =C2=A0Besides not comparing what RIFT uses with other technology... The references to Appendix A are Normative (as in, Appendix A is how RIFT does it), right? =C2=A0But the reference to RFC1982 is listed as Normative while the one to Wikipedia is Informative. =C2=A0Which one should the implementation follow? =C2=A0The options I see are: RFC1982, Wikipedia, or Appendix A (the specification is self-contained there). I would be very happy with the answer being Appendix A as Wikipedia is not really a stable reference and you already said that RFC1982 is not a good solution. =C2=A0This would mean eliminating the two references and relying on the text in the Appendix. =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D 2485 =C2=A0 A node SHOULD NOT send out any topology information elements i= f the 2486 =C2=A0 adjacency is not in a "ThreeWay" state. =C2=A0No further tight= ening of 2487 =C2=A0 this rule as to e.g. sequence is possible due to possible link 2488 =C2=A0 buffering and re-ordering of LIEs and TIEs/TIDEs/TIREs in a re= al 2489 =C2=A0 implementation for e.g. =C2=A0performance purposes. [major] "SHOULD NOT send...if the adjacency is not in a "ThreeWay" state" Is it ok to ever send TIEs if not in ThreeWay? =C2=A0Why is this behavior recommended and not required? =C2=A74.2.2.1 defines ThreeWay as: =C2=A0 =C2=A0ThreeWay: this state signifies that ThreeWay valid LIEs from a =C2=A0 =C2=A0neighbor are received. On achieving this state the link can be =C2=A0 =C2=A0advertised in `neighbors` element in `NodeTIEElement`. I read that to mean that the TIE describing the local node MUST NOT be sent unless in ThreeWay. =C2=A0But that is different from forwarding other TIEs. I saw that the next sentence says that "No further tightening of this rule as to e.g. sequence is possible..." but what you wrote is not clear (maybe missing a couple of commas) and it doesn't seem to be related to origination vs flooding. 2491 =C2=A0 A node MUST drop any received TIEs/TIDEs/TIREs unless it is in 2492 =C2=A0 ThreeWay state. [] This sentence sort of answers some of my questions above. 2494 =C2=A0 TIDEs and TIREs MUST NOT be re-flooded the way TIEs of other n= odes 2495 =C2=A0 MUST be always generated by the node itself and cross only to = the 2496 =C2=A0 neighboring node. [] Suggestion> =C2=A0 =C2=A0TIEs generated by other nodes MUST be re-flooded. =C2=A0TIDEs = and TIREs =C2=A0 =C2=A0MUST NOT be re-flooded.