Re: [PATCH 04/14] text-based mount command: get_client_address support for IPv6

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Jul 10, 2008 at 04:35:57PM -0400, Chuck Lever wrote:
> On Thu, Jul 10, 2008 at 3:43 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Thu, Jul 10, 2008 at 03:36:21PM -0400, Chuck Lever wrote:
> >> On Jul 10, 2008, at 3:30 PM, J. Bruce Fields wrote:
> >>> On Wed, Jul 09, 2008 at 08:37:24PM -0400, Chuck Lever wrote:
> >>>> Introduce IPv6-enabled version of get_client_address.  The legacy
> >>>> mount
> >>>> command could use this eventually as well.
> >>>>
> >>>> I don't remember how to tell an NFSv4 server to disable the callback
> >>>> channel:  whether an ANY address is passed with SETCLIENTID, or a
> >>>> loopback address is passed.  The patch allows either to be used with
> >>>> a
> >>>> compile-time switch.
> >>>
> >>> I would have thought INADDR_ANY.  But in any case we should just pick
> >>> one....
> >>
> >> Well, yes, but we should pick the "correct" one.  :-)  The patch does it
> >> this way just to make a note of this issue so we can make a decision
> >> before committing this upstream.
> >>
> >> Does RFC 3530 have any recommendation about this?
> >
> > Not that I can find on a quick skim.
> 
> Is it clarified in the NFSv4.1 draft?

4.1 callbacks use sessions, which I don't really know yet.

> > I don't see why the spec would forbid running over loopback, though, in
> > which case a loopback callback address would make sense.  And I assume
> > INADDR_ANY is always meaningless as a destination address, so is a
> > logical way to tell the server it can't call back to you.
> 
> Yep, I agree.
> 
> I recall a few years back at a CITI bake-a-thon there was a certain
> server vendor who had trouble with loopback callback addresses,
> probably because their implementation was server-only, so an NFSv4
> callback from loopback would make no sense for them.  The client in
> this case was sending a loopback callback address because it hadn't
> implemented a callback service and wanted to prevent the server from
> calling it back.
> 
> Do we have a high degree of certainty that sending an ANY address is
> appropriate if the client can't determine a reasonable callback
> address to send with SETCLIENTID?
> 
> Is it at least OK for Linux's NFSv4 server?

If not, it's a bug I'd want to fix.

--b.

> 
> If we think this might be a problem, I can change all this back to
> simply failing the mount request when the mount.nfs command can't
> figure out a valid callback address.
> 
> >>>>
> >>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >>>> ---
> >>>>
> >>>> utils/mount/network.c |  122 +++++++++++++++++++++++++++++++++++++++
> >>>> ++++++++++
> >>>> utils/mount/network.h |    2 +
> >>>> 2 files changed, 124 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/utils/mount/network.c b/utils/mount/network.c
> >>>> index 3f2721b..128d7f7 100644
> >>>> --- a/utils/mount/network.c
> >>>> +++ b/utils/mount/network.c
> >>>> @@ -918,3 +918,125 @@ int get_client_address(struct sockaddr_in
> >>>> *saddr, struct sockaddr_in *caddr)
> >>>>     }
> >>>>     return 1;
> >>>> }
> >>>> +
> >>>> +/*
> >>>> + * Try a getsockname() on a connected datagram socket.
> >>>> + *
> >>>> + * Returns 1 and fills in @buf if successful; otherwise, zero.
> >>>> + *
> >>>> + * A connected datagram socket prevents leaving a socket in
> >>>> TIME_WAIT.
> >>>> + * This conserves the ephemeral port number space, helping reduce
> >>>> failed
> >>>> + * socket binds during mount storms.
> >>>> + */
> >>>> +static int nfs_ca_sockname(const struct sockaddr *sap, const
> >>>> socklen_t salen,
> >>>> +                      struct sockaddr *buf, socklen_t *buflen)
> >>>> +{
> >>>> +   struct sockaddr_in sin = {
> >>>> +           .sin_family             = AF_INET,
> >>>> +           .sin_addr.s_addr        = htonl(INADDR_ANY),
> >>>> +   };
> >>>> +   struct sockaddr_in6 sin6 = {
> >>>> +           .sin6_family            = AF_INET6,
> >>>> +           .sin6_addr              = IN6ADDR_ANY_INIT,
> >>>> +   };
> >>>> +   int sock;
> >>>> +
> >>>> +   sock = socket(sap->sa_family, SOCK_DGRAM, IPPROTO_UDP);
> >>>> +   if (sock < 0)
> >>>> +           return 0;
> >>>> +
> >>>> +   switch (sap->sa_family) {
> >>>> +   case AF_INET:
> >>>> +           if (bind(sock, (struct sockaddr *)&sin, sizeof(sin)) < 0) {
> >>>> +                   close(sock);
> >>>> +                   return 0;
> >>>> +           }
> >>>> +           break;
> >>>> +   case AF_INET6:
> >>>> +           if (bind(sock, (struct sockaddr *)&sin6, sizeof(sin6)) < 0) {
> >>>> +                   close(sock);
> >>>> +                   return 0;
> >>>> +           }
> >>>> +           break;
> >>>> +   default:
> >>>> +           errno = EAFNOSUPPORT;
> >>>> +           return 0;
> >>>> +   }
> >>>> +
> >>>> +   if (connect(sock, sap, salen) < 0) {
> >>>> +           close(sock);
> >>>> +           return 0;
> >>>> +   }
> >>>> +
> >>>> +   return !getsockname(sock, buf, buflen);
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * Try to generate an address that prevents the server from calling
> >>>> us.
> >>>> + *
> >>>> + * Returns 1 and fills in @buf if successful; otherwise, zero.
> >>>> + */
> >>>> +static int nfs_ca_gai(const struct sockaddr *sap, const socklen_t
> >>>> salen,
> >>>> +                 struct sockaddr *buf, socklen_t *buflen)
> >>>> +{
> >>>> +   struct addrinfo *gai_results;
> >>>> +   struct addrinfo gai_hint = {
> >>>> +           .ai_family      = sap->sa_family,
> >>>> +#ifdef GENERATE_LOOPBACK_ADDRESS
> >>>> +           .ai_flags       = 0,            /* loopback */
> >>>> +#else
> >>>> +           .ai_flags       = AI_PASSIVE,   /* ANYADDR */
> >>>> +#endif
> >>>> +   };
> >>>> +
> >>>> +   if (getaddrinfo(NULL, "", &gai_hint, &gai_results))
> >>>> +           return 0;
> >>>> +
> >>>> +   *buflen = gai_results->ai_addrlen;
> >>>> +   memcpy(buf, gai_results->ai_addr, *buflen);
> >>>> +
> >>>> +   freeaddrinfo(gai_results);
> >>>> +
> >>>> +   return 1;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * nfs_callback_address - acquire our local network address
> >>>> + * @sap: pointer to address of remote
> >>>> + * @sap_len: length of address
> >>>> + * @buf: pointer to buffer to be filled in with local network
> >>>> address
> >>>> + * @buflen: IN: length of buffer to fill in; OUT: length of filled-
> >>>> in address
> >>>> + *
> >>>> + * Discover a network address that an NFSv4 server can use to call
> >>>> us back.
> >>>> + * On multi-homed clients, this address depends on which NIC we use
> >>>> to
> >>>> + * route requests to the server.
> >>>> + *
> >>>> + * Returns 1 and fills in @buf if an unambiguous local address is
> >>>> + * available; returns 1 and fills in an appropriate ANYADDR address
> >>>> + * if a local address isn't available; otherwise, returns zero.
> >>>> + */
> >>>> +int nfs_callback_address(const struct sockaddr *sap, const
> >>>> socklen_t salen,
> >>>> +                    struct sockaddr *buf, socklen_t *buflen)
> >>>> +{
> >>>> +   struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)buf;
> >>>> +
> >>>> +   if (nfs_ca_sockname(sap, salen, buf, buflen) == 0)
> >>>> +           if (nfs_ca_gai(sap, salen, buf, buflen) == 0)
> >>>> +                   goto out_failed;
> >>>> +
> >>>> +   /*
> >>>> +    * The server can't use an interface ID that was generated
> >>>> +    * here on the client, so always clear sin6_scope_id.
> >>>> +    */
> >>>> +   if (sin6->sin6_family == AF_INET6)
> >>>> +           sin6->sin6_scope_id = 0;
> >>>> +
> >>>> +   return 1;
> >>>> +
> >>>> +out_failed:
> >>>> +   *buflen = 0;
> >>>> +   if (verbose)
> >>>> +           nfs_error(_("%s: failed to construct callback address"));
> >>>> +   return 0;
> >>>> +
> >>>> +}
> >>>> diff --git a/utils/mount/network.h b/utils/mount/network.h
> >>>> index 8da7e20..2f4ff3a 100644
> >>>> --- a/utils/mount/network.h
> >>>> +++ b/utils/mount/network.h
> >>>> @@ -58,6 +58,8 @@ int nfs_string_to_sockaddr(const char *, const
> >>>> size_t,
> >>>> int nfs_present_sockaddr(const struct sockaddr *,
> >>>>                      const socklen_t, char *, const size_t);
> >>>> int get_client_address(struct sockaddr_in *, struct sockaddr_in *);
> >>>> +int nfs_callback_address(const struct sockaddr *, const socklen_t,
> >>>> +           struct sockaddr *, socklen_t *);
> >>>> int nfs_call_umount(clnt_addr_t *, dirpath *);
> >>>> int clnt_ping(struct sockaddr_in *, const unsigned long,
> >>>>             const unsigned long, const unsigned int,
> >>>>
> 
> 
> 
> -- 
> Chuck Lever