Subject: comments about draft-chakrabarti-ipv6-addrselect-api-03

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Fri, 12 Aug 2005 00:38:21 +0900

As requested, here are my comments about the addrselect-api draft.

First of all, I must say I'm not particularly fascinated by this API.
As (I think) I said before, I believe this can be useful in very
limited situations only (especially because applications need to be
modified - I generally agree with Francis on this point), and most
applications (and application writers) wouldn't be bothered with using
the additional feature.  In fact, the only people I've ever seen who
expressed interest in this API are mobile IP(v6) implementors,
expecting this API can be used in their mobile IP(v6) specific
implementations (e.g., ones that negotiate with the home agent).  I
respect that the draft provides one section showing usage examples,
but these are not convincing enough to me.

But I hear that the IPv6 chairs admitted this document to be published
as an individual RFC, and so I won't insist this point further.

Specific comments follows:

SUBSTANTIAL COMMENTS

1. I'm (still) not convinced that we need two flags for each single
   rule (e.g., IPV6_PREFER_SRC_TMP and IPV6_PREFER_SRC_PUBLIC for
   whether to prefer temporary addresses).  As we see in the current
   API proposal, introduces invalid combinations, making error
   handling more complex.  It also consumes more bits (see comment #2
   below) from the limited space.  I'd rather prefer a single flag bit
   for each rule.  For example, we'd only need IPV6_PREFER_SRC_TMP: if
   this is set, we prefer temporary addresses; if not set, we prefer
   public addresses.

   According to the draft, the main reason for having two bits is to
   allow different implementations to have different system default.
   But I believe we could deal with this with a single bit.  Again, as
   mentioned in the draft, we can do this with a single bit for the
   socket option.  Then we should be able to apply the same default
   for the getaddrinfo, since if the system default for the socket
   option were different from that for getaddrinfo it would not work
   as intended anyway (at least for source address selection).  In
   addition (or alternatively) we might be able to define something
   like "AI_DEFAULT" (which was in fact defined in RFC2553),
   containing the implementation default values, particularly for
   destination address selection.

2. I'm concerned about defining (yet other) family-specific flags for
   getaddrinfo() for two reasons.  First, such flags would be against
   the sense of "protocol-independentness" of this function.  Second,
   those consume the relatively scarce bit field just for one
   particular protocol.

   Regarding the first point, one may point out RFC3493 also define
   AF_INET6-specific flags: AI_V4MAPPED and AI_ALL.  That's true
   (unfortunately), but IMO we should not use the fact as an excuse
   for introducing further violation.

   As for the second point, ai_flags is an 'int' member as per
   RFC3493, which can contain at most 32 flags for today's most
   systems.  There may even be systems where getaddrinfo() is provided
   and the 'int' type is a 16-bit integer.  RFC3493 already defines 7
   flags for this member, and the current proposal of this API is
   going to add 10 more.  So this API wouldn't work if 'int' is a
   16bit integer.  I'm also concerned about the fact that this API
   consumes nearly one third of the available flags even for 32-bit
   systems, particularly for one specific protocol.

   So, if possible, I'd like to realize the API's goal without messing
   ai_flags further.  I admit this would be difficult in the current
   interface to getaddrinfo() though.  At least I don't have a clear
   idea about how to do that.  Possible random thoughts at this moment
   (while those are not a perfect solution) include:
   - reduce the number of additional flags by the '1 flag per rule'
     approach (comment #1)
   - add another member to the addrinfo structure for additional
     (particularly protocol dependent) flags.  note that we could do
     this without breaking (binary) backward compatibility since
     multiple addrinfo structure are combined as a linked list.
   - allow the 'hints' structure to have 'next' entries where protocol
     dependent flags are defined (although this does not work on
     *BSD's implementation since it requires hints->ai_next be NULL)

3. I suspect the necessity of IPV6_PREFER_SRC_{LARGE,SMALL}SCOPE.
   Those might actually be even harmful.  The API draft says these
   correspond to Rule 2 of the source address selection defined in
   RFC3484.  But the RFC says:

     Rule 2 (prefer appropriate scope) MUST be implemented and given high
     priority because it can affect interoperability.

  Is it really okay to provide an easy knob for applications to
  "reverse" the logic despite the strong requirement?  And, I'm
  actually not sure in which (practical) case we want to specify the
  non default rule.  The last paragraph of Section 3 provides an
  example, but it's not very specific, and cannot be used as a
  practical example.

  At the very least, I'm not even sure how the logic of Rule 2 is
  modified if we specify IPV6_PREFER_SRC_SCOPE.  The rule defined in
  RFC3484 is:

    Rule 2:  Prefer appropriate scope.
    If Scope(SA) < Scope(SB): If Scope(SA) < Scope(D), then prefer SB
    and otherwise prefer SA.  Similarly, if Scope(SB) < Scope(SA): If
    Scope(SB) < Scope(D), then prefer SA and otherwise prefer SB.

  How would this look like with IPV6_PREFER_SRC_SCOPE?

  At the moment, my primary suggestion is to remove
  IPV6_PREFER_SRC_{LARGE,SMALL}SCOPE.  If we really want to keep
  those, I'd request the semantics be defined much more clearly.

  [Note: I can see the meaning and the usage of
  AI_PREFER_SRC_{SMALL,LARGE}SCOPE for destination address selection.
  In fact, this is another example why these IPV6_xxx and AI_xxx sets
  cannot necessarily be corresponding.]

SEMI SUBSTANTIAL COMMENTS

4. Considering the background motivation of the advanced API, I think
   we should also provide an ancillary-data object corresponding to
   the proposed socket option in order to control the address
   selection policy per-packet basis with a single system call (i.e.,
   sendmsg()).

5. (AFAICS) This document does not explicitly define the return value
   of getsockopt(IPV6_ADDR_PREFERENCES).  Even though it's pretty
   trivial, I believe an API specification should generally provide
   such 'trivial' things very clearly and explicitly.

6. [1st line of page 4 (continued from page 3)]

     [...]  Since source address selection and destination address
     ordering need to be partially implemented in getaddrinfo() [3] the
     corresponding set of flags are also defined for that routine.

   I think this is too strong.  Although destination address ordering
   (selection) would typically be implemented through getaddrinfo(),
   the selection rules themselves are quite general and could be
   realized in some other way.

7. [3rd paragraph of Section 5]
     [...] Thus when
     AI_PREFER_SRC_TMP is set , getaddrinfo() returns DA before DB and SA
     before SB likewise.

   I don't understand this sentence.  What does 'and SA before SB
   likewise' mean?

8. [page 12 (Section 5), just after the flag list]

     The above flags are ignored for the AF_INET address family as the
     address selection algorithm defined in section 5 of [1] only applies
     to the IPv6 addresses.

   This is not really true.  RFC3484's destination address selection
   considers both IPv4 and IPv6 addresses (See Section 3.2 of
   RFC3484).

   Besides, even if RFC3484 only considered IPv6 addresses, the API
   statement would still not be very accurate: what if we have
   IPv9?:-)

9. [first paragraph of Section 6]

     An application only needs to call getsockopt() prior calling
     setsockopt() if the application needs to be able to restore the
     socket back to the system default preferences.

   This seems odd.  Shouldn't the application just be able to reset
   the rules to the default by specifying the all-0 flags?  If not,
   why do we need the sets of two flags to begin with?

10. [second code fragment shown in Section 6]

   This does not seem to be very accuratete or consistent.  According
   to manual pages (available around me), the third arguments of
   getsockopt() and setsockopt() are 'void *' and 'const void *',
   respectively.  On the other hand, this example code casts the
   arguments to setsockopt() as 'char *' while it simply passes
   'uint32_t *' to getaddrinfo().  If we ignore minor type-matching
   issues for brevity, I'd simply remove all the cast.  If we're
   willing to show casting for accuracy, we should be consistent and
   use accurate casting for all the examples.

11. [second to last paragraph of Section 6]

     In order to allow different implementations to do different parts of
     address selection in getaddrinfo() and in the protocol stack, this
     specification requires that applications set the same flags when
     calling getaddrinfo() and when calling setsockopt().  For example, if
     the application sets IPV6_PREFER_SRC_COA flag, it must use
     AI_PREFER_SRC_COA flag when calling getaddrinfo().  If applications
     are not setting the same flags the behavior of the implementation is
     undefined.

  The phrase of "the same flags" (there are two occurrences) is not
  very accurate, since these are actually different, at least as macro
  names.

12. [second bullet of Section 7]

  What would getsockopt() return for preference flags that are not
  supported in the system? (see also comment #5)

13. [third bullet of Section 7]

      If an implementation supports both stream and datagram sockets, it
      should implement the address preference mechanism API described in
      this document on types of sockets.

  I simply don't understand what this means...could you elaborate?
  (BTW: perhaps 'on types of sockets' should be 'based on types of
  sockets'?  Even if so, I still don't understand what this means,
  though)

14. [second to last paragraph of Section 8]

     Other destination rules (#4-prefer home address; #7-prefer native
     interfaces) could have been applicable.  But the problem is that the
     local system does not know whether a destination address is a tunnel-
     address for destination rule #7.  It can only know for sure if the
     destination address is one of its own.

  I don't think this is really correct.  First, destination rule #7 is
  'Prefer native transport' not 'interfaces'.  Second, the
  implementation can syntacatically identify 'non-native' destinations
  in some cases (e.g., for 6to4 addresses).  Third, at least in
  theory, the local implementation (even an application) could
  identify the outgoing interface for a given destination, and might
  be able to identify it's a tunneling interface.  I do not
  necessarily require this API consider destination rule #7, but the
  "excuse" for not doing so should be accurate and convincing.

15. [Section 10]
  Do we really need (standardize) a separate function like
  inet6_is_srcaddr()?  Perhaps I'm pretty dubious about this
  just because I'm dubious about the usefulness of the entire API in
  the first place, but I'd suggest confirming this point with other
  implementers, particularly with those who are interested in this
  API.

16. [last paragraph of page 20]
     The function returns true when srcaddr corresponds to a valid address
     in the node and that address type satisfies the preference flag(s).

  I don't think 'that address type satisfies the preference flag' is
  well-defined, although it is intuitively understandable.  Please be
  more precise (c.f. comment #5).  There may actually be subtle cases:
  e.g., what does it mean if I specify IPV6_PREFER_SRC_SMALLSCOPE as
  'flags'?  That is, 'small' is only meaningful when we are comparing
  multiple things, while this function takes only one address.  I
  suspect this indicates we should reconsider the function design.

17. [regarding inet6_is_srcaddr() in general]

  If we really need this function (see comment #15), I believe we
  should clearly separate error cases, i.e., we should not overload
  the 'false' return value.  This might sound a matter of taste, but I
  believe it's a common and well-known practice.

PURELY EDITORIAL COMMENTS

18. [first bullet of Section 2]
   o  While some system have per environment/application flags (such as
      environment variables in Unix systems) this might not be available
      in all systems which implement the socket API

=> s/some system have/some systems have/

19. [last sentence of page 9]
   The following example illustrates how it is used on a AF_INET6
   socket:

=> s/a AF_INET6/an AF_INET6/

20. [overall]
   There is a mixture of 'Care-of' (even not at the BOL), 'care-of',
   'Care-Of' and 'Care-of' (and perhaps more).  There are even some
   odd forms like "Care-Of_address" or "care-of -address".  Those
   should be unified.

21. [overall]
   There are some redundant white space after opening parenthesis.
   For example, 
      Destination address selection rule #8 ( prefer smaller scope):
   (page 17)

   Those should be removed.

22. [first line of page 21]

   or it does not match an address which satisfy the preferences
   indicated, the function returns false.

=> s/satisfy/satisfies/

					JINMEI, Tatuya
					Communication Platform Lab.
					Corporate R&D Center, Toshiba Corp.
					[email-masked]

--------------------------------------------------------------------
IETF IPv6 working group mailing list
[email-masked]
Administrative Requests: https://www1.ietf.org/mailman/listinfo/ipv6
--------------------------------------------------------------------

---

From: Erik Nordmark <[email-masked]>
Date: Fri, 26 Aug 2005 15:59:51 -0700

Jinmei,

Thanks for your careful comments on the draft.

>      multiple addrinfo structure are combined as a linked list.

That is a good idea, but I think we still need to define one flag in 
order to provide compatibility for unmodified applications (at the 
binary as well as source level). I'd like you comments on the approach 
below:

We add a field to the struct addrinfo (to the end so that the offsets of 
the existing fields are not changed, since this would break existing 
binaries) called ai_preferences.

In this ai_preferences we can then put all the preference related flags.

We also define a single new AI_ flag, call it AI_PREFERENCES. This flag 
indicates that the application has initialized the ai_preferences field 
(either to zero, or set some of the preferences flags).

The getaddrinfo() implementation will only look for flags when 
AI_PREFERENCES is set; this avoids looking in a ai_preferences field 
that was not initialized by the application (to provide compatibility 
for old applications, or those that do not wish to express any preferences).

This means that struct addrinfo gets larger, but that will not cause any 
applications to break, since they use freeaddrinfo() to free the 
returned list of addrinfo structures.

>      priority because it can affect interoperability.

In the past there was requests to add flags for the SCOPE, for both 
source and destinations. I can't say I'm a big fan of these flags.

If there is consensus that we don't need them (Hint: those that want
*PREFER_*_SCOPE should speak up now), I don't have a problem dropping them.

I'll comment on your other points in a separate mail once we can get 
past the above issues.

    Erik

---

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Sun, 28 Aug 2005 06:28:44 +0900

>>>>> On Fri, 26 Aug 2005 15:59:51 -0700, 
>>>>> Erik Nordmark <[email-masked]> said:

> (either to zero, or set some of the preferences flags).

This looks like a reasonable idea.  And I see the need for the new
AI_PREFERENCES flag (i.e., the hint structure is built by the
application, and the added field passed by an old application can be
garbage).

One may want to make the new field more generic, rather than the
specific purpose of preferences, but I don't have a strong opinion on
this.

> *PREFER_*_SCOPE should speak up now), I don't have a problem dropping them.

Perhaps I was not clear enough in my previous message, so I repeat my
suggestion (or preference) here.  I suggest

- to remove IPV6_PREFER_SRC_{LARGE,SMALL}SCOPE, and
- to keep IPV6_PREFER_DST_{LARGE,SMALL}SCOPE

> I'll comment on your other points in a separate mail once we can get 

Okay.  But I'll be out of town from now for one week, and I'll be
inactive on the (possible) follow-up discussion during the period.

					JINMEI, Tatuya

---

From: Erik Nordmark <[email-masked]>
Date: Mon, 29 Aug 2005 11:18:27 -0700

> specific purpose of preferences, but I don't have a strong opinion on

Yes, we could call the new field ai_extended_flags or something like 
that (ai_eflags?).
Taking your concern about address family specific flags, we could also 
explicitly label this (ai_ipv6_flags?).

I don't know how to decide which way to go on this, since I currently 
can't imagine what other things might be added to getaddrinfo() over time.

Also, if we change it to only define a single AI flag, are you still 
concerned about having one flag per "feature" (both a PUBLIC and TMP 
flag for instance)?

I've tried to think of ways of avoiding that for getaddrinfo() (it's 
easy for setsockopt - just require that the applications call getsockopt 
to get the system default flags and then modify those and pass them to 
setsockopt). For getsockopt, in order to allow the system default to 
evolved over time and not assuming the applications are recompiled, it 
seems that, in order to have one flag per feature one would need to 
introduce a new API to get the initial addrinfo features. Something like
	hints.ai_preferences = getaddrinfo_default_preferences();
or
	getaddrinfo_defaults(&hints);
and then the application can do this request TMP instead of PUBLIC:
	hints.ai_preferences &= ~PUBLIC;
	hints.ai_preferences |= TMP;

To me this looks messier than having two flags per feature. And with 
dynamic libraries it will cause an application linked against the "new" 
world to fail when run on an older system (which doesn't have the new 
function in libc/libnsl/libsocket/wherever).

Comments?

> - to keep IPV6_PREFER_DST_{LARGE,SMALL}SCOPE

ok.

Patiently waiting for arguments from others to keep 
IPV6_PREFER_SRC_{LARGE,SMALL}SCOPE.

    Erik

---

From: Erik Nordmark <[email-masked]>
Date: Mon, 29 Aug 2005 12:29:33 -0700

> but these are not convincing enough to me.

FWIW To me, the public/temporary address knob is just as important, if 
not more important than the home/coa knob.

>   How would this look like with IPV6_PREFER_SRC_SCOPE?

FWIW the semantics of IPV6_PREFER_SRC_SMALLSCOPE would be to invert rule 
2, so that the algorithm instead of prefering the smallest scoped source 
which has a scope of at least the destination address, it would prefer 
the largest scoped source which has a scope which is at most the same as 
the destination address.

But, like you, I question the utility of this.

>    selection policy per-packet basis with a single system call (i.e.,

We can definitely add that.
Although I sometimes wonder how useful ancillary data options are on the 
send side (they are critical on the receive side, so that the 
application can see what was "part of" each received datagram).

>    such 'trivial' things very clearly and explicitly.

Do you mean the actual return value (which is 0/-1 according to the Open 
Group standard for getsockopt), or what value is returned in the *optval?

>    realized in some other way.

Yes, the point is that the API shouldn't constrain or assume which parts 
of address selection is implemented in getaddrinfo() or in the kernel. 
We'll make this more clear.
Something along the lines of "should not assume that implementations 
implementa particular aspect of RFC 3484 in getaddrinfo"

>    I don't understand this sentence.  What does 'and SA before SB

Oops. I should say
"Then SA is used when communication to DA, thus the temporary
address gets preferred."

>    considers both IPv4 and IPv6 addresses (See Section 3.2 of

The statement was try when there was no DST flags.
So we can correct it to say "the above SRC flags ... as the source 
address selection ..."

>    statement would still not be very accurate: what if we have

Currently the document is silent on AF_INET9 semantics; did you want it 
to say something specific about it? ;-)

>    why do we need the sets of two flags to begin with?

We need two flags per feature in order handle getaddrinfo() without 
casting the default flags in concrete and not introducing some new
getaddrinfo_default_preferences() function.
To me it makes sense to have setsockopt work the same way conceptually 
as getaddrinfo, so as long as getaddrinfo has two flags per feature, 
then setsockopt should have the same.

With two flags then for any X/not-X pair, the semantics of setting 
neither X or not-X is a no-op.
This implies that setting no flag at all must also be a no-op.

>    use accurate casting for all the examples.

Will fix.

>      calling getaddrinfo() and when calling setsockopt().  For example,if

Correct.
The same flag at the semantic level, which correspond to different flags 
for the two API parts.

But if we do a ai_preferences type field, then we can actually use the 
same flag constants in both places (IPV6_PREFER_...) if we want to. This 
would make the document shorter, and might be a simplication for the users.

>   supported in the system? (see also comment #5)

I think it makes sense to return the defaults per RFC 3484 and its 
future updates. Thus IPV6_PREFER_SRC_PUBLIC would be set even if 
temporary addresses are not implemented, and IPV6_PREFER_SRC_HOME set 
even if mobile IPv6 is not implemented.
But this is more from a consistency reason than by necessity; returning 
0 for those flags would be ok as well assuming the application just 
turns flags on and off.

>   sockets'?  Even if so, I still don't understand what this means,

Oops - should say "on both types of sockets."

>   'Prefer native transport' not 'interfaces'.

Correct. Will fix.

>  Second, the
>   in some cases (e.g., for 6to4 addresses). 

Yes, but we have the preference table to handle that, without any 
explicit rules on the nativeness of the transport.
(It might be a defect in 3484 that it has both the "prefer native 
transport" and the 6to4 addresses in the table. What happens when they 
conflict?)

> Third, at least in

Sure, at the local end.
But the text in RFC 3484 can be interpreted to mean that by magic the 
host can determine whether there is some IPv6 in IPv4 tunnel somewhere 
between the host and a particular destination address. It can only do 
that if 1) the destination syntactically indicates it does tunneling as 
in the case of 6to4, or 2) the tunnel starts on the host itself.

What do you suggest we clarify to make this more accurate?

>   implementers, particularly with those who are interested in this

I guess the feedback we've gotten doesn't capture whether or not it is 
necessary; only whether or not it is accurately described.
I don't recall anybody else arguing against this function. So I'm 
interested in the opinion of others.

And since you're dubious about the entire API, I guess you could argue 
we should remove all the other aspects of the API as well ;-)
Can't do it a function at a time.

>   suspect this indicates we should reconsider the function design.

We tried to make it clear which flags can and can not be tested, but I 
see the text is still not clear on this. The SCOPE aspects can not be 
tested with inet6_is_srcaddr(), but can be tested with 
INET6_IS_LINKLOCAL etc.

>   believe it's a common and well-known practice.

OK.

There are two different ways to do this and I'd appreciate your opinion 
on which one to choose.
1. A function which returns 3 values: true, false, failure. E.g
	int inet6_is_srcaddr(struct sockaddr_in6 *srcaddr,
		uint32_t flags);
    with 3 defined constans for the 3 possible return values.
    A special case is true=1, false=0, fail=-1.
2. A function which returns 0/-1 for ok/fail (like most library calls), 
and has an extra result parameter for the true/false response. For example,
	int inet6_is_srcaddr(struct sockaddr_in6 *srcaddr,
		uint32_t flags, int *resultp);

      Erik