Re: [Detnet] Martin Vigoureux's No Objection on draft-ietf-detnet-mpls-11: (with COMMENT)

[Balázs Varga A <[e-mail]>]

Hi Martin,

Many thanks for the review. My reactions/comments inline.

Thanks & Cheers
Bala'zs

-----Original Message-----
From: Martin Vigoureux via Datatracker <[e-mail]> 
Sent: Thursday, September 10, 2020 12:48 AM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; Ethan Grossman <[e-mail]>; [e-mail]
Subject: Martin Vigoureux's No Objection on draft-ietf-detnet-mpls-11: (with COMMENT)

Martin Vigoureux has entered the following ballot position for
draft-ietf-detnet-mpls-11: No Objection

When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-detnet-mpls/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Hi,

thank you for this document. I have few COMMENTs.

I support Magnus' DISCUSS. I think implementing replication/duplicate elimination is non trivial at layer "2.5" and implementers would, I guess, benefit from extra clarifications. I'm not an expert in that field but I sense some challenges around the size of the tables needed to keep track of the received S/N, associated expiry timers, race conditions, ...

<Bala'zs> Implementers may check IEEE 802.1CB-2017 defining possible replication/elimination implementation.

   A DetNet control word (d-CW) conforms to the Generic PW MPLS Control
   Word (PWMCW) defined in [RFC4385].
This kind of suggests that there is a *single* format for d-CW which is:
      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |0 0 0 0|                Sequence Number                        |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

but then
   As shown in Figure 3 of [RFC5085] when the first nibble of the d-CW
   is 0x0 the payload following the d-CW is normal user data.  However,
   when the first nibble of the d-CW is 0X1, the payload that follows
   the d-CW is an OAM payload with the OAM type indicated by the value
   in the d-CW Channel Type field.
suggests that, for OAM, d-CW format becomes
      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |0 0 0 1|Version|   Reserved    |         Channel Type          |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Is that a d-CW with a different format or is that something else (an OAM ACH) as this text suggests:
   To support service sub-layer level OAM, an OAM Associated Channel
   Header (ACH) [RFC4385] together with a Generic Associated Channel
   Label (GAL) [RFC5586] MAY be used in place of a d-CW.

<Bala'zs> Details of OAM are out-of-scope in this document. We have a dedicated document for it
https://datatracker.ietf.org/doc/draft-ietf-detnet-mpls-oam/

   The sequence number length MUST be provisioned on a per Detnet
   service basis via configuration, i.e., via the controller plane
   described in [I-D.ietf-detnet-data-plane-framework].
Should there be a requirement that the sequence number length be the same on both ends of the service?

<Bala'zs> Sequence number length is a per service parameter, so it is the same at each point of the 
Service, including endpoints.

   In some PREOF topologies, the node performing
   replication will be sending to multiple nodes performing PEF or POF,
   and may need to send different S-Label values for the different
   member flows for the same DetNet service.

   When replication is
   configured, the same app-flow data will be sent over multiple
   outgoing DetNet member flows using forwarding sub-layer LSPs.  An
   S-Label value MUST be configured per outgoing member flow.
It's not clear to me whether that last sentence means "a *different* S-Label value per ..." but I kind of sense a conflict between these two paragraphs. Am I wrong?

<Bala'zs> I think the text is fine. First says that egress member flows may have different S-Label values. 
Second says that the S-Label value have to be configured for each egress member flow.

   zero or more F-Labels and optionally, the incoming interface,
   proceeding the S-Label MUST be used
Maybe I'm not reading this correctly, but if using the incoming interface is optional, and using zero F-labels is allowed, then it seems to me that this sentence allows for "nothing MUST be used"

<Bala'zs> Sentences continues as "... MUST be used together with the S-Label". 
So, S-Label is _always_ used to identify the DetNet service. Zero F-Labels means 
that only the S-Label is used.

Also, right after you say:
   The incoming interface MAY also be used ...
So I wonder if mentioning the incoming interface in the previous sentence is necessary.

<Bala'zs> Yes, same comment received recently. This sentences is fixed in the next version.

   The edge and relay node internal procedures related to PREOF are
   implementation specific.  The order of a packet elimination or
   replication is out of scope in this specification.

   For example, a relay node that
   performs both PEF and PRF first performs PEF on incoming packets to
   create a compound flow.  It then performs PRF and copies the app-flow
   data and the d-CW into packets for each outgoing DetNet member flow.
Again, I kind of sense a conflict between these two paragraphs (first says order is OOS, second gives an order). Am I wrong?

<Bala'zs> Second paragraph is just an example. Many other examples may exists,
but  they are out of scope.

   RFC7322 limits the number of authors listed on the front page of a
   draft to a maximum of 5.  The editor wishes to thank and acknowledge
   the follow author for contributing text to this draft.
I see 6 authors on front page. I don't have any specific opinion on that but there seems to be a mismatch between that text and the current number of authors on front page.

s/MAY required that PEF/MAY require that PEF/ s/F-Labels are supported the DetNet forwarding sub-layer./F-Labels are supported by the DetNet forwarding sub-layer./ s/Valid values included/Valid values include/ s/traffic paramters/traffic parameters/ s/the follow author/the following author/

<Bala'zs> Thanks, I will fix them.