Subject: Comments on 2292bis (Adv. API)

From: Vladislav Yasevich <[email-masked]>
Date: Wed, 11 Aug 1999 10:09:05 -0400

Richard, Matt, Erik

Please find attached some comments on draft-ietf-ipngwg-2292bis-00.txt,
Advanced Sockets API for IPv6.

This is a compilation of comments received from several members of the
Tru64 UNIX IPv6 team: Jim Bound, Gerri Harter, Jack McCann, Ken Powell, Sowmini
Varadhan, Eric Wong, Farrell Woods, and Vlad Yasevich.

- This document used the wording "described later" and "described shortly."
It would ease readability if you include the section numbers into these
statements.

Section 2.1 [Page 8]
- The comment in the following definitions is incorrect:
  402      struct ip6_hdrctl {
  403      uint32_t ip6_un1_flow; /* 8 bits traffic class, 24 bits flow-ID */

It should be 4 bit version, 8 bit traffic class, 20 bit flow-ID.

Section 2.1.2 [Page 9]
- The reviewers were questioning the need for the following definition:
  490      struct in6_addr  ip6r0_addr[1];  /* up to 127 addresses */

This definition made the ip6_rthdr0 structure stick out from all the others.
The suggestion is to remove this line and add a comment (just like all the
other header structures) that states that the Routing Header 0 is followed
by an array of IPv6 addresses.

- It would also be helpful if the following text was moved to Section 2.
[From page 10...]
  520     ...This API assumes that the fields in the
  521     protocol headers are left in the network byte order, which is big-
  522     endian for the Internet protocols.  If not, then either these
  523     constants or the fields being tested must be converted at run-time,
  524     using something like htons() or htonl().

Section 2.2.1 [page 11]
- The following definition is no longer valid:
  614     #define ICMP6_DST_UNREACH_NOTNEIGHBOR 2 /* not a neighbor */

It now refers to an address that is beyond the scope of source address as
defined in Section 3.1 of draft-ietf-ipngwg-icmp-v3-00.txt
The draft defines the code value 2 as "beyond scope of source address".

Section 2.3 [page 15]
  793     int  IN6_ARE_ADDR_EQUAL(const struct in6_addr *,
  794                             const struct in6_addr *);

- Please explain the the return type of this macro.  Does it return true/false,
or does it return 0 for equal, 1 for the first address > second address, -1 for
first address < second address.  I am guessing it's true/false since that is
what all the other macros return, but it is not clear.

Section 3 [page 16]
- In the last paragraph of the section you talk about special treatment for
sockets created with IPPROTO_RAW as the third argument.  You then go on
to say:
  892     ...(Note: This feature was added to
  893     IPv4 in 1988 by Van Jacobson to support traceroute, allowing a
  894     complete IP header to be passed by the application

What is the feature is the spec talking about?  This caused some confusion.

Section 3.1 [page 17]
- What happens if the IPV6_CHECKSUM option is set for ICMPv6 sockets?

- Is this option disabled by default only for raw sockets that were not
created with IPPROTO_ICMPV6?

Answers to these questions could help clarify this section.

Section 4 [page 20]
- A comment on the definitions of 'on' and 'off' for the setsockopt() calls
would be helpful.

- Describe the behavior of getsockopt() for IPV6_ options in this section.
Do these options support a getsockopt() call and what do they return?

Section 5.1 [page 26]
- What is the precedence of sin6_scope_id relative to the interface
specified by the IPV6_MULTICAST_IF, IPV6_PKTINFO and/or IPV6_NEXTHOP options?

Section 6.2 [page 29]
- What are the initialization values for the routing header?
We noticed that buffer length (bp_len) and the number of segments (segments)
are passed to inet6_rth_init() and this caused confusion as to what these
arguments are for.  Do we set seg_left to segments and if not, are these
arguments simply for error checking?

Section 7 [page 31]
  1691     ....  These
  1692     requirements and the terminology used with these options are
  1693     discussed in Section 4.2 and Appendix B of [RFC-2460].

- Is the assumption here that all options MUST be defined according to
section 4.2 and Appendix B of rfc2460?  We noticed that if we do not
follow the instructions in Appendix B, we were able to design options
that broke the API.  If the working of this API depends on Appendix B,
it should be stated so.

  1701     ....  The
  1700     functions below need to know the alignment of the end of the option
- Why does this spec institute this limitation for the API?  Why not just pass
the alignX and alignY?  I am guessing these will be defined in the Option
Specification and makes it easier for application programmers to use the API.
  It would make the API simpler if we were to pass X and Y to the
inet6_opt_append().  This would also cause the API not to break if
following Appendix B is NOT a MUST.

Section 8.2 [page 33]
  1825    When using ancillary data a Destination options header is passed
  1826    between the application and the kernel as follows: The set preceding
  1827    a Routing header are specified with the cmsg_level member is set to
  1828    IPPROTO_IPV6 and the cmsg_type member is set to IPV6_RTHDRDSTOPTS.
  1829    Any setsockopt or ancillary data for IPV6_RTHDRDSTOPTS is silently
  1830    ignore when sending packets unless a Routing header is also
  1831    specified.

- As the spec states here, IPV6_RTHDRDSTOPTS options are ignored unless Routing
header is present.  Does this mean that if the user has IPV6_RTHDRDSTOPTS
options and a Routing header, and then the user removes the Routing header, the
IPV6_RTHDRDSTOPTS options go away as well?  Should the implementation cache the
IPV6_RTHDRDSTOPTS options in case Routing header will appear later?

Section 9.1 [page 34]
  1875     ...  If the extlen value is too small or not a multiple of 8 the
  1876     function fails and returns -1.

- What do you mean by "too small" in this sentence?

Section 9.2 [page 34]
- This is the same question as question 2 for Section 7.  Providing X and Y
in the interface would make the API easier to use.
The only time that we've successfully calculated Y based on X and length was
when our options followed Appendix B or 2460.

Section 9.2 [page 35]
  1917     The align parameter must have a value of 1, 2, 4, or 8.  The len
  1918     value can not exceed the value of align.

The last sentece of this paragraph contradicts the previous paragraph as well
as the examples in the Appendix.  It seems like an unnecessary restriction
and it was the opinion of reviewers that it should be removed.

Section 9.5 [page 36]
- How is the "previous" length updated in this sentence?
  1977     ....  This
  1978     function returns the updated "previous" length taking into account
  1979     the option that was returned.

- Also, the main body of the spec does not indicate if inet6_opt_next returns
pad options or just skips over them. The example code in the appendix
shows the application code does recognize and ignore pad options.
Suggest a note be added to this section indicating inet6_opt_next does
return pad options.

Section 10 [page 37]
  2040    All Hop-by-Hop options must be specified in a single ancillary data
  2041    object.  Should multiple be specified the implementation might choose
  2042    an arbitrary one or drop the packet.

- Does this refer to multiple options or multiple ancillary data objects?

Sections 12.2 and 12.3
- What was the reason to put rcmd_af() and rexec_af() in this spec?
It was the concensus of the reviewers that these functions are not really needed
since they both used to call rresvport() and can be made to call
rresvport_af().  As it is currently, rcmd() and rexec() do the hostname
lookups.  At this point these functions can make the decision  to use V6 or V4
based on the addresses returned.  We do not really see the need for rcmd_af()
and rexec_af().

Section 13.
- Why does this section exist in addition to section 18 (TODO and Open Issues)? 
Should this be merged into one section?

Section 22 [page 47]
- This section and it's subsections add a lot of new requirements to
an implementation's handling of the msghdr and cmsghdr structures.
We don't think this should be treated as "informational" given that
many of these requirements are necessary to support the main body of
the spec.  Also, the new requirements are being made to posix standard
structures and macros.  Something needs to be done to make this section
less "informational."

Section 22.3.2 [page 51]
- It might be good to qualify that cmsg is a pointer to the cmsghdr struct
returned by a previous call to CMSG_NEXTHDR() or CMSG_FIRSTHDR().

Section 22.3.4 [page 52]
- This sentence is a little confusing, since the macro does not in
fact allocate space:
  2905     ....  This macro can be
  2906     used, for example, to allocate space dynamically for the ancillary
  2907     data.

One possible rewording might be:
  2905     ....  This macro can be
  2906     used, for example, to determine the size of the space needed to
  2907     dynamically allocate ancillary data.

Section 23.1:  The example starts with:

       void  *extptr;
       int   extlen;
       struct msghdr  msg;
       struct cmsghdr  *cmsgptr;
       int   cmsglen;
       struct sockaddr_in6  I1, I2, I3, D;

       extlen = inet6_rth_space(IPV6_RTHDR_TYPE_0, 3);
       cmsglen = CMSG_LEN(extlen);
       cmsgptr = malloc(cmsglen);
       cmsgptr->cmsg_len = cmsglen;

Should the last three lines shown here be changed to:

       cmsglen = CMSG_SPACE(extlen);
       cmsgptr = malloc(cmsglen);
       cmsgptr->cmsg_len = CMSG_LEN(extlen);

-vlad

+++++++++++++++++++++++++++++++
Vladislav Yasevich
Compaq Computer Corp.
110 Spit Brook Rd ZK03-3/T07
Nashua, NH 03062
(603) 884-1079
--------------------------------------------------------------------
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: Francis Dupont <[email-masked]>
Date: Thu, 12 Aug 1999 21:12:59 +0200

In your previous mail you wrote:

   Section 2.1.2 [Page 9]

=> I agree. In fact you can have zero addresses in a routing header
(not very useful if you are not tracking bug). I believe one'd like
to have ip6r0_addr[0] but some compilers don't like that...

   Section 2.2.1 [page 11]

=> I propose ICMP6_DST_UNREACH_BEYONDSCOPE

   Section 5.1 [page 26]
   - What is the precedence of sin6_scope_id relative to the interface
   specified by the IPV6_MULTICAST_IF, IPV6_PKTINFO and/or IPV6_NEXTHOP
   options?

=> this is an important issue (one already found a code with conflicting
IPV6_MULTICAST_IF, IPV6_PKTINFO and sin6_scope_id...)
   
   Sections 12.2 and 12.3

=> I agree but this is not an important issue.

Thanks

[email-masked]

---

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Mon, 16 Aug 1999 17:04:07 +0900

>>>>> On Thu, 12 Aug 1999 21:12:59 +0200, 
>>>>> Francis Dupont <[email-masked]> said:

   
> => I propose ICMP6_DST_UNREACH_BEYONDSCOPE

Agreed. In fact, the latest KAME snap uses (and the forthcoming
"unified" stack may also use) ICMP6_DST_UNREACH_BEYONDSCOPE. Also,
some KAME's userland applications such as tcpdump and ping6 have
already been rewritten on the new macro basis.

I would also like to propose to updating macro names as much as
current specification like. For example, macro names for MLD related
messages are not very MLD-like:

       #define ICMP6_MEMBERSHIP_QUERY      130
       #define ICMP6_MEMBERSHIP_REPORT     131
       #define ICMP6_MEMBERSHIP_REDUCTION  132

On the other hand, the corresponding messages in the MLD specification
are

     Multicast Listener Query,
     Multicast Listener Report, and
     Multicast Listener Done, respectively.

Though the meaning does not change, the names defined in the RFC 2292
(and 2292bis) are a bit confusing for application programmers.
Just FYI, KAME locally defines the following definitions (please note
that we don't insist on these local definitions at all):

#define MLD6_LISTENER_QUERY		130 	/* multicast listener query */
#define MLD6_LISTENER_REPORT		131	/* multicast listener report */
#define MLD6_LISTENER_DONE		132	/* multicast listener done */

Moreover, there are some new protocols that have been defined since
RFC 2292 and are about to be standardized, e.g.

	Router Renumbering
	Router Alert Option
	ICMPv6 name lookups
	Mobile IPv6
	...

I admit that it's hard (even impossible) to catch up all such new
protocols, but 2292bis would be a good opportunity. I hope the revised
API reflects as much changes as possible.

Again, just FYI, KAME locally defines the following definitions for
some of the above new protocols:

Macros for router renumbering:
  #define ICMP6_ROUTER_RENUMBERING_COMMAND  0	/* rr command */
  #define ICMP6_ROUTER_RENUMBERING_RESULT   1	/* rr result */
  #define ICMP6_ROUTER_RENUMBERING_SEQNUM_RESET   255	/* rr seq num reset */

Macros for ICMPv6 name lookups:
  #define ICMP6_FQDN_QUERY		139	/* FQDN query */
  #define ICMP6_FQDN_REPLY		140	/* FQDN reply */
  #define ICMP6_NI_QUERY			139	/* node information request */
  #define ICMP6_NI_REPLY			140	/* node information reply */

  #define ICMP6_NI_SUCESS		0	/* node information successful reply */
  #define ICMP6_NI_REFUSED	1	/* node information request is refused */
  #define ICMP6_NI_UNKNOWN	2	/* unknown Qtype */

Macros for IPv6 Router Alert HbH option:
  #define IP6OPT_RTALERT		0x05	/* 00 0 00101 */
  #define IP6OPT_RTALERT_MLD	0	/* Datagram contains an MLD message */
  #define IP6OPT_RTALERT_RSVP	1	/* Datagram contains an RSVP message */
  #define IP6OPT_RTALERT_ACTNET	2 	/* contains an Active Networks msg */

					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: Francis Dupont <[email-masked]>
Date: Mon, 16 Aug 1999 17:05:11 +0200

=> I agree for everything with these exceptions:

   Macros for router renumbering:
     #define ICMP6_ROUTER_RENUMBERING_COMMAND  0	/* rr command */
     #define ICMP6_ROUTER_RENUMBERING_RESULT   1	/* rr result */
     #define ICMP6_ROUTER_RENUMBERING_SEQNUM_RESET   255	/* rr seq num reset 
  */
   
=> I prefer shorter ICMP6_RT_RENUM_COMMAND, ... ie I replace
_ROUTER_RENUMBERING_ by _RT_RENUM_ for router renumbering "sub-definitions".
(I don't like names with more than 32 characters even I expect the compilers
I use can manage real long names :-)

=> I think we should wait for the new I-D (but the style is fine for me)?

=> I use a different style:

#define OPT6_ROUTER_ALERT	5	/*** Router Alert */

struct opt6_ra {			/* Router-Alert */
	u_int8_t	ra_ext;		/* extension type (*** 5) */
	u_int8_t	ra_len;		/* length (2) */
	u_int16_t	ra_code;	/* code */
};
#define OPT6_RA_GROUP	0		/* ICMPv6 Group Membership */
(will be replaced by OPT6_RA_MLD at the next cleanup)
#define OPT6_RA_RSVP	1		/* RSVP */
#define OPT6_RA_ACTNET	2		/* Active Networks */

I'd REALLY like to have a standard way to defined properly
aligned/packed option structures (opt6_ra & co definitions are in fact
for documentation only). RFC 2460 "xn+y" notation is not yet used
everywhere which is a *shame* and a source of problems for complex
options like binding update!
Of course I am (ie. my emacs is) ready to change definitions as soon as
there is a rough consensus...

Thanks

[email-masked]

---

From: Erik Nordmark <[email-masked]>
Date: Tue, 31 Aug 1999 13:47:59 -0700 (PDT)

> => I propose ICMP6_DST_UNREACH_BEYONDSCOPE

Done.

  Erik

---

From: Erik Nordmark <[email-masked]>
Date: Tue, 31 Aug 1999 14:05:08 -0700 (PDT)

> #define MLD6_LISTENER_DONE		132	/* multicast listener done */

To be consitent with ND_ (and since the protocol is really MLD and
not MLDv6) it would make sense to use the MLD_ prefix instead of MLD6_.

> RFC 2292 and are about to be standardized, e.g.

Is name lookups moving forward? Is it close to a WG last call?
(I haven't heard much about it.)

> API reflects as much changes as possible.

I agree that this would make sense.

Have you guys defined data structures for the above packet/option
formats in addition to the constants you list below?

If I get the info I can add it to 2292bis.

   Erik