add INET6 support to nfsdumpstate for the client callback address

[rmacklem]

Summary

PR#223036 noted that the client callback address was always printed out as 0.0.0.0
by nfsdumpstate(8) when an INET6 address was used.
This patch adds INET6 support for this, so that an INET6 address prints out.

Test Plan

Has been tested for NFSv4.0 and NFSv4.1 on an INET6 mount.

[rmacklem]

Fixed one place where I had typed AF_INET when it should have been AF_INET6.
This didn't seem to have caused problems during testing, so I don't think it actually mattered,
but it is now correct.

[rgrimes]

sys/fs/nfsserver/nfs_nfsdserv.c

3720

This may need #ifdef INET6 protection, as I do not believe struct sockaddr_in6 should be defined unless INET6 is defined, if it is that is another bug. Fixing this may lead to the further rethink this code.

3764

So we print the port number as octets using .%d.%d on the end of the IP address? I would of thought it was in the more normal form of uint16 :%d. I see that that the printf's are untouched by your diff, so that is long standanding prior art perhaps?

4035

Again, a reference to a struct sockaddr_in6 outside of ifdef INET6

sys/fs/nfsserver/nfs_nfsdstate.c

4077

Is INET6_ADDRSTRLEN defined outside of #ifdef INET6? If so there is a bug someplace else.

4130

Here the port length is only 7, in all the code I have read up to this point in this review the claim is 8. When doing the max calculation it appeared to be .255.255. which is 8 characters. I would expect the minimum to be .0.0 which is 4, not 3.

4231

Shouldnt that 0x0 be IN_ADDRANY? Also isnt it valid for this to be bound to a wildcard socket and how does this get printed if it is bound to a wildcard socket? It looks as if when bound to a wildcard socket we force the port to be 0 to, which is probably wrong?
I may totally be miss reading this code too.

[rmacklem]

Added replies to the inline comments. I will update the patch with the changes mentioned
in the replies later to-day.
Thanks for the review, rick

sys/fs/nfsserver/nfs_nfsdserv.c

3720

Here's a snippet from netinet/in.h, which I understand includes in6.h
automatically for all kernel builds. (It will complain if you try to include netinet6/in6.h
directly.)
As such, sockaddr_in6 will always be defined.

/* INET6 stuff */
#if POSIX_VISIBLE >= 200112
#define KAME_NETINET_IN_H_INCLUDED_
#include <netinet6/in6.h>
#undef __KAME_NETINET_IN_H_INCLUDED_
#endif

3764

That is my understanding of RFC 1833, which is the one referenced by RFC 7530
(the NFSv4.0 one). It has been this way since the code was written and I have not
heard any reports of interoperability problems, although I'll admit I haven't tested
an INET6 mount against a non-FreeBSD server.
(I know the .%d.%d is correct for INET.)

4035

Yep. There will be lots of those all over the NFS kernel code. I think it is
always defined, as noted above.

sys/fs/nfsserver/nfs_nfsdstate.c

4077

Again, defined in in6.h, which is always included.

4130

The above code is correct, but you are right, it is confusing.
More correctly, it is "8 - 1", where the "- 1" is because the
total here does not include NULL termination (which is included
in INET6_ADDRSTRLEN).

I'll change the comment above and re-write the lines below as
INET[6]6_ADDRSTRLEN - 1 + 8.

4231

I suppose it could be written as INADDR_ANY, but that might be more confusing.
It might be better for me to add a comment explaining that the NFSv4 folks use
0.0.0.0 to indicate "no valid address" for this case. This is another case where the
code has been this way for at least 15 years.
(Same goes for all zeros for INET6.)

Works to indicate "invalid address" because it can't work as a destination host
address.

[rmacklem]

This version of the patch has the changes mentioned in the inline comments.
A comment has been added explaining that INET[6]_ADDRSTRLEN includes
NULL termination and the length calculation has been changed to "-1 + 8"
from "+ 7" to make the calculation more understandable.

I have also replaced "0x0" with INADDR_ANY and added a comment explaining
that this is (mis)used to indicate "no valid callback address exists" by NFSv4.0.

I have not #ifdef INET6'd the uses of "struct sockaddr_in6" since it is always
included when "netinet/in.h" is included.

[rgrimes]

You can mark all my comments as done, you have addressed them to my satisfaction.

sys/fs/nfsserver/nfs_nfsdserv.c

3720

You can mark my comment as done, this is not a bug you have introduced, but is a much bigger problem of things being done wrong for INET6.

3764

I am satisfied, you can mark my comment as done.

4035

Ok again, and mark as done please.

[rmacklem]

Marked rgrimes@ inline comments as done, per his instructions.

[bz]

I only scrolled through and what I saw looked ok to me.