Re: [Rift] comments on draft-ietf-rift-rift-11

[Antoni Przygienda <[e-mail]>]

5. yes, explanation is in spec
8. field size is fixed in schema but you can set seqnr# any way you want as long you don’t violate the spec
12. this will only confuse the issue, all pretty cram’ed together & I have a very narrow ASCII margin to ahere to.
15. I would not know how to adjust it
17. Hmm, an example is probably bit more than that. Pls sync up with Bruno maybe to help you write something more clear. I think he did the preso on the weak nonces.

Thanks


  *   Tony

From: "[e-mail]" <[e-mail]>
Date: Tuesday, May 19, 2020 at 10:59 PM
To: Antoni Przygienda <[e-mail]>
Cc: "Sharma, Alankar" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, Dmitry Afanasiev <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, Zhaohui Zhang <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: Re:[Rift]  comments on draft-ietf-rift-rift-11

[External Email. Be cautious of content]


Hi Tony,



Thank you very much for your quick response!

And thank you for the accepted comments.

Please find my comments in line for some responds with Sandy>.



Thanks,

Sandy


原始邮件
发件人：AntoniPrzygienda <[e-mail]>
收件人：张征00007940;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
抄送人：[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;Jeffrey (Zhaohui)Zhang <[e-mail]>;[e-mail] <[e-mail]>;
日 期 ：2020年05月20日 00:55
主 题 ：Re: [Rift]  comments on draft-ietf-rift-rift-11
Hey Sandy, thanks, iline


  1.  That’s intentional. There was a complain that 4.1.2.1 did not define “fallen leaf” in the context of the section and it was added

Sandy> OK.

  1.  Correct, that should not be leaf, changed to “node”. Redundancy was already removed (I’m working on version -12 that will be published right before it goes to IESG reviews again [not sure whether chairs forwarded for 2nd review yet])

Sandy> I am not sure if it should be the second review. IMO it's not necessary if the modification only addresses these comments.

  1.  Good catch, left from previous schema version
  2.  Done
  3.  No, it’s not. Spec explains it in flooding somewhere else, basically you have to reconverge in pathological cases where multiple levels go down in an orchestrated fashion and old northbound information may persist

Sandy> Oh. It means that in common cases it will not occur, but in some pathological situation it may occur, right?

  1.  Ack
  2.  Ack
  3.  Nope, it’s not RIFT specific. Issue well understood and implementation specific but we metion is since people were questioning the way it’s done in RIFT

Sandy> OK. So it's a optional way to choose 16-bit in sequence to be checksum, right?

  1.  Lower level always reflects, so yes but saying it again would state a tautology, not helpful.

Sandy> OK.

  1.  Non-polynomial, well-known terem in computer science.https://en.wikipedia.org/wiki/NP-completeness<https://urldefense.com/v3/__https:/en.wikipedia.org/wiki/NP-completeness__;!!NEt6yMaO-gk!SbJIq5ypqA-HXQgscgLINtDZf4PvI7JT5uyT-LUQnf_RWGvIdLoTmar82Ts2tA$>

Sandy> Thank you! I misunderstood it.

  1.  Yes, good catch
  2.  Yes, they are. The comment has been made before. Read 4.3.3. in its entirety carefully and you’ll see it’s water-tight.

Sandy> Yes. I read all the section and I understand it. But somebody may only read the section 4.3.3.1 to get the comparison rule. So it may be better by only add one sentence: "The newest one is always preferred."

  1.  Done
  2.  Space
  3.  Hard to really classifiy since rift allows to mix configured and unconfigured nodes just fine. Yes, it’s kind of FAM

Sandy> Thanks for clarification. So if the figure will be adjust a little?

  1.  Nope, that’s correct
  2.  If you want example, write the text please

Sandy> May like this? "For example, simple incremental computation rule can be used. In case of node A is incremental by multiplying 5, node B is incremental by plus 3, then A/B can decide that the packet from B/A is not valid because the value is not incremental correctly according to the rules."

  1.  Ack

--- tony

From: "[e-mail]" <[e-mail]>
Date: Tuesday, May 19, 2020 at 1:32 AM
To: Antoni Przygienda <[e-mail]>, "Sharma, Alankar" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, Dmitry Afanasiev <[e-mail]>
Cc: "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, Zhaohui Zhang <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: [Rift]  comments on draft-ietf-rift-rift-11

[External Email. Be cautious of content]


Hi authors,



This draft is well-written. Thank you very much for your work on this draft!

I have some questions and comments:



=============================

1. In section 4.1.2.1, there is the "fallen leaf" definition.

And in section 4.1.3, there is also a definition for "fallen leaf":

"We define a "Fallen Leaf" as a leaf that can be reached by only a

   subset, but not all, of Top-of-Fabric nodes due to missing

   connectivity."

The fallen leaf definition seems like different though in fact the meaning is the same.

So changing the wording in section 4.1.3 may be better, such as don't use the word "define".



=============================

2. In section 3.1, The definition of "Leaf" is "A node without southbound adjacencies.".

In section 4.1.3, the cases description for do not require particular action are:

"If a southern link on a leaf node goes down, then connectivity to ..." ,

"If a southern link on a leaf node goes down, then connectivity through that leaf is...",

Is the "southern link" in section 4.1.3 means the connection to servers?

And it seems like the two paragraphs of "If a southern link on a leaf node..."are almost the same. Is it duplicated?



=============================

3. In section 4.2.2.1,

"3. SEND_LIE: create a new LIE packet

 ...

 3.  setting `you_are_not_flood_repeater` to computed value"



In fact "you_are_flood_repeater" is used in the following sections, though there is no ambiguity,  unified word would be better.



=============================

4. In Section 4.2.3.3.1,

The last paragraph,

"TIDEs and TIREs MUST NOT be re-flooded the way TIEs of other nodes

   are are MUST be always generated by the node itself and cross only to

   the neighboring node."

Nit:

are are/ are and



=============================

5. In section 4.2.3.3.1.2.2 TIDE Processing

"b. for every HEADER in TIDE do

  ...

  6.  if DBTIE.HEADER < HEADER then

    I)    if originator is this node then bump_own_tie else

      i.     if this is a North TIE header from a northbound

                        neighbor then override DBTIE in LSDB with HEADER"

Is this a nit?

a North TIE header from a northbound neighbor/ a North TIE header from a southbound neighbor?



==============================

6. In section 4.2.3.3.1.3.1.  TIRE Generation

"There is not much to say here.  Elements from ..."

If it's better to remove the sentence "There is not much to say here."?



==============================

7. In section 4.2.3.5.  'Flood Only Node TIEs' Bit

"RIFT includes an optional ECN mechanism to prevent ..."

Reader may not understand the acronym "ECN", "ECN (Explicit Congestion Notification)" may be better.



==============================

8. In section 4.2.3.7,

The emulation of "checksum behavior" is not very clearly to me. I am not sure I got the difference between computing checksum and fill in a specific field like tranditional link-state protocol and computing checksum and fill it in the sequence field.

If more descriptions can be added for it?



==============================

9. In section 4.2.3.8,

" The term "all other nodes at X's' level" describes obviously just the

   nodes at the same level in the PoD with a viable lower level ..."

How do we understand the words "with a viable lower level"? Is it "with a viable lower level reflection"?



==============================

10. In section 4.2.3.9,

"... is (similar to) a NP-Complete problem and a globally..."

What is NP? Network Processor?



==============================

11. In section 4.2.3.9,

"let G be a grandparent node of N, reachable transitively via a

      parent P over adjacencies ADJ(N, P) and ADJ(P, G).  Observe that N

      does not have enough information to check bidirectional

      reachability of A(P, G);"

Shoud a unified abbreviation "A" or "ADJ" be used?



==============================

12. In section 4.3.3.1,

In proceeding sections it's clear that the newest one is preferred. But the comparison rules with "older" description are in rule 1 and 4.

There is no clear sentence that describes the result leading by these rules.



==============================

13. In section 4.3.3.4,

LISP is mentioned in this section, but there is no reference to LISP protocol in the document.

IMO it's better to add the reference to LISP (rfc6830bis) in informative references section or rfc6830 in normative references.



==============================

14. In figure 2, 29, 32, 33 and 34, the "Spine" losts the "e". But it is not even a nit.



==============================

15. In section 4.4.1,

I am confused with "Level Provisioning", is this configured in each level? It seems like it should be configured on each node, right?

If it's right, I am also confused with the integrity and flexibility comparison in figure 30. It seems like the integrity of "Level Provisioning" is beyond "FAM".



==============================

16. In section 4.4.3,

"Remaining TIE Lifetime:  32 bits.  In case of anything but TIEs this

      field MUST be set to all ones and Origin ..."

If the "TIEs" should be "LIEs"?



==============================

17. In section 4.4.4 Weak nonces,

I remember that a good nonce example was presented in previous IETF meeting.

It may be better to add a simple example in this section to make it more easier to understand.



==============================

18. In section 5.3,

"The mechanism to resolve this scenario hinges on ToF 21's Sout TIEs ..."

sout/south



Thanks,

Sandy




Juniper Business Use Only




Juniper Business Use Only