Sockets Application Program Interface (API) for Multihoming Shim
draft-ietf-shim6-multihome-shim-api-17

State changed to Approved-announcement to be sent::AD Followup from IESG Evaluation::AD Followup.
Approved the document with RFC Editor notes that I'm sending to the authors. Wes has not picked up Lars' Discuss, and according to my own review the issues are either non-issues or have actually been changed in the new review. The Editor Notes are on changed text where I think the authers went too far.

[Ballot comment]
#1) Why assume only one shim sub-layer is active at a time?  Wouldn't the most useful thing for this API to work when WiFi and GSM data are both usable? (Or maybe I'm misunderstanding the text.) In any case, it'd be useful to say why handling both being active is hard or out of scope.

#2) I'm curious why there are no requirements to expose security related information, esp IPsec? If it's available wouldn't it be good to provide that?

[Ballot comment]
The fourth and fifth paragraphs of the Introduction would fit better
someplace else; perhaps in section 2, renamed "Terminology and
Background".

In section 5, what is a "context"?

Third bullet in section 5: s/Notification from
applications/Notification from applications and upper layer protocols/
(for consistency throughout the text of this bullet).

(Nit) Fifth bullet in section 5: s/for the/for a/
(Clarification) "The host" seems imprecise; is it really the shim
sub-layer that makes the substitution?

Ninth bullet: define "deferred context" (as well as "context"?) in
section 3.

In the description of SHIM_ASSOCIATED, I suggest leaving out the
specific parameter values (0/1), as those values are specified in
section 6.1; in fact, there appears to be a third value specified for
SHIM_ASSOCIATED (2) in section 6.1 that is not mentioned in the
earlier description.

The indication of which parameters are unused in a given setsockopt()
call, as in this text from section 6.4:

  Note that some members of the
  shim_locator (lc_ifidx and lc_flags) are ignored in the set
  operation.

is not consistently specified in section 6.1-14.

The word "placeholder" seems unnecessary at the beginning of
section 8.1.  Why not just call it a "data structure"?

8.1.  Data structure for Locator Information

  As defined in Section 6, the SHIM_LOC_*_PREF, SHIM_LOC_*_SEND, and
  SHIM_LOCLIST_* socket options need to handle one or more locator
  information.  Locator information includes not only the locator
  itself but also additional information about the locator which is
  useful for locator management.

  Figure 4 illustrates [...]

Also, for readability, perhaps s/locatior information/locator
information block/ ?

[Ballot discuss]
The description of SHIM_LOC_LOCAL_PREF in section 6.4 is unclear to
me.  Does this mecahnism set the preferred local locator or the
preference values (lc_prio and lc_weight) on a local locator?  Does it
get the currently preferred locator or the preference values for the
specified locator?  The same questions apply to section 6.5.

lc_proto is not specified in any of the examples in section 6, while
lc_ifidx and lc_flags are explicitly set to 0 even in the cases where
they are not used (e.g., section 6.4).  Is lc_proto a "don't care";
does it need to be set to 0 to show no NAT is involved; is it set to a
non-zero value to indicate a NAT will be traversed?

In section 8:

  lc_proto
      Internet Protocol number for the protocol which is used to handle
      locator behind NAT.  Typically, this value is set as UDP (17) when
      the locator is a UDP encapsulation interface.

What is the value of lc_proto if the locator is not behind a NAT, or
is there some other way to indicate that the lcoator is not behind a
NAT?  "Typically" is imprecise; if the value is not always
IPPROTO_UDP, where are the values formally defined?

Also in section 8:

  lc_addr
      Contains the locator.  In the case where a locator whose size is
      smaller than 16 bytes, an encoding rule should be provided for
      each locator of a given address family.  For instance, in case of
      AF_INET (IPv4), the locator should be in the format of an IPv4-
      mapped IPv6 address as defined in [RFC4291].

Why not "must be provided"?  Where are these encoding rules provided?
Why not "in case of AF_INET (IPv4), the locator is encoded in the
format of [...]"

Still in section 8:

  lc_ifidx
      Interface index of the network interface to which the locator is
      assigned.  This field is only used in a read (getsockopt())
      operation.

But the example in section 6.8 seems to indicate the use of lc.ifidx
with setsockopt().  I recommend dopping the last sentence here in
section 8 and leaving the explanation of the use of lc_ifidx (and
lc_flags, which also appears to be unused in some setsockopt() calls)
to the specific specifications in section 6.

[Ballot comment]
Section 2., paragraph 1:
>    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>    document are to be interpreted as described in [RFC2119].

  This document is very inconsistent with its use of RFC2119 terms.
  There are many lower-case instances of those terms that I think should
  be capitalized. There are other cases where wording changes to use
  RFC2119 terms would clarify things.


Section 6., paragraph 0:
> 6.  Socket Options for Multihoming Shim Sub-layer

  In many of the subsections under Section 6 that discuss the individual
  options, the explanatory text doesn't always explicitly state if what
  is being described is when an option is used with setsockopt vs. with
  getsockopt. The (unstated) default seems to assume setsockopt. It
  would be very good to explicitly clarify this, maybe even by
  explicitly adding subsections for set and get under each.

[Ballot discuss]
Section 5., paragraph 2:
>    o  Providing locator information to applications.  An application
>      should be able to obtain information about the locator pair which
>      was actually used to send or receive the packet.

  DISCUSS: What do you mean by "the packet"? Applications see sockets
  out of which they read data. With UDP, you could add a sockopt to get
  information about the last packet sent or received, but with TCP, I
  don't see how this is supposed to work? Or do you mean the addresses
  the shim layer is currently using for the socket?


Section 6.2., paragraph 3:
>    Default value is set as 0, which means that the shim sub-layer
>    performs identifier/locator adaptation if available.

  DISCUSS: I don't think an Informational API document is the place to
  specify mandatory default protocol behavior. (If this default behavior
  is in the protocol specification, please rephrase this and refer to
  the text there.) Note that there are several sections below (6.3,
  6.12, etc.) that define "default" protocol behavior, and this issue
  occurs whenever the default value for an option turns protocol
  behavior on or off (i.e., it doesn't apply in cases where the default
  merely affects the behavior of the API itself.)


Section 8.2., paragraph 0:
> 8.2.  Path Exploration Parameter

  DISCUSS: This section needs to use a RFC2119 MUST whenever it talks
  about what do do with parameters from RFC5334. It's not up to an
  Informational API document to weaken what is mandated there.

State changed to In Last Call from Last Call Requested.

The following Last Call Announcement was sent out:

From: The IESG
To: IETF-Announce
CC:
Reply-To: ietf@ietf.org
Subject: Last Call:  (Socket Application Program Interface (API) for Multihoming Shim) to Informational RFC


The IESG has received a request from the Site Multihoming by IPv6
Intermediation WG (shim6) to consider the following document:
- 'Socket Application Program Interface (API) for Multihoming Shim'
  as an Informational RFC

The IESG plans to make a decision in the next few weeks, and solicits
final comments on this action. Please send substantive comments to the
ietf@ietf.org mailing lists by 2011-02-10. Exceptionally, comments may be
sent to iesg@ietf.org instead. In either case, please retain the
beginning of the Subject line to allow automated sorting.

The file can be obtained via
http://datatracker.ietf.org/doc/draft-ietf-shim6-multihome-shim-api/

IESG discussion can be tracked via
http://datatracker.ietf.org/doc/draft-ietf-shim6-multihome-shim-api/

I have reviewed this draft. I think the draft is in pretty good shape, I have no major concerns. I do have a number of suggested clarifications, however. I would like you to update the draft and then we can go to IETF last call.

Technical:

> The role of the multihoming shim sub-layer (hereafter called "shim
> sub-layer" in this document) is to avoid impacts to upper layer
> protocols which may be caused when the endhost changes its attachment
> point to the Internet, for instance, in the case of rehoming event
> under the multihomed environment.

I think this should be stated slightly differently. The point of the shim layers is that for most cases, there will be no impacts to upper layers. However, the API is necessary for two cases: when the upper layer is particularly sensitive to impacts, or when it wants to benefit from better knowledge of what is going on underneath.

> Note that the shim sub-layer may conflict with other multihoming
> mechanisms such as SCTP and multipath
> TCP[I-D.ietf-shim6-applicability].  To avoid any conflict, only one
> of SHIM6 and HIP should be in use.
> 

Clarification. Did you intend to say that only one of SHIM6 and HIP *and* other multihoming mechanism should be in use?

>      *  In the case of SHIM6, an identifier called a ULID serves as an
>          EID.  A ULID is chosen from locators available on the host.
> 

Define/expand the ULID acronym.

> *  Preferred locator - The (source/destination) locator currently
>    used to send packets within a given context.  As defined in
>    [RFC5533], the preferred locator of a host 'A' is denoted as
>    Lp(A).
> 

Section 6.1 of RFC 5533 talks about Lp(A) as the "current locator", not preferred. Text later in Section 4 talks about preferred again. I wonder if there's some bigger issue here. Current is not necessarily the same as preferred, and I might imagine being able to set either one. I'm reading on...

> All of these socket options are defined at
>    level SOL_SHIM.
> 

Please refer to the relevant socket call that defines the level system (setsockopt).

> | SHIM_LOC_LOCAL_RECV        | o  | o  | Get or set the  | int  |
> |                            |    |    | parameter which |      |
> |                            |    |    | is used to      |      |
> |                            |    |    | request the    |      |
> |                            |    |    | shim sub-layer  |      |
> |                            |    |    | to store the    |      |
> |                            |    |    | destination    |      |
> |                            |    |    | locator of the  |      |
> |                            |    |    | received IP    |      |
> |                            |    |    | packet.        |      |
> | SHIM_LOC_PEER_RECV          | o  | o  | Get or set the  | int  |
> |                            |    |    | parameter which |      |
> |                            |    |    | is used to      |      |
> |                            |    |    | request the    |      |
> |                            |    |    | shim sub-layer  |      |
> |                            |    |    | to store the    |      |
> |                            |    |    | source locator  |      |
> |                            |    |    | of the received |      |
> |                            |    |    | IP packet.      |      |

I should either get more coffee or the explanations above are not very clear.

For instance, I do not understand why it would make sense to set the locator of received IP packets. And why do you say "parameter that is used to request ... store the ...."?

The opening sentence of Section 5.6 would be better fit here, IMO.

> | SHIM_APP_TIMEOUT            | o  | o  | Get or set the  | int  |
> |                            |    |    | timeout value  |      |
> |                            |    |    | for detecting  |      |
> |                            |    |    | failure.        |      |

Is this a timeout that the application will use to fail whatever it was trying to do, or a timeout that the shim should use to start exploring proactively, or a timeout that the shim should use to determine that the current locators have failed?

The opening sentence of Section 5.12 would be more accurate, IMO.

> The preferred locator can be set by setsockopt().  The shim sub-layer
> shall verify requested locator before it updates the preferred
> locator.
>    An error EINVALIDLOCATOR is returned when the validation of the
>    specified locator fails.

Verify in the security sense (check the HBA) or verify in the reachability sense? Will the setsockopt call return before or after reachability is verified? What errors are returned if the verification fails? Why does the first text fragment talk about verification and the second about validation? Are these the same or different thing?

> For example, an application can get the preferred locator by using
> the socket option as follows.
>
>        struct shim_locator lc;
>        int len;
>
>        len = sizeof(lc);
>
>        getsockopt(fd, SOL_SHIM, SHIM_LOC_LOCAL_PREF, &lc, &len);

How do you get/set preferences for multiple addresses? Do you set each address separately, or give an array of shim_locators in the socket call?


>  SHIM_LOC_LOCAL_SEND

Can you specify what is the difference to SHIM_LOC_LOCAL_PREF? One sets the preference, the other forces a choice?

> For example, an application can get the preferred local locator by
> using the socket option as follows.
>
>        struct shim_locator locator;
>
>        memset(&locator, 0, sizeof(locator));
>
>        getsockopt(fd, SOL_SHIM, SHIM_LOC_LOCAL_SEND, &locator,
>                  sizeof(locator));

Aren't you mixing SHIM_LOC_LOCAL_PREF/SEND in the text and the code above?

>        /* obtain local IP addresses from local interfaces */

The example in Section 5.10 is not that motivating, because presumably applications do not need to do anything if they want the shim to use all possible addresses from all interfaces.

Secondly, the example uses AF_INET addresses as well, and that is not supported by SHIM6. Perhaps that should be pointed out.

> The SHIM_APP_TIMEOUT option is used to get or set the Send Timeout
> value of the REAP protocol.  This option is effective only when there
> is a shim context associated with the socket.
Reference needed.

What does this option do if there is no SHIM6 but, say, HIP underneath?

> EINVALIDLOCATOR
>  This indicates that at least one of the necessary validations
>  inside the shim sub-layer for the specified locator has failed.
>  In case of SHIM6, there are two kinds of verifications required
>  for security reasons prior to sending an IP packet to the peer's
>  new locator; one is the return routability (check if the peer is
>  actually willing to receive data with the specified locator) and
>  the other one is the verification based on crypto identifier
>  mechanisms [RFC3972], [RFC5535].

Please specify what kind of return routability is being employed, do you specifically mean REAP and the corresponding checks in HIP?

> 6.2. Set Locator for Outgoing Packet
>
>
>    An application can specify the locators to be used for transmitting
>    an IP packet by sendmsg().  When the ancillary data of cmsg_type
>    SHIM_LOC_LOCAL_SEND and/or SHIM_LOC_PEER_SEND are specified, the
>    application can explicitly specify the source and/or the destination
>    locators to be used for the communication over the socket.  If the
>    specified locator pair is verified, the shim sub-layer overrides the
>    locator(s) of the outgoing IP packet.  Note that the effect is
>    limited to the datagram transmitted by the sendmsg().
>
>    When there is no shim context associated with the socket, an error
>    code ENOENT is returned to the application.
>
>    An error code EINVALIDLOCATOR is returned when validation of the
>    specified locator fails.

If the verification of the locators would require protocol exchanges (REAP), will sendmsg block?

> Such feedbacks are particularly useful for the
> shim sub-layer in the absence of REAP mechanism to monitor the
> reachability status of the currently used locator pair in a given
> shim context.

The feedback is useful even with REAP.


>    lc_proto
>      Internet Protocol number for the protocol which is used to handle
>      locator behind NAT.  Typically, this value is set as UDP (17) when
>      the locator is a UDP encapsulation interface.
>    lc_port
>      Port number which is used for handling locator behind NAT.

You lost me here. This is the first time NAT functionality is mentioned. Please describe this in more detail, or at the very least, point the reader forward to Section 7.1.1.

>    lc_weight
>      The weight value indicates a relative weight for locators with the
>      same priority value.  The range is 0-65535.

Which values are higher priority, 0 or 65535?

>    This document contains no IANA consideration.

Really? Where are the various socket option number spaces allocated?

> 13.1.2. Treatment of Unknown Destination Locator
>
>
>    If the shim sub-layer turns out to be SHIM6, the SHIM6 implementation
>    MUST reject the request for using unknown destination locator.
>
>    If the shim sub-layer turns out to be HIP, the HIP implementation MAY
>    accept the request for using unknown destination locator.

This seems like an insufficiently explained security issue. Why is it OK for HIP to do this, or at least you should describe what are consequences if it does so?

Editorial:

Please add the standard RFC 2119 reference and boilerplate text (as you are using the keywords from there).

>  == You're using the IETF Trust Provisions' Section 6.b License Notice from
>      12 Sep 2009 rather than the newer Notice from 28 Dec 2009.  (See
>      http://trustee.ietf.org/license-info/)
> 

Please switch to the newest boilerplate if and when you update the draft.

>  == Outdated reference: A later version (-22) exists of
>      draft-ietf-behave-v6v4-xlate-21
>
>  == Outdated reference: draft-ietf-hip-nat-traversal has been published as
>      RFC 5770
>
>  == Outdated reference: A later version (-06) exists of
>      draft-ietf-shim6-applicability-05
> 

Update these in the next revision, too.

> In this document, syntax and semantics of the API are given in the
> same way as the Posix standard [POSIX]

... in the same way as *in* the Posix standard?

>      provide positive feedbacks or negative feedbacks to the shim sub-

s/feedbacks/feedback/g

> | SHIM_LOC_LOCAL_PREF        | o  | o  | Get or set the  | *1    |
>

"Note 1" on the last column might be clearer. At least this read was wondering for a few seconds what datatype *1 was :-)


> | SHIM_ASSOCIATED            | o  |    | Get the        | int  |
> |                            |    |    | parameter which |      |
> |                            |    |    | indicates      |      |
> |                            |    |    | whether the    |      |
> |                            |    |    | socket is      |      |
> |                            |    |    | associated with |      |
> |                            |    |    | any shim        |      |
> |                            |    |    | context or not. |      |
> 

Can we say explicitly that 0 = not associated and 1 = associated?

>    *1: cmsg_data[] includes a single sockaddr_in{} or sockaddr_in6{} and
>    padding if necessary

Maybe "... csmg_data[] within msg_control includes ..." (provides the reader a better understanding of what you are talking about, since caddr_t appears in multiple fields of the struct).

> 7. Data Structures
>
>    This section data structures for the shim sub-layer.

Something is missing here.

>    lc_ifidx
>      Interface index of the network interface to which the locator is
>      assigned.  This field should be valid only in a read
>      (getsockopt()) operation.

... This field is only used in a read ...

>                uint8_t  pe_probenum;      /* # of initial probe */

... probes

> It is ffs exactly how the shim sub-layer should behave in
>    such erroneous cases.

It is outside the scope of this document to describe how ...

> SHIM_MAX_LOCATORS  The maximum number of the locators to be included
> in a locator list. 32.

Odd formatting.