| Message ID | 20250118192122.327-2-joshuamurphy@posteo.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] net/9p/fd: support ipv6 for trans=tcp | expand |
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.
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..)
> 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.
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);
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(-)