[-- Attachment #1: Type: text/plain, Size: 819 bytes --] The current code ignores the address as passed to the BSD socket function bind(). It seems that that may not have been possible with earlier versions of the net API? I discovered by reading the code and experimentation that this seems to work fine in the current net API, so this implements it for BSD sockets. I removed some previous code that did not make sense to me, so you may want to check that I did not screw up. What I mean is "bind *" in bind() and about "bind 0" in listen(). The handling when ports < 0 are specified is not consistent yet. Either we should normalize the ports to be at least 0, or we should return an error condition for ports < 0. The documentation in the man page ip(3) for the control commands "bind" and "announce" does not mention the local address, should that be fixed, too? [-- Attachment #2.1: Type: text/plain, Size: 316 bytes --] from postmaster@4ess: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: inline; filename=0004-ape-bsd-fix-listening-to-individual-addresses.patch [-- Attachment #2.2: 0004-ape-bsd-fix-listening-to-individual-addresses.patch.suspect --] [-- Type: application/octet-stream, Size: 3570 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Sat, 25 Dec 2021 14:03:00 +0000 Subject: [PATCH] ape/bsd: fix listening to individual addresses The former code does not bind to an address, just to a port. The net interface actually supports binding to addresses, so implement that in the BSD socket code. Also: Do not issue "bind *", that does not mean what you think it means. --- diff f96fae96efebf3eb7917e2c0b55439db2b242f79 4648260a1e4915035b09b502fd81d6716b7f77c0 --- a/sys/src/ape/lib/bsd/_sock_ingetaddr.c Sat Dec 25 14:59:59 2021 +++ b/sys/src/ape/lib/bsd/_sock_ingetaddr.c Sat Dec 25 15:03:00 2021 @@ -40,6 +40,29 @@ } int +_sock_inisany(int af, void *addr) +{ + int alen; + void *any; + static struct in_addr inaddr_any; + + switch(af){ + case AF_INET: + alen = sizeof inaddr_any.s_addr; + any = &inaddr_any; + break; + case AF_INET6: + alen = sizeof in6addr_any; + any = &in6addr_any; + break; + default: + return 0; + } + + return 0 == memcmp(addr, any, alen); +} + +int _sock_inaddr(int af, char *ip, char *port, void *a, int *alen) { int len; @@ -96,4 +119,24 @@ } close(fd); } +} + +char * +_sock_inaddr2string(Rock *r, char *dest, int dlen) +{ + int af = r->domain; + void *addr = _sock_inip(&r->addr); + int port = _sock_inport(&r->addr); + char *d = dest; + char *dend = dest+dlen; + + if(!_sock_inisany(af, addr)){ + inet_ntop(af, addr, d, dlen-1); + d = memchr(d, 0, dlen-1); + *(d++) = '!'; + } + + snprintf(d, dend-d, "%d", port); + + return dest; } --- a/sys/src/ape/lib/bsd/bind.c Sat Dec 25 14:59:59 2021 +++ b/sys/src/ape/lib/bsd/bind.c Sat Dec 25 15:03:00 2021 @@ -24,7 +24,7 @@ int bind(int fd, void *a, int alen) { - int n, len, cfd, port; + int n, len, cfd; struct sockaddr *sa; Rock *r; char msg[128]; @@ -55,20 +55,23 @@ errno = EBADF; return -1; } - port = _sock_inport(&r->addr); - if(port > 0) - snprintf(msg, sizeof msg, "bind %d", port); - else - strcpy(msg, "bind *"); + + strcpy(msg, "bind "); + _sock_inaddr2string(r, msg + 5, sizeof msg - 5); + n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ close(cfd); return -1; } + close(cfd); - if(port <= 0) + + if(_sock_inport(&r->addr) <= 0) _sock_ingetaddr(r, &r->addr, 0, "local"); return 0; } + + --- a/sys/src/ape/lib/bsd/listen.c Sat Dec 25 14:59:59 2021 +++ b/sys/src/ape/lib/bsd/listen.c Sat Dec 25 15:03:00 2021 @@ -121,7 +121,7 @@ int backlog; { Rock *r; - int n, cfd, port; + int n, cfd; char msg[128]; struct sockaddr_un *lunix; @@ -139,17 +139,9 @@ errno = EBADF; return -1; } - port = _sock_inport(&r->addr); - if(port >= 0) { - if(write(cfd, "bind 0", 6) < 0) { - errno = EGREG; - close(cfd); - return -1; - } - snprintf(msg, sizeof msg, "announce %d", port); - } - else - strcpy(msg, "announce *"); + strcpy(msg, "announce "); + _sock_inaddr2string(r, msg + 9, sizeof msg - 9); + n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ --- a/sys/src/ape/lib/bsd/priv.h Sat Dec 25 14:59:59 2021 +++ b/sys/src/ape/lib/bsd/priv.h Sat Dec 25 15:03:00 2021 @@ -43,5 +43,7 @@ extern int _sock_ipattr(char*); extern void* _sock_inip(struct sockaddr*); extern int _sock_inport(struct sockaddr*); +extern int _sock_inisany(int af, void *addr); extern int _sock_inaddr(int, char*, char*, void*, int*); extern void _sock_ingetaddr(Rock*, void*, int*, char*); +extern char * _sock_inaddr2string(Rock *r, char *dest, int dlen); -- 1.0
Ping? Nobody interested?
Quoth Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>:
> Ping?
>
> Nobody interested?
>
Thanks for the ping. I'll look at it
this weekend.
Hi all, This is still open. Ori, will you have time to review it? I appreciate that it is probably the most complicated of my patches. In contrast there is also still open "Re: ape/mkstemp: Set define (was: Include <bsd.h>" for a more simple change ;-) Thanks, benny
Apologies for taking so long to respond. Life got busy, and I forgot. Quoth Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>: > The current code ignores the address as passed to the BSD socket > function bind(). It seems that that may not have been possible with > earlier versions of the net API? I discovered by reading the code and > experimentation that this seems to work fine in the current net API, so > this implements it for BSD sockets. > > I removed some previous code that did not make sense to me, so you may > want to check that I did not screw up. What I mean is "bind *" in > bind() and about "bind 0" in listen(). > > The handling when ports < 0 are specified is not consistent yet. Either > we should normalize the ports to be at least 0, or we should return an > error condition for ports < 0. This looks like it's attempting (incorrectly?) to implement INADDR_ANY, binding to any ip address. However, INADDR_ANY is 0: /sys/include/ape/netinet/in.h:98: #define INADDR_ANY (unsigned long)0x00000000 And I see no specification that describes what should be done with negative ports in the sockets world, so I can only assume that this is a bug. I don't think 'bind *' should be removed, since it seems to me that it works as intended for binding to all addresses, but the ape userspace for it is wrong. > The documentation in the man page ip(3) for the control commands "bind" > and "announce" does not mention the local address, should that be fixed, > too? Yes, that would be wonderful.
Hi Ori, Ori writes: > Apologies for taking so long to respond. Life got busy, and I forgot. No problem. I often take my time myself ;-) Thanks for looking at it now. Note that this is some time ago for me now, too, so I may remember wrongly how and why I did things and I will have to redo the tests. > This looks like it's attempting (incorrectly?) to implement > INADDR_ANY, binding to any ip address. However, INADDR_ANY is 0: > > /sys/include/ape/netinet/in.h:98: > #define INADDR_ANY (unsigned long)0x00000000 As far as I can tell, using IP address INADDR_ANY and using port 0 to indicate that the IP stack should choose the actual number both work with the native control commands, so they do not need much special care here. > And I see no specification that describes what should be done with > negative ports in the sockets world, so I can only assume that this is > a bug. I/we should probably just make negative ports an error. > I don't think 'bind *' should be removed, since it seems to me that it > works as intended for binding to all addresses, but the ape userspace > for it is wrong. When I tried this a long time ago, when this was called it just returned errors. INADDR_ANY just works, I think, so there does not seem to be a reason for this special handling. I will have to create some test cases to see what works and what doesn't. >> The documentation in the man page ip(3) for the control commands >> "bind" and "announce" does not mention the local address, should that >> be fixed, too? > Yes, that would be wonderful. Another one for the todo list then ;-) Thanks, benny
Hi Ori, Benjamin Riefenstahl writes: > I often take my time myself ;-) Again this has taken some time, sorry. > I will have to create some test cases to see what works and what > doesn't. That's what I have done now, see <https://git.sr.ht/~cc_benny/test-socket/tree>. I append the results, a log from Linux as a reference, one with the current 9front (9front-front.log) and one with my patch (9front-patch.log). Next for me is refining the patch. > Ori writes: >> And I see no specification that describes what should be done with >> negative ports in the sockets world, so I can only assume that this is >> a bug. > > I/we should probably just make negative ports an error. Turns out negative ports do not occur with BSD sockets because the numbers get filtered through 'htons' which returns 'unsigned short', naturally. I will drop considerations of negative ports in my patch. Benjamin Riefenstahl writes: >>> The documentation in the man page ip(3) for the control commands >>> "bind" and "announce" does not mention the local address, should that >>> be fixed, too? I will include that in my next version. Thanks, benny
[-- Attachment #1: Type: text/plain, Size: 24 bytes --] The promise log files. [-- Attachment #2: linux.log --] [-- Type: application/octet-stream, Size: 4241 bytes --] ==== Tests for 127.0.0.1 ... Connect to a given port (127.0.0.1 1234) ... Connect to a given port (127.0.0.1 1234) ... success Connect to an auto-selected port (127.0.0.1 0) ... Connect to an auto-selected port (127.0.0.1 0) ... success Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... success Try a different IP (should fail) (127.0.0.1 1234 arrian 1234) ... Try a different IP (should fail) (127.0.0.1 1234 arrian 1234) ... success Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... success ==== Tests for ::1 ... Connect to a given port (::1 1234) ... Connect to a given port (::1 1234) ... success Connect to an auto-selected port (::1 0) ... Connect to an auto-selected port (::1 0) ... success Try a different port (should fail) (::1 0 ::1 123) ... Try a different port (should fail) (::1 0 ::1 123) ... success Try a different IP (should fail) (::1 1234 arrian 1234) ... Try a different IP (should fail) (::1 1234 arrian 1234) ... success Listen to any address (::0 1234 ::1 1234) ... Listen to any address (::0 1234 ::1 1234) ... success ==== Tests for 192.168.9.78 ... Connect to a given port (192.168.9.78 1234) ... Connect to a given port (192.168.9.78 1234) ... success Connect to an auto-selected port (192.168.9.78 0) ... Connect to an auto-selected port (192.168.9.78 0) ... success Try a different port (should fail) (192.168.9.78 0 192.168.9.78 123) ... Try a different port (should fail) (192.168.9.78 0 192.168.9.78 123) ... success Try a different IP (should fail) (192.168.9.78 1234 127.0.0.1 1234) ... Try a different IP (should fail) (192.168.9.78 1234 127.0.0.1 1234) ... success Listen to any address (0.0.0.0 1234 192.168.9.78 1234) ... Listen to any address (0.0.0.0 1234 192.168.9.78 1234) ... success ==== Tests for 2a06:8781:16:42:6bd8:4934:bb39:d972 ... Connect to a given port (2a06:8781:16:42:6bd8:4934:bb39:d972 1234) ... Connect to a given port (2a06:8781:16:42:6bd8:4934:bb39:d972 1234) ... success Connect to an auto-selected port (2a06:8781:16:42:6bd8:4934:bb39:d972 0) ... Connect to an auto-selected port (2a06:8781:16:42:6bd8:4934:bb39:d972 0) ... success Try a different port (should fail) (2a06:8781:16:42:6bd8:4934:bb39:d972 0 2a06:8781:16:42:6bd8:4934:bb39:d972 123) ... Try a different port (should fail) (2a06:8781:16:42:6bd8:4934:bb39:d972 0 2a06:8781:16:42:6bd8:4934:bb39:d972 123) ... success Try a different IP (should fail) (2a06:8781:16:42:6bd8:4934:bb39:d972 1234 127.0.0.1 1234) ... Try a different IP (should fail) (2a06:8781:16:42:6bd8:4934:bb39:d972 1234 127.0.0.1 1234) ... success Listen to any address (::0 1234 2a06:8781:16:42:6bd8:4934:bb39:d972 1234) ... Listen to any address (::0 1234 2a06:8781:16:42:6bd8:4934:bb39:d972 1234) ... success ==== Tests for 172.17.0.1 ... Connect to a given port (172.17.0.1 1234) ... Connect to a given port (172.17.0.1 1234) ... success Connect to an auto-selected port (172.17.0.1 0) ... Connect to an auto-selected port (172.17.0.1 0) ... success Try a different port (should fail) (172.17.0.1 0 172.17.0.1 123) ... Try a different port (should fail) (172.17.0.1 0 172.17.0.1 123) ... success Try a different IP (should fail) (172.17.0.1 1234 127.0.0.1 1234) ... Try a different IP (should fail) (172.17.0.1 1234 127.0.0.1 1234) ... success Listen to any address (0.0.0.0 1234 172.17.0.1 1234) ... Listen to any address (0.0.0.0 1234 172.17.0.1 1234) ... success ==== Tests for 192.168.122.1 ... Connect to a given port (192.168.122.1 1234) ... Connect to a given port (192.168.122.1 1234) ... success Connect to an auto-selected port (192.168.122.1 0) ... Connect to an auto-selected port (192.168.122.1 0) ... success Try a different port (should fail) (192.168.122.1 0 192.168.122.1 123) ... Try a different port (should fail) (192.168.122.1 0 192.168.122.1 123) ... success Try a different IP (should fail) (192.168.122.1 1234 127.0.0.1 1234) ... Try a different IP (should fail) (192.168.122.1 1234 127.0.0.1 1234) ... success Listen to any address (0.0.0.0 1234 192.168.122.1 1234) ... Listen to any address (0.0.0.0 1234 192.168.122.1 1234) ... success ==== Summary Errors: 0 of 30 [-- Attachment #3: 9front-front.log --] [-- Type: application/octet-stream, Size: 3162 bytes --] ==== Tests for ::1 ... Connect to a given port (::1 1234) ... Connect to a given port (::1 1234) ... success Connect to an auto-selected port (::1 0) ... Error: Server socket bind: OP not supported Connect to an auto-selected port (::1 0) ... error Try a different port (should fail) (::1 0 ::1 123) ... Error: Server socket bind: OP not supported Try a different port (should fail) (::1 0 ::1 123) ... success Try a different IP (should fail) (::1 1234 9front 1234) ... Try a different IP (should fail) (::1 1234 9front 1234) ... error Listen to any address (::0 1234 ::1 1234) ... Listen to any address (::0 1234 ::1 1234) ... success ==== Tests for 127.0.0.1 ... Connect to a given port (127.0.0.1 1234) ... Connect to a given port (127.0.0.1 1234) ... success Connect to an auto-selected port (127.0.0.1 0) ... Error: Server socket bind: OP not supported Connect to an auto-selected port (127.0.0.1 0) ... error Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... Error: Server socket bind: OP not supported Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... success Try a different IP (should fail) (127.0.0.1 1234 9front 1234) ... Try a different IP (should fail) (127.0.0.1 1234 9front 1234) ... error Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... success ==== Tests for 192.168.122.202 ... Connect to a given port (192.168.122.202 1234) ... Connect to a given port (192.168.122.202 1234) ... success Connect to an auto-selected port (192.168.122.202 0) ... Error: Server socket bind: OP not supported Connect to an auto-selected port (192.168.122.202 0) ... error Try a different port (should fail) (192.168.122.202 0 192.168.122.202 123) ... Error: Server socket bind: OP not supported Try a different port (should fail) (192.168.122.202 0 192.168.122.202 123) ... success Try a different IP (should fail) (192.168.122.202 1234 127.0.0.1 1234) ... Try a different IP (should fail) (192.168.122.202 1234 127.0.0.1 1234) ... error Listen to any address (0.0.0.0 1234 192.168.122.202 1234) ... Listen to any address (0.0.0.0 1234 192.168.122.202 1234) ... success ==== Tests for fe80::5054:ff:fe04:8095 ... Connect to a given port (fe80::5054:ff:fe04:8095 1234) ... Connect to a given port (fe80::5054:ff:fe04:8095 1234) ... success Connect to an auto-selected port (fe80::5054:ff:fe04:8095 0) ... Error: Server socket bind: OP not supported Connect to an auto-selected port (fe80::5054:ff:fe04:8095 0) ... error Try a different port (should fail) (fe80::5054:ff:fe04:8095 0 fe80::5054:ff:fe04:8095 123) ... Error: Server socket bind: OP not supported Try a different port (should fail) (fe80::5054:ff:fe04:8095 0 fe80::5054:ff:fe04:8095 123) ... success Try a different IP (should fail) (fe80::5054:ff:fe04:8095 1234 127.0.0.1 1234) ... Try a different IP (should fail) (fe80::5054:ff:fe04:8095 1234 127.0.0.1 1234) ... error Listen to any address (::0 1234 fe80::5054:ff:fe04:8095 1234) ... Listen to any address (::0 1234 fe80::5054:ff:fe04:8095 1234) ... success ==== Summary Errors: 8 of 20 0.07u 0.25s 20.43r ape/sh ./test-socket.psh # status= 1 [-- Attachment #4: 9front-patch.log --] [-- Type: application/octet-stream, Size: 2813 bytes --] ==== Tests for ::1 ... Connect to a given port (::1 1234) ... Connect to a given port (::1 1234) ... success Connect to an auto-selected port (::1 0) ... Connect to an auto-selected port (::1 0) ... success Try a different port (should fail) (::1 0 ::1 123) ... Try a different port (should fail) (::1 0 ::1 123) ... success Try a different IP (should fail) (::1 1234 9front 1234) ... Try a different IP (should fail) (::1 1234 9front 1234) ... success Listen to any address (::0 1234 ::1 1234) ... Listen to any address (::0 1234 ::1 1234) ... success ==== Tests for 127.0.0.1 ... Connect to a given port (127.0.0.1 1234) ... Connect to a given port (127.0.0.1 1234) ... success Connect to an auto-selected port (127.0.0.1 0) ... Connect to an auto-selected port (127.0.0.1 0) ... success Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... Try a different port (should fail) (127.0.0.1 0 127.0.0.1 123) ... success Try a different IP (should fail) (127.0.0.1 1234 9front 1234) ... Try a different IP (should fail) (127.0.0.1 1234 9front 1234) ... success Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... Listen to any address (0.0.0.0 1234 127.0.0.1 1234) ... success ==== Tests for 192.168.122.202 ... Connect to a given port (192.168.122.202 1234) ... Connect to a given port (192.168.122.202 1234) ... success Connect to an auto-selected port (192.168.122.202 0) ... Connect to an auto-selected port (192.168.122.202 0) ... success Try a different port (should fail) (192.168.122.202 0 192.168.122.202 123) ... Try a different port (should fail) (192.168.122.202 0 192.168.122.202 123) ... success Try a different IP (should fail) (192.168.122.202 1234 127.0.0.1 1234) ... Try a different IP (should fail) (192.168.122.202 1234 127.0.0.1 1234) ... success Listen to any address (0.0.0.0 1234 192.168.122.202 1234) ... Listen to any address (0.0.0.0 1234 192.168.122.202 1234) ... success ==== Tests for fe80::5054:ff:fe04:8095 ... Connect to a given port (fe80::5054:ff:fe04:8095 1234) ... Connect to a given port (fe80::5054:ff:fe04:8095 1234) ... success Connect to an auto-selected port (fe80::5054:ff:fe04:8095 0) ... Connect to an auto-selected port (fe80::5054:ff:fe04:8095 0) ... success Try a different port (should fail) (fe80::5054:ff:fe04:8095 0 fe80::5054:ff:fe04:8095 123) ... Try a different port (should fail) (fe80::5054:ff:fe04:8095 0 fe80::5054:ff:fe04:8095 123) ... success Try a different IP (should fail) (fe80::5054:ff:fe04:8095 1234 127.0.0.1 1234) ... Try a different IP (should fail) (fe80::5054:ff:fe04:8095 1234 127.0.0.1 1234) ... success Listen to any address (::0 1234 fe80::5054:ff:fe04:8095 1234) ... Listen to any address (::0 1234 fe80::5054:ff:fe04:8095 1234) ... success ==== Summary Errors: 0 of 20 0.05u 0.18s 20.70r ape/sh ./test-socket.psh
Quoth Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>:
>
> That's what I have done now, see
> <https://git.sr.ht/~cc_benny/test-socket/tree>.
awesome, thanks for taking the time!
401 unauthorized -- mind granting me access?
I'd also like to add it to the regress repo, if
you're ok with it (and it makes sense)
Hi Ori, > Quoth Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>: >> That's what I have done now, see >> <https://git.sr.ht/~cc_benny/test-socket/tree>. Ori writes: > awesome, thanks for taking the time! > > 401 unauthorized -- mind granting me access? Sorry, that repo should have been public. > I'd also like to add it to the regress repo, if you're ok with it (and > it makes sense) Sure, go ahead. benny
[-- Attachment #1: Type: text/plain, Size: 881 bytes --] Hi Ori, all, Benjamin Riefenstahl writes: > Next for me is refining the patch. Attached the changes that I propose, broken down as far as I can make it. f0663be649266240196268bc8d157881ec6feafd ape/bsd/bind.c: Do not issue "bind *". e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf ape/bsd/listen.c: Do not try to issue "announce *". 8714cece801fa275d804308911631e2c54988fbd ape/bsd/bind.c, ape/bsd/listen.c: Set local IP. 45126352dac3cc5020538ae4dadd6b901737c4c7 ape/bsd/bind.c: Do not pretend that the port could be negative. 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 ape/bsd/listen.c: Drop "bind 0". 7990e340626b85606111b4898c0287b93faa492d ape/bsd/bind.c, ape/bsd/listen.c: Use _syserrno. cd8c4892cbd302a346078483175fe78a58ebe178 man/3/ip (ip): Discuss local IP addresses for "announce" and "bind". Please tell what should be changed or if something is unclear. so long, benny [-- Attachment #2.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=1-f0663be649266240196268bc8d157881ec6feafd.patch [-- Attachment #2.2: 1-f0663be649266240196268bc8d157881ec6feafd.patch.suspect --] [-- Type: application/octet-stream, Size: 880 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Fri, 10 Jun 2022 16:02:13 +0000 Subject: [PATCH] ape/bsd/bind.c: Do not issue "bind *". Despite the documentation, "bind *" is invalid and gives the error "bad numeric port" in devip.c:setladdrport. "bind 0" OTOH is actually supported fine in the Plan9 API and has the right sematics. --- diff 786ed6d38e1ce926ee6718b7eb4298886234714f f0663be649266240196268bc8d157881ec6feafd --- a/sys/src/ape/lib/bsd/bind.c Thu Feb 10 18:53:08 2022 +++ b/sys/src/ape/lib/bsd/bind.c Fri Jun 10 18:02:13 2022 @@ -56,10 +56,7 @@ return -1; } port = _sock_inport(&r->addr); - if(port > 0) - snprintf(msg, sizeof msg, "bind %d", port); - else - strcpy(msg, "bind *"); + snprintf(msg, sizeof msg, "bind %d", port); n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ [-- Attachment #3.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=2-e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf.patch [-- Attachment #3.2: 2-e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf.patch.suspect --] [-- Type: application/octet-stream, Size: 1375 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Fri, 10 Jun 2022 20:22:39 +0000 Subject: [PATCH] ape/bsd/listen.c: Do not try to issue "announce *". "port >= 0" is always true, because the port always gets filtered through "htons" which returns "unsigned short", so we can just drop the "else" branch here. Anyway "announce 0" works fine with the Plan9 API, there is not need for "announce *" here. --- diff f0663be649266240196268bc8d157881ec6feafd e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf --- a/sys/src/ape/lib/bsd/listen.c Fri Jun 10 18:02:13 2022 +++ b/sys/src/ape/lib/bsd/listen.c Fri Jun 10 22:22:39 2022 @@ -121,7 +121,7 @@ int backlog; { Rock *r; - int n, cfd, port; + int n, cfd; char msg[128]; struct sockaddr_un *lunix; @@ -139,17 +139,13 @@ errno = EBADF; return -1; } - port = _sock_inport(&r->addr); - if(port >= 0) { - if(write(cfd, "bind 0", 6) < 0) { - errno = EGREG; - close(cfd); - return -1; - } - snprintf(msg, sizeof msg, "announce %d", port); + /* FIXME: What is this good for? */ + if(write(cfd, "bind 0", 6) < 0) { + errno = EGREG; + close(cfd); + return -1; } - else - strcpy(msg, "announce *"); + snprintf(msg, sizeof msg, "announce %d", _sock_inport(&r->addr)); n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ [-- Attachment #4.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=3-8714cece801fa275d804308911631e2c54988fbd.patch [-- Attachment #4.2: 3-8714cece801fa275d804308911631e2c54988fbd.patch.suspect --] [-- Type: application/octet-stream, Size: 3104 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Fri, 10 Jun 2022 20:39:28 +0000 Subject: [PATCH] ape/bsd/bind.c, ape/bsd/listen.c: Set local IP. Despite what the man pages say, local addresses can actually be set so do that. --- diff e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf 8714cece801fa275d804308911631e2c54988fbd --- a/sys/src/ape/lib/bsd/_sock_ingetaddr.c Fri Jun 10 22:22:39 2022 +++ b/sys/src/ape/lib/bsd/_sock_ingetaddr.c Fri Jun 10 22:39:28 2022 @@ -40,6 +40,30 @@ } int +_sock_inisany(int af, void *addr) +{ + int alen; + void *any; + /* an IPv4 address that is auto-initialized to all zeros */ + static struct in_addr inaddr_any; + + switch(af){ + case AF_INET: + alen = sizeof inaddr_any.s_addr; + any = &inaddr_any; + break; + case AF_INET6: + alen = sizeof in6addr_any; + any = &in6addr_any; + break; + default: + return 0; + } + + return 0 == memcmp(addr, any, alen); +} + +int _sock_inaddr(int af, char *ip, char *port, void *a, int *alen) { int len; @@ -96,4 +120,24 @@ } close(fd); } +} + +char * +_sock_inaddr2string(Rock *r, char *dest, int dlen) +{ + int af = r->domain; + void *addr = _sock_inip(&r->addr); + int port = _sock_inport(&r->addr); + char *d = dest; + char *dend = dest+dlen; + + if(!_sock_inisany(af, addr)){ + inet_ntop(af, addr, d, dlen-1); + d = memchr(d, 0, dlen-1); + *(d++) = '!'; + } + + snprintf(d, dend-d, "%d", port); + + return dest; } --- a/sys/src/ape/lib/bsd/bind.c Fri Jun 10 22:22:39 2022 +++ b/sys/src/ape/lib/bsd/bind.c Fri Jun 10 22:39:28 2022 @@ -24,7 +24,7 @@ int bind(int fd, void *a, int alen) { - int n, len, cfd, port; + int n, len, cfd; struct sockaddr *sa; Rock *r; char msg[128]; @@ -55,17 +55,23 @@ errno = EBADF; return -1; } - port = _sock_inport(&r->addr); - snprintf(msg, sizeof msg, "bind %d", port); + + strcpy(msg, "bind "); + _sock_inaddr2string(r, msg + 5, sizeof msg - 5); + n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ close(cfd); return -1; } + close(cfd); - if(port <= 0) + + if(_sock_inport(&r->addr) <= 0) _sock_ingetaddr(r, &r->addr, 0, "local"); return 0; } + + --- a/sys/src/ape/lib/bsd/listen.c Fri Jun 10 22:22:39 2022 +++ b/sys/src/ape/lib/bsd/listen.c Fri Jun 10 22:39:28 2022 @@ -145,7 +145,8 @@ close(cfd); return -1; } - snprintf(msg, sizeof msg, "announce %d", _sock_inport(&r->addr)); + strcpy(msg, "announce "); + _sock_inaddr2string(r, msg + 9, sizeof msg - 9); n = write(cfd, msg, strlen(msg)); if(n < 0){ errno = EOPNOTSUPP; /* Improve error reporting!!! */ --- a/sys/src/ape/lib/bsd/priv.h Fri Jun 10 22:22:39 2022 +++ b/sys/src/ape/lib/bsd/priv.h Fri Jun 10 22:39:28 2022 @@ -43,5 +43,7 @@ extern int _sock_ipattr(char*); extern void* _sock_inip(struct sockaddr*); extern int _sock_inport(struct sockaddr*); +extern int _sock_inisany(int af, void *addr); extern int _sock_inaddr(int, char*, char*, void*, int*); extern void _sock_ingetaddr(Rock*, void*, int*, char*); +extern char* _sock_inaddr2string(Rock *r, char *dest, int dlen); [-- Attachment #5.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=4-45126352dac3cc5020538ae4dadd6b901737c4c7.patch [-- Attachment #5.2: 4-45126352dac3cc5020538ae4dadd6b901737c4c7.patch.suspect --] [-- Type: application/octet-stream, Size: 624 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Sat, 11 Jun 2022 12:34:31 +0000 Subject: [PATCH] ape/bsd/bind.c: Do not pretend that the port could be negative. Ports are filtered through "htons" which returns an "unsigned short". --- diff 8714cece801fa275d804308911631e2c54988fbd 45126352dac3cc5020538ae4dadd6b901737c4c7 --- a/sys/src/ape/lib/bsd/bind.c Fri Jun 10 22:39:28 2022 +++ b/sys/src/ape/lib/bsd/bind.c Sat Jun 11 14:34:31 2022 @@ -68,7 +68,7 @@ close(cfd); - if(_sock_inport(&r->addr) <= 0) + if(_sock_inport(&r->addr) == 0) _sock_ingetaddr(r, &r->addr, 0, "local"); return 0; [-- Attachment #6.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=5-7ace52e5b1637bb4d89e243f14e47d0a1ece5f82.patch [-- Attachment #6.2: 5-7ace52e5b1637bb4d89e243f14e47d0a1ece5f82.patch.suspect --] [-- Type: application/octet-stream, Size: 773 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Wed, 06 Jul 2022 03:38:17 +0000 Subject: [PATCH] ape/bsd/listen.c: Drop "bind 0". There does not seem to be a good reason for this. The "bind" command has no practical consequence. --- diff 45126352dac3cc5020538ae4dadd6b901737c4c7 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 --- a/sys/src/ape/lib/bsd/listen.c Sat Jun 11 14:34:31 2022 +++ b/sys/src/ape/lib/bsd/listen.c Wed Jul 6 05:38:17 2022 @@ -139,12 +139,6 @@ errno = EBADF; return -1; } - /* FIXME: What is this good for? */ - if(write(cfd, "bind 0", 6) < 0) { - errno = EGREG; - close(cfd); - return -1; - } strcpy(msg, "announce "); _sock_inaddr2string(r, msg + 9, sizeof msg - 9); n = write(cfd, msg, strlen(msg)); [-- Attachment #7.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=6-7990e340626b85606111b4898c0287b93faa492d.patch [-- Attachment #7.2: 6-7990e340626b85606111b4898c0287b93faa492d.patch.suspect --] [-- Type: application/octet-stream, Size: 998 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Wed, 06 Jul 2022 03:46:07 +0000 Subject: [PATCH] ape/bsd/bind.c, ape/bsd/listen.c: Use _syserrno. --- diff 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 7990e340626b85606111b4898c0287b93faa492d --- a/sys/src/ape/lib/bsd/bind.c Wed Jul 6 05:38:17 2022 +++ b/sys/src/ape/lib/bsd/bind.c Wed Jul 6 05:46:07 2022 @@ -61,7 +61,9 @@ n = write(cfd, msg, strlen(msg)); if(n < 0){ - errno = EOPNOTSUPP; /* Improve error reporting!!! */ + _syserrno(); + if(errno == EPLAN9) + errno = EOPNOTSUPP; close(cfd); return -1; } --- a/sys/src/ape/lib/bsd/listen.c Wed Jul 6 05:38:17 2022 +++ b/sys/src/ape/lib/bsd/listen.c Wed Jul 6 05:46:07 2022 @@ -143,7 +143,9 @@ _sock_inaddr2string(r, msg + 9, sizeof msg - 9); n = write(cfd, msg, strlen(msg)); if(n < 0){ - errno = EOPNOTSUPP; /* Improve error reporting!!! */ + _syserrno(); + if(errno == EPLAN9) + errno = EOPNOTSUPP; close(cfd); return -1; } [-- Attachment #8.1: Type: text/plain, Size: 314 bytes --] from postmaster@9front: The following attachment had content that we can't prove to be harmless. To avoid possible automatic execution, we changed the content headers. The original header was: Content-Type: text/x-diff Content-Disposition: attachment; filename=7-cd8c4892cbd302a346078483175fe78a58ebe178.patch [-- Attachment #8.2: 7-cd8c4892cbd302a346078483175fe78a58ebe178.patch.suspect --] [-- Type: application/octet-stream, Size: 2304 bytes --] From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net> Date: Sat, 11 Jun 2022 13:00:05 +0000 Subject: [PATCH] man/3/ip (ip): Discuss local IP addresses for "announce" and "bind". --- diff 7990e340626b85606111b4898c0287b93faa492d cd8c4892cbd302a346078483175fe78a58ebe178 --- a/sys/man/3/ip Wed Jul 6 05:46:07 2022 +++ b/sys/man/3/ip Sat Jun 11 15:00:05 2022 @@ -682,33 +682,43 @@ The connect fails if the combination of local and remote address/port pairs are already assigned to another port. .TP -.BI announce\ X -.I X -is a decimal port number or -.LR * . -Set the local port -number to -.I X -and accept calls to -.IR X . -If -.I X -is +.BI announce\ [ip-address ! ]port +Set the local IP address and port number and accept calls there. If +ip-address is left out, accept calls on any address. If port is 0, a +port is automatically choosen that is not yet announced. +If the address is .LR * , -accept -calls for any port that no process has explicitly announced. -The local IP address cannot be set. +accept calls on any address. +If port is +.LR * , +accept calls on any port. +If port is +.LR * , +and the address is left out, accept calls on any address and port. .B Announce -fails if the connection is already announced or connected. +fails if the connection is already announced. .TP -.BI bind\ X -.I X -is a decimal port number or -.LR * . -Set the local port number to -.IR X . -This exists to support emulation -of BSD sockets by the APE libraries (see +.BI bind\ [ip-address ! ]port +Set the local IP address and port number like for a server connection +similar to the +.B announce +command. +If ip-address is left out, an address is automatically selected. If +port is 0, a port is automatically choosen that is not yet announced. +This command has no actual effect, beyond remembering the parameters +and possibly selecting an unused port. +The commands +.B announce +and +.B connect +reset both the local address and the port according to their own +parameters. +This command also does +.B not +reserve the IP address and port, another connection can use them, even +while they are registered in a connection by this command. +This exists to support emulation of BSD sockets by the APE libraries +(see .IR pcc (1)) and is not otherwise used. .\" this is gone
Quoth Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>: > Hi Ori, all, > > Benjamin Riefenstahl writes: > > Next for me is refining the patch. > > Attached the changes that I propose, broken down as far as I can make it. Thank you -- they look good to me; accepted all of them as-is. > f0663be649266240196268bc8d157881ec6feafd ape/bsd/bind.c: Do not issue "bind *". > e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf ape/bsd/listen.c: Do not try to issue "announce *". > 8714cece801fa275d804308911631e2c54988fbd ape/bsd/bind.c, ape/bsd/listen.c: Set local IP. > 45126352dac3cc5020538ae4dadd6b901737c4c7 ape/bsd/bind.c: Do not pretend that the port could be negative. > 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 ape/bsd/listen.c: Drop "bind 0". > 7990e340626b85606111b4898c0287b93faa492d ape/bsd/bind.c, ape/bsd/listen.c: Use _syserrno. > cd8c4892cbd302a346078483175fe78a58ebe178 man/3/ip (ip): Discuss local IP addresses for "announce" and "bind". > > Please tell what should be changed or if something is unclear. > > so long, benny > >
Ori writes:
> Thank you -- they look good to me; accepted all of them as-is.
Thanks, that's cool!
benny