Subject: Re: [radext] draft-ietf-radext-ip-port-radius-ext

From: Paul Aitken <[email-masked]>
Date: Tue, 11 Oct 2016 21:43:25 +0100

Authors I've been waiting 2 months, yet no reply?

P.

On 08/11/16 17:24, Paul Aitken wrote:
> Dear Authors, some questions came to mind while reviewing section 3.2 
> of your draft:
>
>
> Q1: Why define a new TLV-Type 1..11 simply to encode an IPFIX 
> Information Element?
>
> Since there's a 1:1 mapping between the TLV-Type and IPFIX Element ID, 
> the scheme you propose would be more extensible if the IPFIX Element 
> ID was used instead of the TLV-Type since it would require one less 
> registry. True, one additional byte would be required in the encoding. 
> However, the advantage is that in future anyone could encode a new TLV 
> by using the relevant IPFIX ID, without having to also define a new 
> TLV-Type for it.
>
>
> Q2: In each TLV, the Length field is redundant since it can always be 
> determined from the TLV-Type: it's always 6, with special cases for 
> TLV-Types 5 (Length is 18) and 11 (Length could be determined by a 
> NULL terminator on the string). This suggests that the length is not 
> required in the protocol, so why not remove it entirely?
>
>
> However, many of the descriptions say "contains the data ... right 
> justified, and the unused bits in this field MUST be set to zero. "
>
> Q3: Wouldn't it be better to limit the length of the field to an 
> appropriate size for the data, and encode that size in the Length?
>
>
> For example, the transportType in 3.2.1 is just one octet long 
> (unsigned8) - so there's no reason to encode it in 32 bits. Why not 
> encode it like so:
>
>     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
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |       TLV-Type = TBAx1        |    Length     | transportType |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>
> The natEvent in 3.2.8 is also an unsigned8, so it would be encoded 
> similarly - except that the TLV-Type would be 230, which is the IE ID 
> of the IPFIX natEvent.
>
>
> The natTransportLimit in 3.2.2 is an unsigned16 - so there's no need 
> to encode it in 32 bits. Why not encode it like so:
>
>     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
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |       TLV-Type = TBAx2        |    Length |natTransportLimit ...
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>   ...              |
>    +-+-+-+-+-+-+-+-+
>
> 3.2.9 and 3.2.10 would be similar. The encoding of 3.2.3, 3.2.4, and 
> 3.2.5 would all be as shown in the draft, except that the TLV-type 
> would be two octets long and contain the IPFIX IE ID. The 
> sourceTransportPort in 3.2.6 is an unsigned16 - so there's no need to 
> encode it in 32 bits. Why not encode it like so:
>
>     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
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    | TLV-Type = sourceTransportPort|    Length     | 
> sourceTransportPort ...
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>   ...              |
>    +-+-+-+-+-+-+-+-+
>
> The encoding of 3.2.7 would be similar.

---

From: Dean cheng <[email-masked]>
Date: Wed, 12 Oct 2016 03:55:51 +0000

Paul,

We certainly missed this email of comments, but 
please see below.

Regards
Dean

> -----Original Message-----
> From: radext [mailto:[email-masked]] On Behalf Of Paul Aitken
> Sent: Tuesday, October 11, 2016 1:43 PM
> To: [email-masked]
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: [radext] draft-ietf-radext-ip-port-radius-ext
> 

The definition of the TLV-Type field (refer to Section 2.3 of RFC6929)
is very different than that of IPFIX IE identifiers (refer to
Section 4 of RFC5102), not just the size (8 bits vs. 16 bits)
but also semantics, reserved values etc.  

> > required in the protocol, so why not remove it entirely?

The length field is the second required one in a TLV structure, refer
to Section 2.3 of RFC6929.

> > appropriate size for the data, and encode that size in the Length?

This question was also raised by Suresh Krishnan, which was 
already answered by Alan DeKok (see the attached).

> _______________________________________________
> radext mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/radext

Subject: Re: [radext] Suresh Krishnan's No Objection on draft-ietf-radext-ip-port-radius-ext-11: (with COMMENT)
From: Alan DeKok <[email-masked]>
To: Suresh Krishnan <[email-masked]>
Cc: The IESG <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>
Date: Thu, 18 Aug 2016 07:33:53 +0000

On Aug 18, 2016, at 5:53 AM, Suresh Krishnan <[email-masked]> wrote:
> * Why are the the uint8 and unit16 based types getting stuffed into 32
> bit fields inside the TLVs? This feels like a complete waste. Is there
> any specific reason this is required?

  The limited nature of RADIUS data types.  This behaviour is mandated by RFC 6158

https://tools.ietf.org/html/rfc6158#appendix-A.2.1

---

From: Paul Aitken <[email-masked]>
Date: Wed, 12 Oct 2016 09:13:53 +0100

Thanks for the quick response, Dean. Please find some responses inline:

> but also semantics, reserved values etc.

Nowhere in this draft says that the TLV-type field refers to RFC6929.

Since there's a 1:1 mapping between the values of this field and IPFIX 
Information Element IDs (since most of section 3.2.x say, "This 
attribute carries IPFIX Information Element X"), it seems to be simply 
another name and another registry for the same thing.

> to Section 2.3 of RFC6929.

And my point is that it's redundant.

> already answered by Alan DeKok (see the attached).

(ie, "The limited nature of RADIUS data types.  This behaviour is 
mandated by RFC 6158")

Abstract the data type from the octets on the wire. Since the length is 
given in the packet (or can be infered from the TLV type per Q2 above), 
there's no need to transmit redundant octets.

P.

>>>     | TLV-Type = sourceTransportPort|    Length     | sourceTransportPort ...
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_radext&d=DQIFAg&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=dEtr3XevxedcGIFIR0evA2jpVRKktTCFIE9_hDx1iO4&s=t1n5rKGP56p05NoEUsOlAkh5nDurkUeCLuNN4hVxXSU&e=

---

From: Dean cheng <[email-masked]>
Date: Wed, 12 Oct 2016 22:28:45 +0000

Paul, please see inline below.
Thanks
Dean

> From: Paul Aitken [mailto:[email-masked]]
> Sent: Wednesday, October 12, 2016 1:14 AM
> To: Dean cheng; [email-masked]
> > We certainly missed this email of comments, but please see below.
> >
> > Regards
> > Dean
> >
> >>> Dear Authors, some questions came to mind while reviewing section
> >>> 3.2 of your draft:
> >>>
> >>>
> >>> Since there's a 1:1 mapping between the TLV-Type and IPFIX Element
> >>> ID, the scheme you propose would be more extensible if the IPFIX
> >>> Element ID was used instead of the TLV-Type since it would require
> >>> one less registry. True, one additional byte would be required in
> the encoding.
> >>> However, the advantage is that in future anyone could encode a new
> >>> TLV by using the relevant IPFIX ID, without having to also define a
> >>> new TLV-Type for it.
> > is very different than that of IPFIX IE identifiers (refer to Section
> > 4 of RFC5102), not just the size (8 bits vs. 16 bits) but also
> > semantics, reserved values etc.
> 

The reference to RFC6929 is repeated in Section 3.1.1, 3.1.2 and 3.1.3 
where the proposed Radius attributes are presented, but also in the
beginning of Section 3.2 before all sub-sections for individual 
proposed TLVs. Here is the statement at the beginning of Section 3.2:
    
   ....These TLVs use the format defined in [RFC6929]....  

> another name and another registry for the same thing.

As said, the TLV-Type field of Radius TLV (RFC6929) has totally
different size and meaning than that of the IPFIX IE identifier
(RFC5102), I'd consider it an engineering kludge to mingle them
together.

Please read Section 2.3 of RFC6929 and Section 4 of RFC5102.

> 
> 
> >>> Q2: In each TLV, the Length field is redundant since it can always
> >>> be determined from the TLV-Type: it's always 6, with special cases
> >>> for TLV-Types 5 (Length is 18) and 11 (Length could be determined
> by
> >>> a NULL terminator on the string). This suggests that the length is
> >>> not required in the protocol, so why not remove it entirely?
> > This question was also raised by Suresh Krishnan, which was already
> > answered by Alan DeKok (see the attached).
> 

Are you saying you want to redefine TLV as TV?

In this draft, we follow the TLV as defined in RFC6929 for
Radius protocol, and more generally, the structure of TLV
here is common across all the protocols that use TLV that
I know. I'm not sure how we could redefine the TLV here.

> >>> similarly - except that the TLV-Type would be 230, which is the IE
> >>> ID of the IPFIX natEvent.
> >>>
> >>>
> >>> sourceTransportPort in 3.2.6 is an unsigned16 - so there's no need
> >> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.ietf.org_mai
> >>
> lman_listinfo_radext&d=DQIFAg&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoC
> >>
> gBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=dEtr3XevxedcGIFIR0evA2jpVRKktTCFIE9
> >> _hDx1iO4&s=t1n5rKGP56p05NoEUsOlAkh5nDurkUeCLuNN4hVxXSU&e=