Re: [Detnet] Last Call: <draft-ietf-detnet-yang-18.txt> (Deterministic Networking (DetNet) YANG Model) to Proposed Standard

[Florian Kauer <[e-mail]>]

Hi,
sorry for being this late, but there are several things that still confuse me when I try to apply the YANG model in a TSN over IP over TSN scenario (i.e. using DetNet to connect several TSNs over IP tunnels in the industrial automation or professional audio context).

It is likely that my confusions are only based on misunderstandings and missing background knowledge (and that's I why I did not speak up before and tried to do more research first), but I still would like to raise them now and let others decide if they are valid concerns or not. In both cases, I still would love to get support clarifying these points.

I also fixed some things that look like typos to me, but take them with a grain of salt since I am not a native speaker.

Greetings,
Florian

--- draft-ietf-detnet-yang-18.txt       2023-07-10 21:24:35.000000000 +0200
+++ draft-ietf-detnet-yang-18-fk-comments.txt   2023-12-06 13:49:42.692711093 +0100
@@ -192,14 +192,14 @@
    |  r  |Forwarding S-L   |Forwarding S-L   | Forwarding S-L|
    +-----+-----------------+-----------------+---------------+
 
-                   Figure 1: Detnet Layers and Node Types
+                   Figure 1: DetNet Layers and Node Types
 
    All of the layers have ingress/incoming and egress/outgoing
    operations, but any instance may be configured as only
    unidirectional.  Ingress refers to any DetNet layer where a DetNet
    context is applied.  Ingress allows functions such as switching,
    aggregation and encapsulation.  Likewise, egress refers to any DetNet
-   layer where a Detnet context is removed.  Egress allows functions
+   layer where a DetNet context is removed.  Egress allows functions
    such as switching, disaggregation and decapsulation.  This means that
    each unidirectional flow identifier configuration is programmed
    starting at the ingress and flow status is reported at ingress on
@@ -454,7 +454,7 @@
 
    This YANG model imports typedefs from [RFC6991], [RFC8519],
    [RFC8294], [RFC8343], [IEEE8021Q], and [IEEE8021QCX].  This YANG
-   model also has the folowing references to RFCs that are not in the
+   model also has the following references to RFCs that are not in the
    document text body [RFC0791], [RFC4303], [RFC8349], [RFC8938],
    [RFC8960], [RFC8964], and [RFC8200].
 
@@ -583,7 +583,7 @@
      identity none {
        base app-status;
        description
-         "This Application has no status. This identity is
+         "This application has no status. This identity is
           expected when the configuration is incomplete.";
        reference
          "RFC 9016 Section 5.8";
@@ -600,7 +600,7 @@
      identity failed {
        base app-status;
        description
-         "Application ingres/egress failed.";
+         "Application ingress/egress failed.";
        reference
          "RFC 9016 Section 5.8";
      }
@@ -608,7 +608,7 @@
      identity out-of-service {
        base app-status;
        description
-         "Application Administratively blocked.";
+         "Application administratively blocked.";
        reference
 
 
@@ -624,7 +624,7 @@
      identity partial-failed {
        base app-status;
        description
-         "This is an Application with one or more Egress ready, and one
+         "This is an application with one or more Egress ready, and one
           or more Egress failed.  The DetNet flow can be used if the
           Ingress is Ready.";
        reference
@@ -639,7 +639,7 @@
             + "/dnet:name";
        }
        description
-         "This is an Application Reference.";
+         "This is an application reference.";
      }
 
      typedef service-sub-layer-ref {
@@ -650,7 +650,7 @@
             + "/dnet:name";
        }
        description
-         "This is a Service sub-layer Reference.";
+         "This is a service sub-layer reference.";
      }
 
      typedef forwarding-sub-layer-ref {
@@ -661,7 +661,7 @@
             + "/dnet:name";
        }
        description
-         "This is a Forwarding sub-layer Reference.";
+         "This is a forwarding sub-layer reference.";
      }
 
      typedef traffic-profile-ref {
@@ -679,7 +679,7 @@
             + "/dnet:name";
        }
        description
-         "This is a Traffic Profile Reference.";
+         "This is a traffic profile reference.";
      }
 
      typedef ipsec-spi {
@@ -1113,6 +1113,79 @@
          description
            "The Application flow type choices.";
          container tsn-app-flow {
+---
+fk: In case of a tsn-app-flow, how does the actual encapsulation take place?
+
+For reference, lets look at a few cases and how I have understood them:
+
+1. ip-app-flow, nothing MPLS related in service or forwarding sub-layers:
+   -> Just transmit unmodified packet with new L2 header.
++---------------+      +----------+
+| Payload       |      | Payload  |
++---------------+      +----------+
+|   UDP         |      |   UDP    |
++---------------+  ->  +----------+
+|   IP          |      |   IP     |
++---------------+      +----------+
+| L2 (original) |      | L2 (new) |
++---------------+      +----------+
+
+2. ip-app-flow, but MPLS label is added:
+   -> Squeeze in MPLS header.
++---------------+      +----------+
+| Payload       |      | Payload  |
++---------------+      +----------+
+|   UDP         |      |   UDP    |
++---------------+  ->  +----------+
+|   IP          |      |   IP     |
++---------------+      +----------+
+| L2 (original) |      |   MPLS   |
++---------------+      +----------+
+                       | L2 (new) |
+                       +----------+
+
+3. mpls-app-flow:
+   -> Only optionally modify MPLS header
++-----------------+      +------------+
+| Payload         |      | Payload    |
++-----------------+      +------------+
+|   UDP           |      |   UDP      |
++-----------------+  ->  +------------+
+|   IP            |      |   IP       |
++-----------------+      +------------+
+| MPLS (original) |      | MPLS (new) |
++-----------------+      +------------+
+| L2 (original)   |      | L2 (new)   |
++-----------------+      +------------+
+
+4. tsn-app-flow:
+   -> In order to be able to reconstruct the full original L2 packet, we need
+      to retain the L2 header. So we need something like:
+
++---------------+      +-----------------+
+| Payload       |      |    Payload      |
++---------------+  ->  +-----------------+
+| L2 (Original) |      |  L2 (Original)  |
++---------------+      +-----------------+
+                       |      ???        |
+                       +-----------------+
+                       |   L2 (new)      |
+                       +-----------------+
+
+I know there are several different ways to realize this, including just
+transmitting the L2 packet over MPLS (apparently not IETF standardized?)
+or several others like L2TPv3 (RFC3931) or VXLAN (RFC 7348) if no MPLS labels are
+to be added (like in case 1 above).
+
+I would expect the YANG model (either this or a linked one) to somehow specify which
+of these options shall actually be applied. Otherwise, two DetNet routers might
+not be able to communicate with each other, because one might use L2TPv3 and
+the other one VXLAN. And also for example in the case of VXLAN,
+we would need the option to specify a VXLAN Network Identifier.
+
+What do you think about that?
+
+---
            uses l2-header;
 
 
@@ -1143,7 +1216,7 @@
          "detnet-flow identification.";
        choice detnet-flow-type {
          description
-           "The Detnet flow type choices.";
+           "The DetNet flow type choices.";
          case ip-detnet-flow {
            uses ip-flow-id;
          }
@@ -1254,7 +1327,7 @@
              case mpls {
                uses rt-types:mpls-label-stack;
                description
-                 "The MPLS Label stack next hop case.";
+                 "The MPLS label stack next hop case.";
              }
            }
          }
@@ -1425,6 +1498,9 @@
            type string;
            description
              "An Aggregation group ID.";
+---
+fk: I would expect "The name of the traffic profile." here as it is similarly used for most other names.
+---
          }
          container traffic-requirements {
            description
@@ -1708,10 +1784,20 @@
            container egress {
              description
                "Route's next-hop attribute.";
+---
+fk: This description does not seem right at this place!?
+---
              uses data-flow-spec;
              choice application-type {
                description
                  "This is the application type choices.";
+---
+fk: This part is quite confusing to me, especially since it does not directly mirror the ingress since.
+I guess one reason for that is there could be multiple interfaces that could be selected based on the packet headers?
+But for the same reason you could also have multiple ingress interfaces.
+Also the "ethernet" application type is confusing. I would expect that it should mirror the "tsn-app-flow",
+but then it mentions "TSN unaware traffic"?
+---
                container ethernet {
                  description
                    "This is TSN unaware traffic that maps to an
@@ -1860,6 +1946,9 @@
                container forwarding-sub-layer {
                  description
                    "This service sub-layer is sent to the forwarding
+--
+fk: "sent" or "sending"?
+--
                     sub-layers of the lower layer for DetNet service
                     forwarding or service-to-forwarding aggregation at
                     the ingress node or relay node.  When the operation
@@ -1978,7 +2067,11 @@
          list sub-layer {
            key "name";
            description
-             "The List is one or more DetNet Traffic types.";
+             "The list is one or more DetNet traffic types.";
+---
+fk: I don't get the meaning of this sentence in this context!?
+This is the forwarding sub-layer name and not a traffic type!?
+---
            leaf name {
              type string;
              description
@@ -1996,6 +2089,9 @@
                 impose-and-forward, pop-and-forward,
                 pop-impose-and-forward, forward, pop-and-lookup.";
            }
+---
+fk: Since these are MPLS forwarding-operations, do we still use e.g. "forwarding" when using IP instead of MPLS or just leave this out?
+---
            container incoming {
              description
                "The DetNet forwarding sub-layer incoming
@@ -2206,9 +2302,18 @@
    be considered sensitive.
 
    /detnet/traffic-profile/member-app: This links traffic profiles to
-   applications, o this also could be considered more sensitive.  The
+   applications, so this also could be considered more sensitive.  The
    traffic profiles liked to service sub-layer and forwarding sub-layer
    are less sensitive.
+---
+fk: I am not so sure if that is the case. Being able to change a traffic
+profile could mean that you can for example change a DetNet flow that
+is specified as lossless into one that is lossy. Or one with a very small
+latency into one with a huge latency.
+If the DetNet flow carries data of a time-critical industrial control loop,
+I wouldn't be too happy if an attacker can occasionally crash my system
+by dropping packets or inducing additional latencies.
+---
 
    /detnet/service/sub-layer/incoming/app-flow: This links applications
    to services.
@@ -2608,6 +2713,14 @@
         |     +--rw operation?            operation
         |     +--rw incoming
         |     |  +--rw (incoming)?
+---
+fk: One thing I raised before, but it either my comment got lost or i missed the response:
+Why is there no incoming forwarding sub-layer for the service sub-layer?
+An app-flow has outgoing service-sub-layer and incoming service-sub-layer,
+a forwarding sub-layer has incoming service-sub-layer and outgoing service-sub-layer,
+a service sub-layer has incoming app-flow and outgoing app-flow
+and it has outgoing forwarding sub-layer, but NO incoming forwarding sub-layer.
+---
         |     |     +--:(app-flow)
         |     |     |  +--rw app-flow
         |     |     |     +--rw flow*   app-flow-ref
@@ -2976,7 +3089,7 @@
       by the configuration.
 
    The following are examples of aggregation and disaggregation at
-   various points in Detnet.  Figures are provided in the PDF and HTML
+   various points in DetNet.  Figures are provided in the PDF and HTML
    version of this document.
 
 B.1.  Example A-1 JSON Configuration/Operational