Subject: SRH TLV Type change

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Wed, 5 Jun 2019 18:06:08 +0000

In revision 18/19, PAD0 and PADN were moved to TLV Type 0 and 1 (section 8.2)
I was reminded that type 1 conflicted with previous versions of the draft and implementations.
In revision 20 I plan to move the PADN type back to 4 (its value from previous revisions/implementations) and add text to highlight the reservations for previous versions and implementations.

Darren

---

From: Tom Herbert <[email-masked]>
Date: Wed, 5 Jun 2019 11:46:32 -0700

> In revision 20 I plan to move the PADN type back to 4 (its value from previous revisions/implementations) and add text to highlight the reservations for previous versions and implementations.

Darren,

Please don't do this. If PAD1 is type 0 and PADN is type 1 then that
is consistent with HBH and DO. This way we can use a common
implementation of the parsing loop (type and length byte definitions
are also common). Besides, in the SRH draft only one implementation
(Linux) is reported to have implemented TLVs and that implementation
does not support padding yet.

Tom

>
> Darren
> --------------------------------------------------------------------
> IETF IPv6 working group mailing list
> [email-masked]
> Administrative Requests: https://www.ietf.org/mailman/listinfo/ipv6
> --------------------------------------------------------------------

---

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Wed, 5 Jun 2019 21:20:37 +0000

Tom, I believe we had this conversation long ago.
The number space is disjoint for option and SRH TLVs so the numeric values for PAD TLV types for option vs SRH TLVs is not relevant.

On the other hand there are macros used by implementations that have had fixed values for 2 years.

Either way an implementation’s equivalent of the Linux ip6_parse_tlv needs to be duplicated for SRH TLVs or simply modified to support the two number spaces.

Darren

> On Jun 5, 2019, at 2:46 PM, Tom Herbert <[email-masked]> wrote:
>

---

From: Tom Herbert <[email-masked]>
Date: Wed, 5 Jun 2019 14:42:20 -0700

> The number space is disjoint for option and SRH TLVs so the numeric values for PAD TLV types for option vs SRH TLVs is not relevant.

PAD1 and PADN are different. Even if the namespace for all types are
different, we can still leverage common values here.

> On the other hand there are macros used by implementations that have had fixed values for 2 years.
And exactly what implementations are those?

> Either way an implementation’s equivalent of the Linux ip6_parse_tlv needs to be duplicated for SRH TLVs or simply modified to support the two number spaces.

Yes, but if you look closely you'll there are only two values and the
default case in the switch statement. Those two explictly cases are
exactly PAD1 and PADN. Other cases are handled by table lookup which
is easy enough to abstract out to allow different number spaces. I
have the implementation that does this and will post shortly.

Tom

---

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Thu, 6 Jun 2019 18:53:21 +0000

Hi Tom, see inline.

> On Jun 5, 2019, at 5:42 PM, Tom Herbert <[email-masked]> wrote:
> 

Well the Linux kernel, so any applications that use those macros from the kernel.  The code was out for 2 years and changing the content changes those applications...
 
> have the implementation that does this and will post shortly.

I have looked closely, and the change is trivial to avoid breaking user land APIs and applications (something not accepted by the maintainers).

switch (nh[off]) {
    ...
case IPV6_TLV_PAD1:
+ case SR6_TLV_PAD0:
    ...
    break;
case IPV6_TLV_PADN:
+ case SR6_TLV_PADDING:
    ...
    break;
...
}

Darren

---

From: Tom Herbert <[email-masked]>
Date: Thu, 6 Jun 2019 12:55:32 -0700

Darren,

Linux kernel doesn't implement SRH padding. It implements precisely
one TLV which is HMAC. It assumes HMAC is first and that the HMAC flag
indicates the presence of the HMAC TLV. The HMAC flag has been
removed, and so the current implementation is out of conformance with
the latest draft. If backwards compatibility with previous versions of
the draft is the goal, which it's not since this is an Internet Draft
and not a standard after all, then it's the HMAC flag that needs to be
restored not the type values of padding TLVs. I'd also point out that
the values for PAD0 and PADN were respectively 128 and 129 in versions
12 through 17 of the draft, so if someone has implemented those drafts
then changing then reverting the values to something earlier don't
help them.

In any case, maintaining uniformity with an Internet standard is as
good an argument as any, so I am still opposed to the proposed
changed. I do have one nit though. The draft refers to single byte pad
as PAD0, however to the rest of the world this is referred to as PAD1
(see RFC8200). Please fix the terminology to be consistent.

Tom

---

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Thu, 6 Jun 2019 20:25:44 +0000

Tom, I’m at a bit of a loss. Robust coding practices require the two lines of code I provided below, and I see no reason to jump on a previous definition of the ingress_tlv since we don’t know if it is in use by any implementation or not.

Darren

On Jun 6, 2019, at 3:55 PM, Tom Herbert <[email-masked]<mailto:[email-masked]>> wrote:

On Thu, Jun 6, 2019 at 11:53 AM Darren Dukes (ddukes) <[email-masked]<mailto:[email-masked]>> wrote:

Hi Tom, see inline.

On Jun 5, 2019, at 5:42 PM, Tom Herbert <[email-masked]<mailto:[email-masked]>> wrote:

On Wed, Jun 5, 2019 at 2:20 PM Darren Dukes (ddukes) <[email-masked]<mailto:[email-masked]>> wrote:

Darren,

Tom

switch (nh[off]) {
   ...
case IPV6_TLV_PAD1:
+ case SR6_TLV_PAD0:
   ...
   break;
case IPV6_TLV_PADN:
+ case SR6_TLV_PADDING:
   ...
   break;
...
}

Darren

Tom

Darren

On Jun 5, 2019, at 2:46 PM, Tom Herbert <[email-masked]<mailto:[email-masked]>> wrote:

On Wed, Jun 5, 2019 at 11:06 AM Darren Dukes (ddukes) <[email-masked]<mailto:[email-masked]>> wrote:

Darren,

Tom

Darren
--------------------------------------------------------------------
IETF IPv6 working group mailing list
[email-masked]<mailto:[email-masked]>
Administrative Requests: https://www.ietf.org/mailman/listinfo/ipv6
--------------------------------------------------------------------

---

From: Bob Hinden <[email-masked]>
Date: Thu, 6 Jun 2019 13:41:02 -0700

Hi,

> On Jun 6, 2019, at 12:55 PM, Tom Herbert <[email-masked]> wrote:
> 

I don’t see this a big issue as long as it is well specified in the IANA considerations section.   

> (see RFC8200). Please fix the terminology to be consistent.

This makes sense, that is, s/PAD0/PAD1/

Bob

---

From: Tom Herbert <[email-masked]>
Date: Thu, 6 Jun 2019 14:17:04 -0700

On Thu, Jun 6, 2019 at 1:25 PM Darren Dukes (ddukes) <[email-masked]> wrote:
>
> Tom, I’m at a bit of a loss. Robust coding practices require the two lines of code I provided below, and I see no

That is severely broken. In any case, if you want to have a meaningful
discussion of implemenation then please produce running code.

> reason to jump on a previous definition of the ingress_tlv since we don’t know if it is in use by any implementation or not.

Neither do we know that someone hasn't implemented PAD1 and PADN as
type values 0 and 1 as the current draft clearly states. In any case,
the "jumping on the previous defintion of the ingress_tlv" has already
occured. Frankly, this dicussion should have happened before version
-18. The values are set to 0 and 1 in the draft, if its changed again
(for the third time) that presents as much risk as it does to
(theoretical) implementations.

Tom

>
> Darren
>
> On Jun 6, 2019, at 3:55 PM, Tom Herbert <[email-masked]> wrote:
>

---

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Thu, 6 Jun 2019 22:28:55 +0000

I made the s/PAD0/PAD1/ change in the text, thanks.
Fair enough, on the double 0 case (see code below) the point remains the same.
The value for INGRESS_TLV hasn’t changed in the kernel, yes we moved the draft specification of the PADN type to 1 and conflicted with INGRESS_TLV but the code stayed the same, so thus the return to the current implementation.

This would be the IANA consideration text:

    Assigned      Description               Reference
     Value
   -----------------------------------------------------
      0           Pad1 TLV                  This document
      1           Reserved                  This document
      2           Reserved                  This document
      3           Reserved for Opaque TLV   draft-xuclad-spring-sr-service-programming
      4           PadN TLV                  This document
      5           HMAC TLV                  This document
      6           Reserved for NSH TLV      draft-xuclad-spring-sr-service-programming
      124-126     Experimentation and Test  This document
      127         Reserved                  This document
      252-254     Experimentation and Test  This document
      255         Reserved                  This document
 
Values 1-2 were defined in draft versions of this specification and
are Reserved for backwards compatibility with early implementations.  
Values 127 and 255 are Reserved to allow for expansion of the Type
field in future specifications if needed.

Thanks
  Darren

> On Jun 6, 2019, at 5:17 PM, Tom Herbert <[email-masked]> wrote:
> 

+ #if (IPV6_TLV_PAD1!=SRV6_TLV_PAD0)
+ #error SRV6 and Option TLV PAD1 TYPE mismatch
+ #endif
...
switch (nh[off]) {
  ...
case: IPV6_TLV_PAD1:
  ...
  break;
case IPV6_TLV_PADN:
+ case SR6_TLV_PADDING:
  ...
  break;
  ...
}

---

From: "Joel M. Halpern" <[email-masked]>
Date: Thu, 6 Jun 2019 18:37:05 -0400

Unless you want to create a normative dependency, blocking this document 
until the service programming drafts are adopted, completed, and 
published, I would recommend against including those code points (3 and 
6) in this IANA assignment.  Leave them as "Reserved".

Yours,
Joel

---

From: Tom Herbert <[email-masked]>
Date: Thu, 6 Jun 2019 15:51:01 -0700

>     Assigned      Description               Reference
And what happens next year when somebody creates a new TLV and
requests type 1? We're right back to having a conflict. If you want to
prevent conflicts and be backwards compatible with any possible
hypothetical implementation that may have ever used any value even
defined in just one draft version, you need to allocate each value and
mark it as obsoleted. And this isn't just TLV types, it's also the
various flags that have ever been defined an subsequently obsoleted in
later versions of the draft.

Tom

> Th

---

From: Bob Hinden <[email-masked]>
Date: Thu, 6 Jun 2019 16:30:04 -0700

Joel,

> On Jun 6, 2019, at 3:37 PM, Joel M. Halpern <[email-masked]> wrote:
> 
> Unless you want to create a normative dependency, blocking this document until the service programming drafts are adopted, completed, and published, I would recommend against including those code points (3 and 6) in this IANA assignment.  Leave them as "Reserved”.

That’s why they say “Reserved for Opaque TLV”, etc.   The request to IANA is to hold them for this purpose.  I don’t see this as normative for this document.  If this becomes a problem later in the process, it can be changed to just “Reserved”.

Bob

---

From: Bob Hinden <[email-masked]>
Date: Thu, 6 Jun 2019 16:34:36 -0700

Tom,

> On Jun 6, 2019, at 3:51 PM, Tom Herbert <[email-masked]> wrote:
> 

This to create an IANA registry.  If someone made a request for a value marked reserved the request would be rejected.  The only values available for assignment are the values not listed in the registry, that is 7-123 and 128-251.

Bob

---

From: Joel Halpern Direct <[email-masked]>
Date: Thu, 6 Jun 2019 20:48:49 -0400

I suggest just changing it to reserved and removing the reference. 
Based on my experience with IANA and the RPC, if the reference is there 
then it will become blocking.  This is because:
1) IANA references are required to be stable
2) RPC will generally consider IANA references as normative beause they 
are needed to understand the entry.

Yours,
Joel

---

From: <[email-masked]>
Date: Fri, 7 Jun 2019 05:45:19 +0000

Hi all, 

I agreed with Joel about removing the non-stable references. 

BTW, these two should be left as "Unassigned" (not "Reserved"). 

Cheers,
Med

> -----Message d'origine-----
> De : ipv6 [mailto:[email-masked]] De la part de Joel Halpern Direct
> Envoyé : vendredi 7 juin 2019 02:49
> À : Bob Hinden
> Cc : IPv6 List
> Objet : Re: SRH TLV Type change
> 
> >> Unless you want to create a normative dependency, blocking this
> document until the service programming drafts are adopted, completed, and
> published, I would recommend against including those code points (3 and 6)
> in this IANA assignment.  Leave them as "Reserved”.
> >
> > That’s why they say “Reserved for Opaque TLV”, etc.   The request to
> IANA is to hold them for this purpose.  I don’t see this as normative for
> this document.  If this becomes a problem later in the process, it can be
> changed to just “Reserved”.
> >
> > Bob
> >
> >
> >>
> >> Yours,
> >> Joel
> >>
> >>> The value for INGRESS_TLV hasn’t changed in the kernel, yes we moved
> the draft specification of the PADN type to 1 and conflicted with
> INGRESS_TLV but the code stayed the same, so thus the return to the
> current implementation.
> >>>>> Tom, I’m at a bit of a loss. Robust coding practices require the two
> lines of code I provided below, and I see no
> >>>>
> >>>>> reason to jump on a previous definition of the ingress_tlv since we
> don’t know if it is in use by any implementation or not.
> >>>>
> >>>>> The number space is disjoint for option and SRH TLVs so the numeric
> values for PAD TLV types for option vs SRH TLVs is not relevant.
> >>>>>
> >>>>>
> >>>>> Well the Linux kernel, so any applications that use those macros
> from the kernel.  The code was out for 2 years and changing the content
> changes those applications...
> >>>>>
> >>>>>
> >>>>> Darren,
> >>>>>
> >>>>> Either way an implementation’s equivalent of the Linux ip6_parse_tlv
> needs to be duplicated for SRH TLVs or simply modified to support the two
> number spaces.
> >>>>>
> >>>>>
> >>>>> I have looked closely, and the change is trivial to avoid breaking
> user land APIs and applications (something not accepted by the
> maintainers).
> >>>>>
> >>>>> switch (nh[off]) {
> >>>>>    ...
> >>>>> case IPV6_TLV_PAD1:
> >>>>> + case SR6_TLV_PAD0:
> >>>>>    ...
> >>>>>    break;
> >>>>> case IPV6_TLV_PADN:
> >>>>> + case SR6_TLV_PADDING:
> >>>>>    ...
> >>>>>    break;
> >>>>> ...
> >>>>> }
> >>>>>
> >>>>> Darren
> >>>>>
> >>>>>
> >>>>> Tom
> >>>>>
> >>>>>
> >>>>> Darren
> >>>>>
> >>>>> In revision 20 I plan to move the PADN type back to 4 (its value
> from previous revisions/implementations) and add text to highlight the
> reservations for previous versions and implementations.
> >>>>>
> >>>>>
> >>>>> Darren,
> >>>>>

---

From: "Darren Dukes (ddukes)" <[email-masked]>
Date: Fri, 7 Jun 2019 19:47:19 +0000

Hi Tom, with expert review we get to say what value is used by which drafts, so I don’t think another assignment for 1 is a problem.

Same for Flags, but three of the four previous flags in the Linux kernel uapi have no draft describing their use.
That’s a lot.
O-flag is in draft-ali-6man-spring-srv6-oam but eventually I expect we’ll need to stomp on some of the other flag definitions out of necessity.

Darren

On Jun 6, 2019, at 6:51 PM, Tom Herbert <[email-masked]<mailto:[email-masked]>> wrote:

On Thu, Jun 6, 2019 at 3:29 PM Darren Dukes (ddukes) <[email-masked]<mailto:[email-masked]>> wrote:

Tom

Th
 Darren

On Jun 6, 2019, at 5:17 PM, Tom Herbert <[email-masked]<mailto:[email-masked]>> wrote:

On Thu, Jun 6, 2019 at 1:25 PM Darren Dukes (ddukes) <[email-masked]<mailto:[email-masked]>> wrote:

Tom

Darren

Hi Tom, see inline.

Darren,

Tom

switch (nh[off]) {
 ...
case IPV6_TLV_PAD1:
+ case SR6_TLV_PAD0:
 ...
 break;
case IPV6_TLV_PADN:
+ case SR6_TLV_PADDING:
 ...
 break;
...
}

Darren

Tom

Darren

Darren,

Tom

Darren

---

From: Tom Herbert <[email-masked]>
Date: Fri, 7 Jun 2019 13:19:57 -0700

> Same for Flags, but three of the four previous flags in the Linux kernel uapi have no draft describing their use.

75% in fact :-) But what about that fourth flag which, unlike
ingress_tlv, has actually been implemented and deployed?

FWIW, I posted patches to Linux to bring segment routing
implementation into conformance with version -19. This is includes
deprecating TLVs and flags as well as calling ip6_parse_tlv for SR
TLVs. See https://marc.info/?l=linux-netdev&m=155993373101865&w=2

Tom

Tom

> On Jun 6, 2019, at 6:51 PM, Tom Herbert <[email-masked]> wrote:
>