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

[Dean cheng <[e-mail]>]

Paul,

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

Regards
Dean

> -----Original Message-----
> From: radext [mailto:[e-mail]] On Behalf Of Paul Aitken
> Sent: Tuesday, October 11, 2016 1:43 PM
> To: [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [radext] draft-ietf-radext-ip-port-radius-ext
> 
> 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.

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.  

> >
> >
> > 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?

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

> >
> >
> > 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?

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

> >
> >
> > 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.
> 
> _______________________________________________
> radext mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/radext

--- Begin Message ---

On Aug 18, 2016, at 5:53 AM, Suresh Krishnan <[e-mail]> 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

--- End Message ---