Skip to content

Commit

Permalink
RE: WGLC for draft-ietf-roll-dao-projection-26 from Dominique Barthel
Browse files Browse the repository at this point in the history
Many thanks for your review! I pulled and am working on the changes for the comments already.
I really like the way you did it, using the pull request to fix the typo and embed the comments.
I hope we see more reviews done that way in the future.

Please see below:

<!-- DB: is "routes" the verb, with the Root the subject? -->
Yes, hardly readable and quite incorrect as written. Reworded to:
"
   RPL forms Destination Oriented Directed Acyclic Graphs (DODAGs) in which
   the Root often acts as the Border router to connect the RPL domain to the
   IP backbone. Routers inside the DODAG route along that graph up towards
   the Root for the default route and down towards destinations in the RPL
   domain for more specific routes.
"

<!--- DB: first occurence of "segment" in this document, same for "leg". They are not explained anywhere near this figure. -->
 and   <!--- DB: first occurence of "East" and West" in this document. They are not explained anywhere near this paragraph. -->
 and   <!--- DB: "East" and West" not defined at this point. -->
and
<!--- DB: Does this serve as the "East", "West", "South" and "North" definition? Or are they to be found in the Raw architecture document, and assumed already known? -->

Those terms are effectively imported from the RAW architecture as discussed in the terminology section. But I agree we can use more text.

In the terminology I added:
"
    Both architecture documents define the concept of Track in a compatible
    fashion. The RAW Architecture also defines the concepts of subTrack, Segment,
    leg, East-West, and more. This documents only builds Tracks that are DODAGs,
    meaning that there is no North/South Segment.
"
Also made the RAW architecture a normative reference, meaning a potential delay for this doc.
And reworded the text you point at in this comment:
"
2.4.5.  Track

   The concept of Track is defined in the RAW Architecture" [RAW-ARCHI]
   as a networking graph that can be followed to transport packets with
   equivalent treatment; as opposed to the definition of a path above, a
   Track is not necessarily linear.  It may contain multiple paths that
   may fork and rejoin, and may enable the RAW Packet ARQ, Replication,
   Elimination, and Overhearing (PAREO) operations.

   Figure 1 illustrates the mapping the DODAG with the generic concept
   of a Track, with the DODAG Root acting a Ingress for the Track, and
   the mapping of legs and segments, and only East-West segments,
   meaning that they are directional and progressing towards the
   destination.

             A ==> B ==> C -=- F ==> G ==> H     T1       I: Ingress
           /              \   /              \ /          E: Egress
         I                  O                 E -=- T2    T1, T2, T3:
           \              /   \              / \            External
             P ==> Q ==> R -=- T ==> U ==> V     T3         Targets

            I ==> A ==> B ==> C : a segment to targets F and O

               I --> F --> E : a leg to targets T1, T2, T3

              I, A, B, C, F, G, H, E : a path to T1, T2, T3

                    Figure 1: A Track and its Components

   This specification builds Tracks that are DODAGs oriented towards a
   Track Ingress, and the forward direction for packets (aka East-West)
   is from the Track Ingress to one of the possibly multiple Track
   Egress Nodes, which is also down the DODAG.
"

   <!-- DB: "toon" is short for cartoon, not platoon, AFAIK. I suggest to write platoon -->
Done
   <!-- DB: connectivity "to the node"'. The connectity "from the node to the internet" was actually never broken. -->
True, though it depends on L4.

   <!-- DB: what does "here" refer to? -->
This draft... reworded to
"
   this mode is preferable for use cases where internet connectivity is
   dominant, or when the Root controls the network activity in the
   nodes, which is the case of this draft.
"

   <!-- DB: "connectivity from the internet to the node". -->
Done

      <!-- DB: what does "This requirement" refer do? -->
Reworded to!
"
      To limit the need for source route headers in deep networks, one possibility
      is to store a routing state associated with the Main DODAG in select RPL
      routers down the path.
"

      <!-- DB: suffer vs. suffer from -->
Dunno so I used "experience" instead 😊

   <!-- DB: "in the encapsulation" is ambiguous. I guess you mean the outer, not the inner header?
   I suggest "the encapsulating source IP address and RPI Instance are set to the Track Ingress IP address and local RPLInstanceID, respectively." -->
Done

<!-- DB: "encapsulates the packet and sends it down the Track signaled ..."? -->
Yes, thanks 😊
   <!-- DB: I can't figure what "When Legs cross within respective Segment" means. Rephrase or illustrate with a drawing? -->

Added
"
   - meaning that there is a node that belongs to a Segment in each Tracks,
   so it may reach the destination over either leg -
"

<!-- DB: "initiates communication to a node"? -->
done

   <!-- DB: "updates" or "Updates"? See "Amends" above -->
OK, I changed to "Amends", that seems right, but I hope Michael double checks.

    <!-- DB: "conserving" seems inappropriate (a French faux-ami). Do you mean "keeping"? -->
Yes, keeping, or retaining...

  <!-- DB: please double-check that my rewriting matches your intention ;-) -->
Does! Many thanks

About
"
    When that is the case the packet is forwarded to the next hop along that segment,
    or a common neighbor with the loose next hop, in which case the packet is forwarded
    to that neighbor
"
      <!-- DB: incorrect sentence grammar. It looks like the previous edit (". When that is the case") was hasardous. -->

Yes; the sentence was hard to read, mush too long, and did not elaborate on
a preference order. The proposed text for the whole block is as follows:

"   The forwarding of a packet along a track will fail if the Track
   continuity is broken, e.g.:

   *  When forwarding along a Segment, if the next strict hop in the RIB
      for the destination of the packet is a direct Neighbor, the packet
      MUST be forwarded to that neighbor.  Otherwise the packet MUST be
      dropped.

   *  When forwarding along a Track, if the next hop in the source route
      header is a direct Neighbor, the packet MUST be forwarded to that
      neighbor; else, the packet can be forwarded in the following case:

      1.  the previous next hop has a common Neighbor that can relay to
          the next loose hop (e.g., learned through a SIO in a multicast
          DAO message, see Section 4.1.4), in which case the packet MUST
          be forwarded to that neighbor,

      2.  else the previous next hop is Ingress of a Segment of the same
          Track for which the loose next hop is a target in which case
          the Segment MUST be used,

      3.  else the previous next hop is Ingress of a (nested) Track to
          the loose next hop, in which case the (nested) Track MUST be
          used; another encapsulation takes place and the process may
          recurse.

      Otherwise the packet MUST be dropped to avoid loops; as an
      example, forwarding the packet along the main DODAG is disallowed
      since it may cause such a loop.
"

I'll make a slide on that one for the ROLL meeting.

Note that the SIO in multicast DAO reminded me of a Cisco IPR that was
not announced on the personal submission and that I'm bound to disclose.
I kicked off that procedure.

   <!-- DB: either there is a missing part at the end of this sentence, or ", representing the case where" should be removed -->
Added
"
   an IPv6-in-IPv6 encapsulation is needed (see Table 19 of [RFC9008]):
"
Again, many thanks Dominique!
  • Loading branch information
pthubert committed Jul 22, 2022
1 parent 4f0e1a1 commit f6b6488
Show file tree
Hide file tree
Showing 2 changed files with 544 additions and 519 deletions.

1 comment on commit f6b6488

@dbarthel-ol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments inserted in the commit

Please sign in to comment.