[PATCH v3] net/9p/fd: support ipv6 for trans=tcp

[PATCH v3] net/9p/fd: support ipv6 for trans=tcp

[Joshua Murphy]

Allows specifying an IPv6 address when mounting a remote 9p file system.

Signed-off-by: Joshua Murphy <joshuamurphy@posteo.net>
---
v2 -> v3:
Replaced inet_addr_is_any with code that actually sets addresses.
Separated `addr == NULL` check to one before inet_pton_with_scope.
Removed no longer used function valid_ipaddr4.
Tested that connections to IPv4, IPv6, and privileged ports actually
connect.

v1 -> v2:
Simplified support using inet_pton_with_scope, initialized
sockaddr_storage variables to zero, and removed unneeded argument
from p9_bind_privport. 

 net/9p/trans_fd.c | 57 +++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 196060dc6..fb9406370 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -11,6 +11,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/in.h>
+#include <linux/in6.h>
 #include <linux/module.h>
 #include <linux/net.h>
 #include <linux/ipv6.h>
@@ -954,64 +955,56 @@ static void p9_fd_close(struct p9_client *client)
 	kfree(ts);
 }
 
-/*
- * stolen from NFS - maybe should be made a generic function?
- */
-static inline int valid_ipaddr4(const char *buf)
-{
-	int rc, count, in[4];
-
-	rc = sscanf(buf, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
-	if (rc != 4)
-		return -EINVAL;
-	for (count = 0; count < 4; count++) {
-		if (in[count] > 255)
-			return -EINVAL;
-	}
-	return 0;
-}
-
 static int p9_bind_privport(struct socket *sock)
 {
-	struct sockaddr_in cl;
+	struct sockaddr_storage stor = { 0 };
 	int port, err = -EINVAL;
 
-	memset(&cl, 0, sizeof(cl));
-	cl.sin_family = AF_INET;
-	cl.sin_addr.s_addr = htonl(INADDR_ANY);
+	stor.ss_family = sock->ops->family;
+	if (stor.ss_family == AF_INET)
+		((struct sockaddr_in *) &stor)->sin_addr.s_addr = htonl(INADDR_ANY);
+	else
+		((struct sockaddr_in6 *) &stor)->sin6_addr = in6addr_any;
 	for (port = p9_ipport_resv_max; port >= p9_ipport_resv_min; port--) {
-		cl.sin_port = htons((ushort)port);
-		err = kernel_bind(sock, (struct sockaddr *)&cl, sizeof(cl));
+		if (stor.ss_family == AF_INET)
+			((struct sockaddr_in *) &stor)->sin_port = htons((ushort)port);
+		else
+			((struct sockaddr_in6 *) &stor)->sin6_port = htons((ushort)port);
+		err = kernel_bind(sock, (struct sockaddr *) &stor, sizeof(stor));
 		if (err != -EADDRINUSE)
 			break;
 	}
 	return err;
 }
 
-
 static int
 p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args)
 {
 	int err;
+	char port_str[6];
 	struct socket *csocket;
-	struct sockaddr_in sin_server;
+	struct sockaddr_storage stor = { 0 };
 	struct p9_fd_opts opts;
 
 	err = parse_opts(args, &opts);
 	if (err < 0)
 		return err;
 
-	if (addr == NULL || valid_ipaddr4(addr) < 0)
+	if (addr == NULL)
 		return -EINVAL;
 
+	sprintf(port_str, "%u", opts.port);
+	err = inet_pton_with_scope(current->nsproxy->net_ns, AF_UNSPEC, addr,
+				   port_str, &stor);
+	if (err < 0) {
+		return err;
+	}
+
 	csocket = NULL;
 
 	client->trans_opts.tcp.port = opts.port;
 	client->trans_opts.tcp.privport = opts.privport;
-	sin_server.sin_family = AF_INET;
-	sin_server.sin_addr.s_addr = in_aton(addr);
-	sin_server.sin_port = htons(opts.port);
-	err = __sock_create(current->nsproxy->net_ns, PF_INET,
+	err = __sock_create(current->nsproxy->net_ns, stor.ss_family,
 			    SOCK_STREAM, IPPROTO_TCP, &csocket, 1);
 	if (err) {
 		pr_err("%s (%d): problem creating socket\n",
@@ -1030,8 +1023,8 @@ p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args)
 	}
 
 	err = READ_ONCE(csocket->ops)->connect(csocket,
-				    (struct sockaddr *)&sin_server,
-				    sizeof(struct sockaddr_in), 0);
+					       (struct sockaddr *) &stor,
+					       sizeof(stor), 0);
 	if (err < 0) {
 		pr_err("%s (%d): problem connecting socket to %s\n",
 		       __func__, task_pid_nr(current), addr);
-- 
2.48.1

Re: [PATCH v3] net/9p/fd: support ipv6 for trans=tcp

[asmadeus]

Joshua Murphy wrote on Sat, Jan 18, 2025 at 07:12:36PM +0000:
> Allows specifying an IPv6 address when mounting a remote 9p file system.
> 
> Signed-off-by: Joshua Murphy <joshuamurphy@posteo.net>

Thank you!

I don't have any problem with this, I need to find some time to take the
dust off my 9p tcp server setup to try this out.

Meanwhile I should have tried earlier but checkpatch has a few qualms
about style; if you send a v4 for any other reason please try running it
yourself.
--------
$ git show --format=email | ./scripts/checkpatch.pl -
CHECK: No space is necessary after a cast
#56: FILE: net/9p/trans_fd.c:965:
+               ((struct sockaddr_in *) &stor)->sin_addr.s_addr = htonl(INADDR_ANY);

CHECK: No space is necessary after a cast
#58: FILE: net/9p/trans_fd.c:967:
+               ((struct sockaddr_in6 *) &stor)->sin6_addr = in6addr_any;

CHECK: No space is necessary after a cast
#63: FILE: net/9p/trans_fd.c:970:
+                       ((struct sockaddr_in *) &stor)->sin_port = htons((ushort)port);

CHECK: No space is necessary after a cast
#65: FILE: net/9p/trans_fd.c:972:
+                       ((struct sockaddr_in6 *) &stor)->sin6_port = htons((ushort)port);

CHECK: No space is necessary after a cast
#66: FILE: net/9p/trans_fd.c:973:
+               err = kernel_bind(sock, (struct sockaddr *) &stor, sizeof(stor));

CHECK: Comparison to NULL could be written "!addr"
#89: FILE: net/9p/trans_fd.c:993:
+       if (addr == NULL)

WARNING: braces {} are not necessary for single statement blocks
#95: FILE: net/9p/trans_fd.c:999:
+       if (err < 0) {
+               return err;
+       }

CHECK: No space is necessary after a cast
#117: FILE: net/9p/trans_fd.c:1026:
+                                              (struct sockaddr *) &stor,
--------

(Some aren't new and I don't mind too much, but most are on new code)


I can't make any promise about testing but will try to find time around
next weekend, after which the patch will slowly make its way in next
cycle.
-- 
Dominique

Re: [PATCH v3] net/9p/fd: support ipv6 for trans=tcp

[asmadeus]

asmadeus@codewreck.org wrote on Mon, Jan 20, 2025 at 05:21:39PM +0900:
> Meanwhile I should have tried earlier but checkpatch has a few qualms
> about style; if you send a v4 for any other reason please try running it
> yourself.
> [..]

There's no value in having you send a v4 just for this so I went ahead
and just removed a couple of spaces in my tree;
you can find the commit here if you want to check:
https://github.com/martinetd/linux/commits/9p-next

> I can't make any promise about testing but will try to find time around
> next weekend, after which the patch will slowly make its way in next
> cycle.

And I found time for this sooner than I thought I would: mount worked
just fine, both in v4 and v6.

Since there is no obvious regression I've pushed the patch to my "next"
branch, which will be merged into linux-next shortly and I'll submit to
Linus next cycle -- we're in a merge window right now but I don't have
any other patch ready and this doesn't feel like it needs to be rushed
so it'll be in a couple of months.


FWIW, the "current" workaround that also works with any released kernel
is to mount as follow, with shells like bash that allow /dev/tcp this is
actually easy to script:
```
exec 3<>"/dev/tcp/$TCP_SERVER/$TCP_PORT"
mount -t 9p -o trans=fd,aname="$TCP_ANAME",rfdno=3,wfdno=3 none /mnt
exec 3<&-
```


Thank you again, this has been on my todo list for almost as long as I've
been cattering to this 9p client!

(and I'll conveniently ignore the fact that trans_rdma has the very same
problem, I haven't had access to hardware to test RDMA for years so
unless someone cares it's probably better to leave it untouched..)

-- 
Dominique Martinet | Asmadeus

Re: [PATCH v3] net/9p/fd: support ipv6 for trans=tcp

[Joshua Murphy]

> There's no value in having you send a v4 just for this so I went ahead
> and just removed a couple of spaces in my tree;
> you can find the commit here if you want to check:
> https://github.com/martinetd/linux/commits/9p-next

Thanks! I’ll know to use checkpatch from now on.

> Thank you again, this has been on my todo list for almost as long as I've
> been cattering to this 9p client!

No problem, it was a great learning experience.

I’ll also look into changing the port into a proper char * rather
than it being converted, soon.

-- 
Joshua Murphy
joshuamurphy@posteo.net