[mpls] A review of draft-ietf-mpls-mna-hdr-04

[Adrian Farrel <[e-mail]>]

Hi,

As the MNA requirements progresses and the framework draft is clearly
ready to progress, I thought now would be a good time to give this
document a thorough review (noting that the current revision expires
in just a few days, and that there has been some discussion over the
last month and a half that hasn't resulted in any updates).

I hope the comments are useful to produce a revision that the WG can
take forward. I have no shame about proposing changes to the encoding
because we know the only implementation is experimental code designed
to investigate the practicality of the solution. Indeed, I think that
some implementation and interoperability work might be necessary to
iron out the wrinkles in the spec.

Cheers,
Adrian

===

Abstract

a. It is unusual to mention I-Ds by name in the Abstract.
b. "This document addresses the MNA requirements specified in
   draft-ietf-mpls-mna-requirements." This sounds like it means "all
   the requirements", but clearly it doesn't do that.
   (See similar text in the Introduction - and the next point.)

---

Given that this document leans on draft-ietf-mpls-mna-requirements, I
think it would be appropriate for it to reference specific requirements
(they have clear numbers in the requirements draft).

I suppose that the alternative is to stop mentioning the requirements
draft, but that is a less preferred option.

---

Introduction

I suggest a little rejigging of some text:

OLD
   This document defines the syntax and semantics of network actions
   encoded within an MPLS Label Stack.  Network actions can be encoded
   with or without Ancillary Data (AD), either in or after the label
   stack.  In stack actions and ancillary data are contained in a
   Network Action Sub-Stack (NAS), which is recognized by a new base
   Special Purpose Label (bSPL) (value TBA).
NEW
   Network actions can be encoded with or without Ancillary Data (AD),
   either in or after the label stack.  This document defines the syntax
   and semantics of network actions and ancillary data encoded within an
   MPLS Label Stack.  In stack actions and ancillary data are contained
   in a Network Action Sub-Stack (NAS), which is recognized by a new
   base Special Purpose Label (bSPL).
END

---

2.2

s/are used/is used/

---

2.2

I think IHS need to reference [I-D.ietf-mpls-mna-fwk] as well as this
document because "select scope" needs a reference.

---

2.2

RLD seems to be out of sequence

---

2.2

I understand why you reference RFC 4377 for OAM, but the world has moved
on as far as the meaning of "OAM" is concerned. the RFC Editor (via the
list at https://www.rfc-editor.org/materials/abbrev.expansion.txt) 
points to RFC 6291 for the expansion. Perhaps the easiest solution here
is:
- Expand OAM as "Operations, Administration, and Maintenance"
- Reference both RFC 6291 and RFC 4377

---

3.

You have:

   the network actions that should be
   invoked for the encapsulated packet

I think that the NA is invoked for the whole packet, so you can say

   the network actions that should be
   invoked for the packet

---

3.

   Network actions and their optional Ancillary Data (AD) may be encoded
   as part of the NAS as a series of LSEs.

Why do you have "may" here? Perhaps...

   This document describes how network actions and their optional 
   ancillary data are encoded as part of an NAS as a series of LSEs.

---

4.1

   LSE Format A is a traditional LSE, as described in [RFC3032] and
   [RFC5462]

No need to call it "traditional". Just...

   LSE Format A is an as described in [RFC3032] and [RFC5462].

---

4.1

Here (or in section 5, or both) you need to say that S MUST be clear
(because you later say that a Format B LSE MUST be present). I guess
you also have to say somewhere what to do if S is 1 in a Format A LSE.

---

4.2

   *  S (1 bit) : The Bottom of Stack [RFC3032].

   *  NASL (4 bits) : The Network Action Sub-Stack Length (NASL).  The
      number of additional LSEs in the sub-stack, not including the
      leading Format A LSE and the Format B LSE.

I think you need to clarify that S MUST be 0 if NASL is non-zero, and
also to explain what to do if there is a discrepancy.

---

4.2

   NOTE: The Format A and B MUST be present while encoding Format C.
   The Format A, B and C MUST be present while encoding Format D.

I think...

   NOTE: Format A and B LSEs MUST be present when a Format C LSE is
   to be carried in the NAS. Format A, B and C LSEs MUST be present
   when one or more Format D LSEs are to be carried in the NAS.

But wait!
Suppose the first NA indicated by the opcode in the Format B LSE 
needs more a chunk of data (more than 33 bits). The rules seem to say
that you have to include a Format C LSE with a repeated opcode in order
to progress to a Format D LSE. Isn't that a waste? 

Yeah, I know, when parsing, how would you tell whether the next LSE is
Format C or D? Even if the opcode tells us this by definition, we need
a way to parse over an NA that we don't process/understand, and that 
needs the NAL.

But a potential solution lies in the fact that the NAL does not need
to be so big. If you allowed the NASL to remain as 4 bits, but the NAL
was limited to 3 bits, you could fit it in the Format B LSE (using the
three Res bits) alongside the NASL meaning that Format D LSEs can follow
a Format B LSE.

If this idea is accepted, I would also move the NAL in the Format C LSE
to be in a corresponding place.

---

4.2

   *  U (1 bit): Unknown Action Handling.  See also Section 5.4.

I think you are proposing to cluster together all opcodes with the same
unknown-handling rules into one NAS, and put the others in another NAS.

The trade-off seems to be using two Format A + B LSE pairs versus
placing a U-bit in every Format B and C LSE. Two additional LSEs seems
like a lot of U-bits.

BTW, I think there is a case where I know what the NAI means, but I 
can't or won't support it. Perhaps pedantry, but "Unsupported" may be
better than "Unknown".

Also, is there a difference between an unknown action (NAI is carried in
an opcode or a flag) and unknown opcode? E.g., in the future, opcode 127
might be used, but today's implementation will not support it? It isn't
an NAI, but...

---

4.3

Similar to the NASL issue in 4.2, there is a tension between S=1 and NAL!=0

---

4.4

   *  1 (1 bit) : The most significant bit MUST be set.  This prevents
      legacy implementations from misinterpreting this LSE as containing
      a special label.

s/special label/special purpose label if the data begins with zeros/

---

4.4

Worse! There is a tension between s=1 in a Format D LSE and the NASL and
NAL values in previous Format B and C LSEs.

What are the processing rules?

---

5.

   The TC and TTL fields of the first LSE retain their traditional
   semantics, as the penultimate node on the path may copy the TTL and
   TC fields from the preceding LSE to the next LSE on the label stack,
   overwriting the TTL and TC fields of the next LSE, as specified in
   Section 3.5 of [RFC3443].

Using "traditional" is, perhaps, wrong. Maybe "as defined in [RFC3032]
and [RFC5462]."

I think you are slightly missing instructions on how to set the two
fields (this is usually a requirement for bSPL specs). For example,
"copied from the forwarding label above/below in the stack."

---

5.

   A NAS MAY contain more Format C (Section 4.3) and Format D
   (Section 4.4) LSEs, up to the length encoded in the NASL field.

s/A NAS/An NAS/

How so, "MAY"?
If the NASL is 1, isn't it a requirement that there is a Format C LSE?
If the NASL is > 1, the requirement is that there is some combination of
Format C and Format D LSEs.

But all that might change depending on the result of my suggestion in 
Section 4.2.

---

5.2

   The data field carries opcode specific data.  This may be ancillary
   data for a network action.

What is it if it isn't AD?

---

5.2

   To preserve backward compatibility, if a network action encodes data
   that will change during packet forwarding, then that data MUST be in
   the least significant 4 bits in the data field of a Format C LSE
   (Section 4.3) or the least significant 8 bits of a Format D LSE
   (Section 4.4).  Some legacy implementations may use the label field
   in all LSEs when computing ECMP decisions and modifying the label
   field might disrupt that packet's flow.

The motivation is clear, but this is quite a limitation, isn't it?
I think that in view of this limitation you need something like...
   If a network action needs to encode more data that might need to
   change during packet forwarding it will need to use a series of
   Format D LSEs (which may be inefficient) or post-stack ancillary
   data (which is beyond the scope of this document).

---

5.3

Building on my comment on the U-bit (see section 4.2) the definition of
IHS means that we might actually need as many as six NASes to cover all
of the scopes and unknown handling options. Each NAS requires two LSEs
to get going, and that is inefficient if each NA needs a new NAS. So,
just like the question for the U-bit, that's a lot of overhead to trade
off against putting the IHS in each Format C LSE.

---

5.3

      Ingress To Egress (I2E) - The NAS MUST be processed only by the
      egress node.

Slightly unusual wording. Perhaps...

      Ingress To Egress (I2E) - The NAS MUST NOT be processed by any 
      node the except the egress node.

---

5.3

   The egress node MAY
   receive a NAS at the top of the label stack.

No problem with this statement, except that it sort of implies that 
other nodes should/must not receive an NAS at the top of the label
stack. I don't see that stated or explained in the document. Did I
miss it?

Hmmm, I see something in Section 7

   ensure the NAS MUST NOT appear at the top of the stack at any MNA
   incapable node on the path.

But no explanation of why not. I guess...

   because a node that does not recognise the MNA-Label bSPL will
   discard the packet as an unknown top label.

But even that doesn't quite address the issue with the 5.3 text 
because other, MNA-capable non-egress nodes MAY receive an NAS at the
top of the label stack. And while you also have...

   The node that receives the NAS at the top of the label stack has to
   remove it.

That doesn't say that a node must not promote the NAS to the top of the
stack. And I think s/has to/MUST/

This seems to finally surface in section 10. Perhaps forward pointers
would solve it all.

---

5.3

   An I2E scope NAS MUST be encoded after any HBH or Select scope NASes.

No, surely not! Perhaps...

   An I2E scope NAS, if present, MUST be encoded after any HBH or Select
   scope NASes.

---

5.3

   Forwarding and egress nodes should process at most a single NAS per
   scope.  If a node is to process multiple NASes, it should process
   them in the order that they appear in the label stack.

a. Why lower case "should" in both cases?
b. Why "should" not "must"? If you mean "should" you need to explain the
   choice and the alternative.
c. Surely you can have two NASes per scope, one for U=0 and one for U=1

---

5.4

If a packet with an unknown opcode is dropped, should any other action
be taken?

---

5.5

   The network actions encoded in the NAS MUST be processed as if they
   were processed in the order that they appear in the NAS

and

   If a label stack contains multiple NASes, then they
   MUST be processed as if they were processed in the order that they
   appear in the label stack

Is this saying something extra that I am missing? 

---

5.5

OLD
   NAI encoded as flags
NEW
   NAI encoded as flags (see Section 6.2)
END

---

5.6

I think it would help to put opcode values in the examples. Perhaps use
letters so there is no confusion with future IANA assignments. Thus...

    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
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                 Label                 | TC  |S|      TTL      |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | Opcode = x  |        Data             |R|IHS|S| Res |U|  NASL |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


    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
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                 Label                 | TC  |S|      TTL      |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | Opcode = y  |        Data             |R|IHS|S| Res |U|  NASL |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | Opcode = z  |        Data                   |S|  Data |  NAL  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |1|                   Data                    |S|     Data      |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |1|                   Data                    |S|     Data      |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

This helps to make it clear that the Format D AD is attached to the
Format C opcode (per my previous comments about the presence of C and D
LSEs.

---

6.

This section needs to make it clear that no new special opcodes can be
defined in future (all values that are not 0, 1, and 127 are available
for use to encode NAIs.

Alternatively, if you intend that future special purpose opcodes can be 
defined and codepoints assigned from the registry, then I think you 
need to redefine the U bit in 4.2 as "Unknown opcode handling".

---

6.2

   If there are sufficient NAI, then Format D LSEs may be used to
   encode more flags for more network actions.

This is good, but it contrary to 4.2 which has
   The Format A, B and C MUST be present while encoding Format D.

So, as I noted for 4.2, do you intend to allow the NAS to consist of
just Format A, B, and D?

---

6.2

   This opcode MAY be used with no flags set in the data field to
   signify that no operation is to be performed.  This can be used, for
   example, if the first action to be performed cannot be encoded in a
   Format B LSE.

Yes. Hmm. What does it mean for an action to be unable to be encoded in
a Format B LSE.

---

6.3 

So opcode 127 should be treated as known or unknown by an implementation
of this specification?

---

7.

   Regardless of whether packets are being forwarded based on Segment
   Routing [RFC8662], LDP [RFC5036], or RSVP-TE [RFC3209]

Packets are not forwarded based on any of those control or management 
plane mechanisms. They are forwarded based on label stacks and
programmed LFAs. Actually, I'm not sure why you need this text.

---

7.

   Each downstream node
   along the path will have Readable Label Depth (RLD)
   [I-D.ietf-mpls-mna-fwk] (including the LSEs of format B, C and D).

Given the next sentence, I think you can just say...

   Each downstream node
   along the path will have a Readable Label Depth (RLD)
   [I-D.ietf-mpls-mna-fwk].

---

7.

   then the entire NAS MUST be placed so that it is within RLD by
   the time the packet reaches the downstream MNA capable node and
   ensure the NAS MUST NOT appear at the top of the stack at any MNA
   incapable node on the path.

a. I think you have to say who does the ensuring.
b. The 2119 language doesn't match in the sentence. 

Perhaps...

   then the entire NAS MUST be placed so that it is within RLD by
   the time the packet reaches the downstream MNA capable node and
   the NAS MUST NOT appear at the top of the stack at any MNA
   incapable node on the path.

---

Section 7 tells us we must process the first copy of an NSA that is
found, but it doesn't tell us whether we must process or ignore
subsequent instances (suppose two copies are found inside the RLD).

I wonder whether the behaviour of different NAs has to cover this, or
whether there is a general rule.

---

7.

5.3 has...
      Select - Only specific nodes along the path will perform the
      action.

But here you have...

   For a NAS with Select scope, it is processed by the node that brings
   it to the top of stack and then the NAS is removed from the stack.

So, really, "only specific nodes" is actually, "only the node that 
brings the NAS to the top of the stack". But even then, what about the
node that is penultimate to the egress? I suppose that case is meant to 
use the I2E scope.

---

7.1

   While pushing additional labels on to the label stack, the MNA
   capable node SHOULD verify that the entire top-most NAS with HBH
   scope is still within the RLD of the downstream MNA capable nodes.

Why "SHOULD"? What is the option? To break the functionality of the MNA?

And what about other HBH NASes that were in the RLD but are now lost to
visibility?

---

7.1

   When an MNA capable node needs to push a new NAS with HBH scope on to
   a received packet that already has an NAS with HBH scope, it SHOULD
   copy (and merge) the network actions (including their Ancillary Data)
   from the received top-most NAS with HBH scope in the new NAS with HBH
   scope.

But it isn't always possible to merge if the setting of the U flag is
different in the two NASes.

---

7.1

s/depend/depends/

---

7.1

   The new network actions added MUST NOT conflict with the network
   actions in the received NAS with HBH scope.  The mechanism to resolve
   such conflicts depend on the network actions and can be based on
   local policy.  The MNA Capable node pushing entries MUST understand
   any network actions which it is pushing which may result in a
   conflict, and MUST resolve any conflicts between new and received
   network actions.  In the usual case of a conflict of duplicating a
   network action, the definition of the network action will generally
   give guidance on likely resolutions.

There is possibly a problem with different RLDs through the network. Is
it assumed that nodes that can reader deeper must not read any deeper 
than any other node on the LSP? That would be complex to arrange. But
here, the node adding NAs might not be able to see an NAS below its RLD
and so will add a conflicting NAS that will be seen by a later node that
has a larger RLD and can find the NAS that has the conflict.

---

8.

   The head-end node which is adding a NAS MUST make sure that the
   egress node removes the NAS.

Well, it can't, can it? All it can do is make sure that the egress node
is capable of removing the NAS.

I think you also need to be careful with the term "egress node". You are
technically right that an egress is a node that pops a label rather than 
swapping, so that the NAS doesn't reach the top of stack except at such
a node. But:
- Many people consider "egress" to refer to the full path across the 
  MPLS domain
- In SR-MPLS labels are popped along the path, and it isn't clear that
  anyone thinks of the popping nodes as egresses.

---

8.
   
   The head-end node MUST make sure that
   the NAS can be processed by the appropriate transit and egress nodes.

"Appropriate"?

---

8.

   *  Each participating node MUST signal the network actions that it
      supports.

   *  Each participating node MUST signal its Readable Label Depth.
      This will allow the head-end node to place a copy of an NAS at the
      correct stack depth.

   The above capability signaling will be added in appropriate
   protocols.  Signaling details are outside the scope of this document.

Right, but I think you are limiting your considerations to control plane
systems with head-end path computation.

What are you really trying to say?
a. The node responsible for selecting a path through the MPLS network 
   needs to know and consider the MNA capabilities and RLD of the transit
   nodes, and the MNA capabilities of the end point.
b. Information about the capabilities of the nodes may be configured,
   collected through management protocols, or distributed by control
   protocols (such as advertising by routing protocols).
c. The mechanisms by which the capabilities of the nodes is known by the
   node responsible for selecting a path through the MPLS network are 
   out of scope for this document.

I think it is OK to develop this document separate to the work on the 
distribution mechanisms. However, this protocol specification is of no
practical use without work on those mechanisms.

---

9.

s/a MPLS/an MPLS/

"a MPLS path" - why not call it an LSP?

---

9.1

This section talks about the "encapsulating node". The term is not
clearly explained.

I think you may mean the node that pushes an NAS. But you could mean 
any node that pushes any label (what the requirements draft calls an
LER).

There is a similar issue with "decapsulating node".

---

9.1

s/a NAS/an NAS/

---

9.1

   If there is an existing label stack, the encapsulating node SHOULD
   NOT change the first 20 bits of each LSE in the label stack to avoid
   ECMP path change.

s/each/any/

What does "SHOULD NOT" mean here? Under what circumstances is it OK to
make a change? How does the node decide?

Why is this an issue if the change is consistent and applied to all
packets?

---

9.1

   If the encapsulating node is also a transit node, then it MUST also
   respect transit node responsibilities.

Now the terms "encapsulating node" and "transit node" are somewhat 
muddied!

"respect the responsibilities" is something we all do. But what does 
it mean?
- what are the responsibilities?
- what does respect entail?

---

9.1

   The path computation needs to know the Maximum SID Depth (MSD) and
   Readable Label Depth (RLD) that can be imposed at the ingress node of
   a given SR path [RFC8664].  This ensures that the label stack depth
   of a computed path does not exceed the maximum number of labels
   (i.e., MSD) the node is capable of imposing and the maximum number of
   labels those could be read by the MNA processing nodes in the path.
   The MSD needs to include the MNA Sub-Stacks to be added.

I think this is text that belongs in section 8.

---

9.2

   Transit nodes SHOULD NOT change the first 20 bits in the LSEs in the
   label stack.

Same issues as in 9.1

s/the LSEs/in any of the LSEs/

What does "SHOULD NOT" mean here? Under what circumstances is it OK to
make a change? How does the node decide?

Why is this an issue if the change is consistent and applied to all
packets?

---

9.2

   Transit nodes MUST process the NASes in the label stack, respecting
   Section 5.5.

s/respecting/according to the rules set out in/

---

9.2

OLD
   A transit node MUST respect the Unknown Action Handling value encoded
   in the NAS.
NEW
   A transit node that processes an NAS and does not recognise the value
   of an opcode MUST follow the rules according to the setting of the 
   Unknown Action Handling value in the NAS as described in Section 5.4.
END

---

9.3.

Doesn't this section also apply to label popping in SR-MPLS?
Conversely, doesn't it only apply in the case of PHP?

---

9.3

s/a HBH/an HBH/

---

9.3

Isn't there a case with HBH where the egress is not MNA capable so we
want the NAS to be removed at the penultimate node?

---

In 9.4 I am lost as to what a decapsulating node is. The text makes it
pretty clear that it is not a term for the node that removes the NAS
(because that would mean you have a circular definition).

You used "egress node" in 9.3, but chose not to here.

---

10.

s/for a allocating/for allocating/

---

10.

The only allocatable codepoints from this registry are "IETF review".
That means that there is an I-D that gets IETF consensus (with the
option of early assignment). So, I think this section can say that an
I-D must be written and contain the information that you list.

---

10.

   *  Interactions: The definition of the new Network Action MUST
      specify its interaction with other currently defined Network
      Action if there are any.

s/Action if there are any/Actions if there is any/

---

10.


             If the action is unlikely to be used in combination with
   other actions and at most 20 bits of ancillary data is required, then
   an opcode may be preferred as the encoding will only consume a single
   LSE. 

Well, unless you try to squeeze it into a Format B LSE.

---

10.

   If the action is likely to be combined with other actions, then
   a flag is more likely to be optimal.

Does that need to be caveated with "...and if no ancillary data is
required"? After all, your registry in 13.3 is called "Network Action
Flags Without Ancillary Data."

---

12.

With the acceptance that AD can be rewritten in-flight, isn't there a
need to consider how the data might be secured? Or perhaps to note 
that it isn't secured or authenticated and so critical network functions
should either not rely on the data or should be aware of the risks and 
use other means to verify the security of the whole network.

---

13.

Have you thought carefully about the use of Private Use NAIs? They only
have local meaning and are not communicated between devices. So they 
only work in a network where the node that sets the NAI knows exactly
which nodes will encounter the NAI and knows exactly how they will 
process it. That sounds pretty risky on the open Internet (although
maybe it works for I2E and possibly for Select). 

If you are really certain that this is a good idea (rather than, for
example, having a vendor-specific opcode and an OID and subcode in the
AD) then I think you need to write a section about the use of Private
Use NAIs with warnings and possibly a suggestion about when it is
appropriate to use them. (I do note the paragraph in section 12, but it
is not easily linked to the use of Private NAIs.)

---

13.3

   The registration procedure for this
   registry is "IETF Review".

Except the table lists other registration procedures.

---

13.3

   Bit Position refers to the position relative to the most significant
   bit in LSE Format B or C Data fields and any subsequent Format D
   LSEs.  Bit Position 0 is the most significant bit a LSE Format B or C
   Data field.  Bit Position 20 is the most significant bit in the first
   LSE Format D Data field.  There are 20 bits available in LSE Format C
   and 30 available in LSE Format D.  There are at most 15 Format D LSEs
   per opcode, so there are at most 20 + 15 * 30 = 470 bit positions.
   The Bit Position is an integer with value 0-469.

This seems to clash with the text in 4.2 where you have

   NOTE: The Format A and B MUST be present while encoding Format C.
   The Format A, B and C MUST be present while encoding Format D.

In other words, you can only have a Format D LSE after both a B and a
C. So the bit counts seem to be off.

See also 14.1.1 where I have to ask:
- why waste the data from the Format B LSE?
- why waste the opcode field in the 3rd LSE when you could go straight 
  to a Format D LSE (skipping Format C)?

---

13.4

   The registration procedure for this registry is "IETF
   Review".

Except the table lists other registration procedures.

---

Contributors

You may want to check the coordinates of all of your contributors in 
order that they are able to respond to IPR polls, etc.

---