Subject: AD review of draft-ietf-ipngwg-rfc2292bis-07.txt

From: Thomas Narten <[email-masked]>
Date: Tue, 06 Aug 2002 16:05:34 -0400

Here is my long overdue review of this document. Note that since this
is document is intended to be published as Informational, no IETF Last
Call will be done, and once the remaining issues are resolved the
document can be brought before IESG as a whole.

Most of these comments are nits.

The one more significant issue is that it would be good to clarify how
this document relates to the on-going Austin Group work (which has
standardized the Basic API). The references to Posix may be dated. The
related text in draft-ietf-ipngwg-rfc2553bis-06.txt might be
helpful. Also, some changes to the Basic API were made recently to
align it with the Austin Group work. Some of those same changes may
need to be made to this document as well.

Detailed comments.

Abstract:

>    A separate specification [RFC-2553] contain changes to the sockets

take out the references; saying "rfc 2553" without making it a
reference is OK. See the RFC editors guidelines on abstracts for more
details (draft-rfc-editor-rfc2223bis-02.txt).

Note: the abstract itself could really be rewritten to be more
clear/concise.

Early in the introduction, the document should say it is
updating/replacing the RFC 2292.

General: it would be good to have a proper reference to the posix
standards in the references section. I.e.:

>    4.3BSD Reno sockets API in 1990.  The reason is that these ancillary
>    data fields are part of the Posix.1g standard and should therefore be
>    adopted by most vendors.

Also, how does this align with the Austin group work and the revised
basic API? Are the documents in sync with each other? I say this
because the Basic API is apparently in alignment with the Austin Group
work. It would probably be useful to have some text saying how this
document relates to that standard.

Also, take a look at the most recent changes to the basic API. Do any
spill over into this document? One change to the basic API that seems
relevant to this one:

>     .  More alignments with [3]:
>         - [3] does not contain protocol family definitions (PF_xxx).
>          Replaced PF_INET6 with AF_INET6, or removed as appropriate.

This document still refernces PF_INET, etc.

I note that there are some new ICMP types that have been assigned that
this document doesn't seem to reference (e.g., those greater than
138). Should they be added?

>   are taken from http://www.iana.org/assignments/protocol-numbers.

this URL should be restricted to http://www.iana.org/numbers.html (the
URL in the document is not stable over long periods of time)

>   All data sent via raw sockets MUST be in network byte order and all

s/MUST/must/? (this document doesn't really use MUST/MAY/SHOULD
language). Either downcase this usage here, or include a definition
(e.g., a reference to the 2119 definitions).

>    In order to clear an installed filter the application can issue a
>    setsockopt for ICMP6_FILTER with a zero length.  When no such filter
>    has been installed corresponding getsockopt() will return the kernel
>    default filter.

last part doesn't parse quite right.

s/installed corresponding/installed, / ??

>    We note that many of the functions and socket options defined in this
>    document may have error returns that are not defined in this
>    document.  Many of these possible error returns will be recognized
>    only as implementations proceed.

This might have been true the first time this document was
produced. Is it still true today?

>    4.3BSD Reno sockets API in 1990.  The reason is that these ancillary
>    data fields are part of the Posix.1g standard and should therefore be
>    adopted by most vendors.

why are there appendices? seems like at least some of these are part of
the spec itself. Putting them in the appendices would imply that they
perhaps aren't important.

Re: references to RFC 2553; should they point to the revised ID
instead? (since it is about ready to become an RFC)

note: "int " is used throughout. Is this a short/long int? Does this
need to be clarified, or is the current usage adequate?

>      inet6_opt_set_val()  - add one component of the option content to the option
> 
>    Three functions deal with a returned options header:
> 
>      inet6_opt_next()     - extract the next option from the options header
>      inet6_opt_find()     - extract an option of a specified type from the header
>      inet6_opt_get_val()  - retrieve one component of the option  content

some of the lines go beyond column 80(?). Rfc editor won't allow
that. Can you reformat to keep lines within acceptable length? (Check
the RFC editor formatting rules for details.)

> 5.  Extensions to Socket Ancillary Data
> 
>    This specification uses ancillary data as defined in Posix.1g with
>    the following compatible extensions:

should this ID also include text relative to the austin group work?

>  -  A new CMSG_SPACE macro is defined.  It is used to determine how much
>     space need to be allocated for an ancillary data item.  See Section

s/need/needs/

>    operation.  Returning the received hop limit is useful for programs
>    such as Traceroute and for IPv6 applications that need to verify that
>    the received hop limit is 255 (e.g., that the packet has not been
>    forwarded).

how does this help traceroute? Traceroute needs to set, not receive
the ttl.

>    To specify the outgoing traffic class value, just specify the control
>    information as ancillary data for sendmsg() or using setsockopt().
>    Just like the hop limit value, the interpretation of the integer
>    traffic class value is
> 
>        x < -1:        return an error of EINVAL
>        x == -1:       use kernel default
>        0 <= x <= 255: use x
>        x >= 256:      return an error of EINVAL

note that the terminology surrounding diffserv/traffic class has
changed (see RFC 3260). Is the current API still OK? there are only 64
diffserv values now. (I think this is OK, but the authors should recheck).

> ENXIO         The interface specified by ipi6_ifindex does not exist
> 
> ENETDOWN      The interface specified by ipi6_ifindex is not enabled for
>               IPv6 use.
>

Indention not right. (too far to left)

>    This document and [RFC-2553] specify various methods that affect the
>    selection of the packet's outgoing interface.  This subsection
>    summarizes the ordering among those in order to ensure determined
>    behavior.

s/determined/deterministic/?

>    every multicast packet on the corresponding socket.  The reason for
>    the ordering comes from expectation that the source address is
>    specified as well and that the pair of the address and the outgoing
>    interface should be desired.

s/desired/preferred/?

   Source routing with IPv4 sockets API (the IP_OPTIONS socket option)

s/with/with the/ ?

>        int inet6_opt_init(void *extbuf, size_t extlen);
> 
>    This function returns the number of bytes needed for the empty
>    extension header i.e. without any options.

I don't understand this sentence. Does this then always return the
same value? And doesn't an "empty" header require 0 bytes? Or is this
actually "length remaining"?

Actully, the "length" field is weird in subsequent usages to. Is it
really a length, or an offset into the options? Calling it length is
confusing. Is it too late to change this?

> -  The new inet6_opt_XXX() functions were made different that the old

s/that/than/

Thomas
--------------------------------------------------------------------
IETF IPng Working Group Mailing List
IPng Home Page:                      http://playground.sun.com/ipng
FTP archive:                      ftp://playground.sun.com/pub/ipng
Direct all administrative requests to [email-masked]
--------------------------------------------------------------------

---

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Wed, 14 Aug 2002 17:28:03 +0900

(I'm explicitly ccing to Erik, because he may have some answers to a
comment.)

>>>>> On Tue, 06 Aug 2002 16:05:34 -0400, 
>>>>> Thomas Narten <[email-masked]> said:

> document can be brought before IESG as a whole.

Thanks for the detailed review and comments, and sorry for not
responding sooner.

I've incorporated trivial editorial nits to my local copy of the
draft, and I've not mentioned them in this response.

> Note: the abstract itself could really be rewritten to be more

I agree.  How about this (which is a total rewrite of abstract):

  This document provides sockets APIs to support "advanced" IPv6
  applications, as a supplement to a separate specification, RFC 2553.
  The expected applications include Ping, Traceroute, routing daemons
  and the like, which typically use raw sockets to access IPv6 or ICMPv6
  header fields.  This document proposes some portable interfaces for
  applications that use raw sockets under IPv6.  There are other
  features of IPv6 that some applications will need to access: interface
  identification (specifying the outgoing interface and determining the
  incoming interface), IPv6 extension headers, and path MTU information.
  This document provides API access to these features too.  Additionally,
  some extended interfaces to libraries for the "r" commands are
  defined.  The extension will provide better backward compatibility to
  existing implementations that are not IPv6-capable.

>> data fields are part of the Posix.1g standard and should therefore be

Are you suggesting to add the POSIX standard in References explicitly?
Or are you suggesting to check the latest version of the standard and
to rewrite the text accordingly (if necessary)?

> document relates to that standard.

Hmm, good point.  As far as I know, there has been no effort to synch
the advanced API to other "official" standards.  Also, rfc2292bis is
not as mature as rfc2553bis; 2292bis is a total revise of RFC 2292,
and backward compatibility is not provided.  So, with the following

> updating/replacing the RFC 2292.

I'd propose to add the following paragraph to Introduction:

   This document is intended to replace RFC 2292 "Advanced Sockets API
   for IPv6", February 1998.  The revise is based on implementation
   experiences of the previous RFC, as well as some additional
   extensions that has been found as important throughout the IPv6
   deployment.  Note, however, that this document is still
   informational.  Once the API specification becomes mature and is
   deployed among implementations, it should be incorporated to official
   standards, such as POSIX (refer to the document explicitly -if
   necessary-).

Does it make sense?

> This document still refernces PF_INET, etc.

Okay, I've changed PF_xxx to AF_xxx at least.  I'm not sure if we need
more.

> 138). Should they be added?

I don't think so.  Since more and more new types will come, we need to
make a fixed point of "feature freeze".  And, at least to me, the
newer types are either immature or less interesting to applications.

> URL in the document is not stable over long periods of time)

Okay.  The corresponding sentence is now

  These are taken under http://www.iana.org/numbers.html.

> produced. Is it still true today?

As I said above, rfc2292bis is not very mature yet.  So, I think this
paragraph is still true, at least to some extent.  How about
s/many/some/ here?

> perhaps aren't important.

Hmm, the appendices were introduced in 2292bis-00, and I was not an
editor at that time.  Perhaps Erik can explain the intention.  Based
on my own check for the appendices, sections 22 and 23 can still be
there, because they are too-detailed examples.  21.3.4 and 21.3.5
should perhaps be moved back to main sections, since they are new
definitions in this API.  What do you think of it, Erik?

> need to be clarified, or is the current usage adequate?

I've checked by grepping the entire text.  I believe the current usage
is okay.

> should this ID also include text relative to the austin group work?

I'm not sure what this comment exactly means...does the proposed
change in Introduction answer to this comment?

> how does this help traceroute? Traceroute needs to set, not receive

Actually, I had the same question before, and I slightly recall
someone explained the intention at that time.  However, I don't have a
clear memory about it.  I admit the description still seems odd.

So, I don't mind to remove traceroute as an example, though.  This
feature is still useful for other purposed (as described just after
this part).  Unless anyone else clarifies this, I'll remove the
traceroute usage.

> diffserv values now. (I think this is OK, but the authors should recheck).

I've asked this to itojun, who originally proposed this socket
option.  Our decision is we should just go with RFC 2460 definitions
(not 3260).  So, the current text will not need to be revised.

> Indention not right. (too far to left)

Are you talking about the space between the keyrword "EXNIO" and its
description, etc?  If so, I first must point out that this part is
aligned with the longest keyword (EADDRNOTAVAIL).  I don't mind to
change the indentation policy, but is it really necessary?

> confusing. Is it too late to change this?

Yes, it always returns the same value (except -1 for errors).  In our
implementation, the value is 2 (the length of the "next header" and
the "length" fields of an IPv6 option header), but I think the actual
value can be hidden from the caller.

As shown in the usage examples in Section 23, the "length" value is
actually used as a kind of "offset."  I don't think the current text
is confusing, but we can rewrite the description using the term
"offset".  (It would not be too late, because the change will not
break compatibility to existing implementations).  Do you think we
need the change?

					JINMEI, Tatuya
					Communication Platform Lab.
					Corporate R&D Center, Toshiba Corp.
					[email-masked]
--------------------------------------------------------------------
IETF IPng Working Group Mailing List
IPng Home Page:                      http://playground.sun.com/ipng
FTP archive:                      ftp://playground.sun.com/pub/ipng
Direct all administrative requests to [email-masked]
--------------------------------------------------------------------

---

From: Erik Nordmark <[email-masked]>
Date: Mon, 19 Aug 2002 18:16:49 +0200 (CEST)

> definitions in this API.  What do you think of it, Erik?

Those are actually mentioned in section 5.
Perhaps the whole definition should be moved to section 5, but for consistency
section 21.3 should say something about them since it talks about the
use of the different CMSG macros.

  Erik

---

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Tue, 20 Aug 2002 05:48:09 +0900

>>>>> On Mon, 19 Aug 2002 18:16:49 +0200 (CEST), 
>>>>> Erik Nordmark <[email-masked]> said:

> Those are actually mentioned in section 5.

Ah, yes, I missed that part.

> use of the different CMSG macros.

Okay, then I'll move the new definitions to Section 5, and change
Section 21.3 so that it will only contain possible implementation
examples and usages.

					JINMEI, Tatuya

---

From: Thomas Narten <[email-masked]>
Date: Wed, 21 Aug 2002 10:32:12 -0400

> draft, and I've not mentioned them in this response.

Sounds good.

>   existing implementations that are not IPv6-capable.

Works for me!

> to rewrite the text accordingly (if necessary)?

Definitely suggesting the first.

I'm asking about the second. I wonder, for example, whether someone
familiar with the POSIX/Austin Group work has reviewed the document.
My concern is that there may be some fairly trivial inconsistencies
with this document and the basic API. It would be nice to try and fix
those (if they exist). Can anyone speak to this point?

>    standards, such as POSIX (refer to the document explicitly -if

Generally looks good, but I might reword it a bit as:

    This document updates and replaces RFC 2292.  This revision is
    based on implementation experience of the RFC 2292, as well as
    some additional extensions that have been found to be useful
    through the IPv6 deployment.  Note, however, that further work on
    this document may still be needed.  Once the API specification
    becomes mature and is deployed among implementations, it may be
    formally standardized by a more appropriate body, such as has been
    done with the Basic API [RFC XXX]

> Okay, I've changed PF_xxx to AF_xxx at least.  I'm not sure if we need

Nor do I. That's why I'm thinking it would be good if someone familiar
with the Austin work/basic API could review the document from this
perspective (if this hasn't been done yet).

> newer types are either immature or less interesting to applications.

OK.

> paragraph is still true, at least to some extent.  How about

OK

> change in Introduction answer to this comment?

I'm asking whether at this point in time we should be referencing the
posix.1g work as what we want to align with, or whether its the newer
revision that has been formally adopted by the Austin
Group. Presumably, they are not the same.

> this part).  Unless anyone else clarifies this, I'll remove the

works for me.

> (not 3260).  So, the current text will not need to be revised.

OK.

> change the indentation policy, but is it really necessary?

This is minor formatting issue. In the ID, the terms "ENXIO" and
"ENETDOWN" are all the way at the left  margin. But all the  other
text in the document is indented 3 spaces. This seems to be an
exception here.

> value can be hidden from the caller.

Right. If the function always returns the same value by definition,
why return a value at all?

> break compatibility to existing implementations).  Do you think we

Personally, I think the term "offset" would be clearer since that is
indeed what the variable seems to be holding..

Thomas

---

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Thu, 22 Aug 2002 16:38:37 +0900

>>>>> On Wed, 21 Aug 2002 10:32:12 -0400, 

>> I agree.  How about this (which is a total rewrite of abstract):

(snip)

> Works for me!

Okay, I'll replace abstract with the proposed one.

> Definitely suggesting the first.

Okay.  I'll do that like in the basic API draft.

> those (if they exist). Can anyone speak to this point?

Hmm, I think there are no more "trivial" inconsistencies, but I'll
hear from others for a while.

>     done with the Basic API [RFC XXX]

Okay.  I'll put it to the draft.

> Group. Presumably, they are not the same.

Okay, perhaps we need comments from experts on this area.  Or is it
possible to get the latest work of the Austin Group on the web?

> text in the document is indented 3 spaces. This seems to be an

Ah, I see.  I'll fix them.  (actually, this is not the only exception.
The document also has the same indentation violation in sections 1, 4,
5, 12, 17, and 18.  I'll fix them too.

> why return a value at all?

Because the actual value may be different among implementations, and
the value is transparently used in the succeeding calls to
inet6_opt_append(), etc.  I believe even an implementation that does
not return a constant value without breaking the specification.

> indeed what the variable seems to be holding..

So, perhaps we can improve the wording by saying

- the return value of inet6_opt_xxx is expected to be used
  transparently, and the application should not care about the exact
  value.
- in a sequence of calling inet6_opt_xxx, the 'prevlen' arguments are
  used as a kind of offset.  but, again, the application should not
  care about the exact value.

- (additionally) the return value of inet6_opt_init() may or may not
  be constant.  in any case, the application should not make any
  assumption on the actual return value.

Also, if necessary, I don't mind to change the parameter name
'prevlen' to (e.g.) 'offset'.

Does this make sense?

					JINMEI, Tatuya