Subject: Re: Comments on 2292bis (Adv. API)

From: Erik Nordmark <[email-masked]>
Date: Thu, 19 Aug 1999 23:07:43 -0700 (PDT)

>----------------Begin Forwarded Message----------------<

Date: Thu, 19 Aug 1999 22:58:15 -0700 (PDT)
From: "Erik Nordmark" <[email-masked]>
Subject: Re: Comments on 2292bis (Adv. API)
To: "Vladislav Yasevich" <[email-masked]>
Cc: "IPNG Working Group" <[email-masked]>, [email-masked],
[email-masked], [email-masked]

Thank you all for the very careful review - it will make the document
much better.

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

Will do.

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

I know - but 80 columns was limiting things. I'll try with "TC" instead
of "traffic class" and see if it fits.

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

I agree there isn't perfect consistency.
But I do find it useful when writing code since I can access 
	rthdr->ip6r0_addr[i] 
instead of something like
	((struct in6_addr *)&rthdr[1])[i]
if I remove the field. 

> - 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().

OK.
 
> 
> 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".

OK.

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

zero/non-zero (i.e. true/false) is the intent. I'll clarify.

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

I'll clarify. It is "to have it mean that the application
will send down complete packets including the IPv4 header."

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

I'll add "An attempt to set IPV6_CHECKSUM for an ICMPv6 socket will fail."

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

Spec already says "By default, this socket option is disabled."
The option has no meaning for ICMPv6 sockets since they always do their
checksum.

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

What would you like clarified? Is is just
	int on = 1;

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

My intent was to leave that unspecified in order to allow implementations that
support both 2292 and 2292bis using the same value for e.g.
IP_RECVRTHDR and IP_RTHDR.
Such an implementation might want to return the value used to enable receipt
i.e. the integer.
Implementations that only support 2292bis would presumably return what was
set with setsockopt i.e. the sticky option content.

If no implementors will provide 2292 compatibilty using the above scheme
then we can nail down the behavior in the spec.

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

Good question! What should it be? (IPv6_NEXTHOP doesn't interact with
the others - it is the only way to specify the nexthop address.)
What we currently implement in Solaris is that the "easiest to change" 
gets precedence.
Thus if sin6_scope_id is set and the destination is link-local it is
used to determine the interface.
If not, if IPV6_PKTINFO is set in acillary data and the ipi6_ifindex is
non-zero it applies.
Finally IPV6_MULTICAST_IF applies for multicast packets.

Do we want to specify such details? If so, does the above order make sense?

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

The example pictures in Appendix B (page 55) shows how things work.
segments is used to set ip6r0_len - inet6_rth_add() increments ip6r0_segleft
(and checks that it doesn't exceed ip6r0_len/2).
bp_len is just used for error checking.

I'll clarify this.

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

I don't understand what "broke the API" means.
If you define an option that doesn't follow the asignment rules in 2460
then you might not be able to access fields in the option used aligned
access.
If you use the inet6_opt_get_val() and inet6_opt_set_val(), and
the implementation of those functions take advantage of aligned access,
then you might see the sending and receiving applications dump core.
Perhaps we should add a note to point out that robust receivers should
verify alignment before calling inet6_opt_get_val().
Or have inet6_opt_get_val() check alignment and fail by returning -1?

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

Because in order to reserve space and lay out the option you would need
three things:
	the size of the option
	alignX
	alignY

You can reduce this by removing alignX (or is it Y?) if you assume that
the largest field is last in the option (as specified in 2460 appendix B).

So what does "break the API" mean?

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

Are you asking about sticky options or ancillary data?
For sticky options the simplest thing to me seems to be to
record the option value first (so that it can be returned in a getsockopt)
and then build a header template. The header template will be a function
of all the sticky options. This means that IPV6_RTHDRDSTOPTS and IPV6_RTHDR
can be setsockopt independently, but that the transmitted header will
only contain a destination header before the routing header when both of
them are set.

But other implementors might see things differently.
Also, is this something we should nail down in the spec?

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

I guess zero is the only too small value that is a multiple of 8.
Thus it is really"
	If the extlen value is not a positive multiple of 8 the
        function fails and returns -1.

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

You would need X, Y, and length since you can't calculate the length based
on X and Y. 2 parameters are simpler than 3, so I hope we can live with the
2460 Appendix B model.

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

In fact it is completely bogus. What I meant to say was the inverse:
The align value can not exceed the value of len.

I'll address the rest of the comments in a second email message.

   Erik
>----------------End Forwarded Message----------------<

--------------------------------------------------------------------
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: Sat, 21 Aug 1999 03:42:39 +0900

>>>>> On Thu, 19 Aug 1999 23:07:43 -0700 (PDT), 
>>>>> Erik Nordmark <[email-masked]> said:

> Finally IPV6_MULTICAST_IF applies for multicast packets.

As I said in some previous mails, I believe the problem is not so
simple.

First, the semantics of sin6_scope_id field is quite
ambiguous. Besides, the semantics for unicast addresses might differ
the semantics for multicast addresses(see Francis' mails).

Also, we have many methods to specify the outgoing interface other
than sin6_sope_id, IPV6_PKTINFO and IPV6_MULTICAST_IF.

> Do we want to specify such details? If so, does the above order make sense?

We want. The current status is quite confusing for application
programmers. In fact, some routing engineers in Japan need more
clarification on how to specifiy the outgoing interface. But I'm
afraid that your order is too simple to clarify the problem. Maybe we
need more work and should reach a consensus on proper precedence.

					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: Vladislav Yasevich <[email-masked]>
Date: Mon, 23 Aug 1999 15:41:25 -0400

>         ((struct in6_addr *)&rthdr[1])[i]

As Francis Dupont pointed out in an earlier message, you can have zero addresses
in a routing header (not very usefull) and the current definition can cause
confusion with this scenario.

For the case above the implementation can define a pointer to the addresses in
the routing header and use that pointer to reference all the addresses.  It will
get rid of the cast you showed.

> The option has no meaning for ICMPv6 sockets since they always do their

Could you state that in the spec?

> then we can nail down the behavior in the spec.

After a closer look at the option processing, I found that there is an ambiguity
between 2292 and 2292bis.  In particular, the ambiguity exists with
IPV6_HOPLIMIT option.  With the 2292, this was an 'on/off' options.  With
2292bis sticky options, this now takes the actually hop limit itself.  If the
implementaion decides to support both specs, how is it to pick which option to
set (both values are int)?

In general here, I agree with Sam Manthorpe's comment that we should not try to
be compatible with 2292.  This would get rid of the ambiguities and would allow
specification of getsockopt().

> Do we want to specify such details? If so, does the above order make sense?

There has been a lot of discussion of this particular issue on the list and I 
belive that we should clarify the details.  As it stands now, there is a lot of
confustion for application programers.

> So what does "break the API" mean?

The problem that we saw with this specification is that it heavily depends
on Appendix B of 2460.  In particular that the fields have to be ordered by
size.  If someone does not follow this specification then the API as it stands
right now will not be able to calculate allignment function correctly.
For example, considere these 2 options:

OPT1:(doesn't follow Appendix B)		OPT2:
	Length = 12					Length = 12
	Data1 = 8 bytes					Data1 = 4 bytes
	Data2 = 4 bytes					Data2 = 4 bytes
							Data3 = 4 bytes

The alignment of the end of both of these options is 4 since the last data field
is only 4 bytes.  If that is the case, then when setting up OPT1, the Data1
field may not be aligned on a natural boundry and may break the applications and
cause
alignment faults.

If we were to fix OPT1 to follow Appendix B, everything works fine.

There reason we are advocating for 'alignX' and 'alignY' as arguments to the
API is that they will already be defined in the Option Specification and
easily used ('length' will be the 3 argument).  If the Option Specification does
not follow Appendix B (it doesn't have to), then this API will still align the
data correctly and conserve as much space as possible (one of the goals stated
in 2460).

> Also, is this something we should nail down in the spec?

Yes, I was refering to sticky otions.  It looks like you are advocating caching
the options.  In you implementation if the user were to set both
IPV6_RTHDRDSTOPTS and IPV6_RTHDR options, and then unset IPV6_RTHDR, you would
still have the IPV6_RTHDRDSTOPTS option set, but will ignore it.  Is my
understanding correct?

It might be nice to have it nailed down in the spec.  What do the others think?

>         function fails and returns -1.

Will this be in the new spec?

> I'll address the rest of the comments in a second email message.

Thanks
-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: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Tue, 24 Aug 1999 12:52:36 +0900

>>>>> On Mon, 23 Aug 1999 15:41:25 -0400, 
>>>>> Vladislav Yasevich <[email-masked]> said:

>> > Section 8.2 [page 33]

(snip)

> It might be nice to have it nailed down in the spec.  What do the others think?

I agree with you that it sould be nailed down. The current
specification is surely ambiguous and this ambiguity will confuse
application programmers.

					JINMEI, Tatuya