On Fri, Jul 11, 2008 at 3:06 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxx> 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.
OK, but I thought the draft also contained some clarifications for
4.0, which is why I asked. Maybe I'm thinking of something else.
>> > 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.
So to sum up, you think it is reasonable for the mount.nfs command to
specify clientaddr=0.0.0.0 if it can't determine the client's own
address for the server to use for callback.
I can clean that up in these patches and repost. Thanks for the review.
>> 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@xxxxxxxxxx>
>> >>>> ---
>> >>>>
>> >>>> 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
>
--
Edward R. Murrow told his generation of journalists no one can
eliminate their prejudices, just recognize them. Here is my bias:
extremes of wealth and poverty cannot be reconciled with a truly just
society.
-- Bill Moyers
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
