diff --git a/cmd/nettest/main.c b/cmd/nettest/main.c index a7f565b..ab6b884 100644 --- a/cmd/nettest/main.c +++ b/cmd/nettest/main.c @@ -6,13 +6,21 @@ // exercises ARP, IP, the TCP handshake, the send and receive paths, the window // and the close, in one exchange. // -// nettest +// nettest [-u] // // Under QEMU's user-mode networking the host is 10.0.2.2, so pointing this at a // listener there is the intended use: // // host$ printf 'HELLO-FROM-HOST\n' | nc -l -p 9000 // guest# nettest 10.0.2.2 9000 +// +// -u does the same exchange over UDP, and checks the one thing TCP cannot: that +// recvfrom reports who sent the datagram. On a connected stream the source is +// known before the first byte arrives, so a kernel that filled the address in +// with anything at all would look correct there. +// +// host$ nc -u -l -p 9000 +// guest# nettest -u 10.0.2.2 9000 #include "types.h" #include "user.h" @@ -80,11 +88,110 @@ parse_port(const char *s) return (*s == '\0' && v > 0 && v < 65536) ? v : -1; } +// The UDP half. One datagram out, one back, and the source of the reply +// printed -- which is the part that has no equivalent over TCP. +static void +udp_exchange(uint ip, int port, const char *ipstr) +{ + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if (fd < 0) { + puts_("nettest: udp socket failed\n"); + return; + } + + struct sockaddr_in to; + for (int i = 0; i < (int)sizeof(to); i++) + ((char *)&to)[i] = 0; + to.sin_family = AF_INET; + to.sin_port = hton16((ushort)port); + to.sin_addr = ip; + + const char *msg = "HELLO-UDP-FROM-SCHOS\n"; + int len = 0; + while (msg[len]) len++; + + puts_("nettest: udp sending to "); puts_(ipstr); + puts_(":"); putn_(port); puts_("\n"); + + if (sendto(fd, msg, len, 0, (const struct sockaddr *)&to, sizeof(to)) != len) { + puts_("nettest: udp send failed\n"); + close(fd); + return; + } + + // Deliberately not zeroed first. If the kernel does not fill this in, the + // address printed is whatever was on the stack, and a test that asserts on + // 10.0.2.2 catches that -- where a zeroed struct would quietly read as + // 0.0.0.0 and look like a mere formatting problem. + struct sockaddr_in from; + int fromlen = sizeof(from); + char buf[512]; + + int n = recvfrom(fd, buf, sizeof(buf) - 1, 0, + (struct sockaddr *)&from, &fromlen); + if (n <= 0) { + puts_("nettest: udp FAILED, nothing came back\n"); + close(fd); + return; + } + buf[n] = '\0'; + + puts_("nettest: udp received "); putn_(n); puts_(" bytes: "); + puts_(buf); + if (buf[0] && buf[n - 1] != '\n') + puts_("\n"); + + if (fromlen < (int)sizeof(from)) { + puts_("nettest: udp FAILED, no source address returned\n"); + close(fd); + return; + } + + // Network order, most significant byte first, which is how parse_ip built + // it -- so the same shifts unpack it. + uint a = from.sin_addr; + puts_("nettest: udp source "); + putn_((int)(a & 0xFF)); puts_("."); + putn_((int)((a >> 8) & 0xFF)); puts_("."); + putn_((int)((a >> 16) & 0xFF));puts_("."); + putn_((int)((a >> 24) & 0xFF)); + puts_(":"); + putn_((int)hton16(from.sin_port)); + puts_("\n"); + + // A second datagram, read separately, because message boundaries are the + // whole difference between UDP and TCP. The responder sends two; if the + // kernel kept them as a byte stream the first read would have swallowed + // both, and the byte counts printed above and below are what says which + // happened. + int m = recvfrom(fd, buf, sizeof(buf) - 1, 0, (struct sockaddr *)&from, + &fromlen); + close(fd); + if (m <= 0) { + puts_("nettest: udp FAILED, no second datagram\n"); + return; + } + buf[m] = '\0'; + puts_("nettest: udp second "); putn_(m); puts_(" bytes: "); + puts_(buf); + if (buf[0] && buf[m - 1] != '\n') + puts_("\n"); + + puts_("nettest: udp OK\n"); +} + int main(int argc, char *argv[]) { + int udp = 0; + + if (argc == 4 && argv[1][0] == '-' && argv[1][1] == 'u' && argv[1][2] == '\0') { + udp = 1; + argv++; + argc--; + } if (argc != 3) { - puts_("usage: nettest \n"); + puts_("usage: nettest [-u] \n"); exit(); } @@ -95,6 +202,11 @@ main(int argc, char *argv[]) exit(); } + if (udp) { + udp_exchange(ip, port, argv[1]); + exit(); + } + int fd = socket(AF_INET, SOCK_STREAM, 0); if (fd < 0) { puts_("nettest: socket failed\n"); diff --git a/include/defs.h b/include/defs.h index 1079024..bef234c 100644 --- a/include/defs.h +++ b/include/defs.h @@ -381,6 +381,10 @@ int sockpair(struct socket **a, struct socket **b); const char* sockname(struct socket*); // AF_INET addressing. Separate from sockbind/sockconnect because the families // do not name the same kind of thing; `addr` and `port` are network order. +int sockrecvfrom(struct socket*, char *buf, int n, + uint *addr, ushort *port); +int socksendto(struct socket*, char *buf, int n, + uint addr, ushort port); int sockbind_in(struct socket*, uint addr, ushort port); int sockconnect_in(struct socket*, uint addr, ushort port); int sock_domain(struct socket*); @@ -395,6 +399,10 @@ void* inet_accept(void *h); int inet_connect(void *h, uint addr, ushort port); int inet_read(void *h, char *buf, int n); int inet_write(void *h, char *buf, int n); +int inet_recvfrom(void *h, char *buf, int n, + uint *addr, ushort *port); +int inet_sendto(void *h, char *buf, int n, + uint addr, ushort port); int inet_readable(void *h); int inet_hup(void *h); void inet_close(void *h); diff --git a/kernel/net/inet.c b/kernel/net/inet.c index 67233ff..df64710 100644 --- a/kernel/net/inet.c +++ b/kernel/net/inet.c @@ -67,6 +67,19 @@ #define NINET 16 // AF_INET sockets in the system #define INET_RXBUF 8192 // per socket; one lwIP window plus slack +#define NDGRAM 16 // datagrams queued on a UDP socket + +// Where one datagram ends and the next begins, and who sent it. +// +// The bytes themselves stay in the same ring TCP uses -- a datagram queue of +// whole buffers would be 16 sockets x 16 datagrams x an MTU of static memory +// for something that is nearly always empty. Only the boundaries and the +// source need to be kept alongside, and those are eight bytes each. +struct dgram { + uint addr; // source, network byte order + ushort port; // source, host byte order + ushort len; +}; struct inet_sock { int inuse; @@ -81,6 +94,11 @@ struct inet_sock { char rx[INET_RXBUF]; uint rhead, rtail; // rhead == rtail is empty + // UDP only: the boundaries within that ring. A stream socket has none -- + // that is the whole difference between the two -- so these stay at zero. + struct dgram dq[NDGRAM]; + uint dhead, dtail; // dhead == dtail is empty + int connecting; // connect() issued, no answer yet int connected; int conn_failed; @@ -179,6 +197,53 @@ rx_take(struct inet_sock *is, char *p, uint n) return n; } +// Throw bytes away without copying them. A datagram larger than the buffer it +// was read into is truncated, and the remainder is discarded rather than left +// to be delivered as if it were the next datagram. +static void +rx_drop(struct inet_sock *is, uint n) +{ + uint have = rx_used(is); + is->rtail += n > have ? have : n; +} + +/* ------------------------------------------------------------------ */ +/* Datagram boundaries. UDP only; callers hold the lwIP lock. */ +/* ------------------------------------------------------------------ */ + +static uint +dq_used(struct inet_sock *is) +{ + return is->dhead - is->dtail; +} + +// Take one datagram, whole. Returns the bytes copied, or -1 if none is queued. +// +// Whole is the point: read() on a UDP socket returns one datagram or nothing, +// never the tail of one followed by the head of the next. Before this queue +// existed the bytes went into the ring unmarked, so two datagrams that arrived +// together came back as one -- which is a bug that only appears when the +// timing is right, and is then blamed on the sender. +static int +dq_take(struct inet_sock *is, char *p, uint n, uint *addr, ushort *port) +{ + if (dq_used(is) == 0) + return -1; + + struct dgram *d = &is->dq[is->dtail % NDGRAM]; + uint want = d->len; + uint got = want < n ? want : n; + + rx_take(is, p, got); + rx_drop(is, want - got); + + if (addr) *addr = d->addr; + if (port) *port = d->port; + + is->dtail++; + return (int)got; +} + /* ------------------------------------------------------------------ */ /* lwIP callbacks. All of these run in the network thread, holding the */ /* lwIP lock. */ @@ -299,23 +364,37 @@ inet_accept_cb(void *arg, struct tcp_pcb *newpcb, err_t err) return ERR_OK; } -// UDP delivers datagrams with no connection. The source is dropped on the -// floor for now: recvfrom is not wired, and inventing a per-datagram source -// queue that nothing reads would be storage with no reader. +// UDP delivers datagrams with no connection, so each one carries its own +// sender. Both the sender and the length are recorded next to the bytes, which +// is what lets recvfrom answer "who sent this" and read() return exactly one +// datagram. static void inet_udp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port) { struct inet_sock *is = arg; - (void)pcb; (void)addr; (void)port; + (void)pcb; if (is == 0 || p == NULL) { if (p) pbuf_free(p); return; } - if (p->tot_len <= rx_space(is)) + + // Two ways to be full, and both mean the same thing for UDP: the datagram is + // dropped. That is the protocol's own contract -- there is no window to + // close and nothing to retransmit -- but it must be dropped *whole*, because + // half a datagram in the ring would corrupt every one after it. + if (p->tot_len <= rx_space(is) && dq_used(is) < NDGRAM) { + struct dgram *d = &is->dq[is->dhead % NDGRAM]; + d->addr = addr ? ip4_addr_get_u32(ip_2_ip4(addr)) : 0; + d->port = port; + d->len = p->tot_len; + is->dhead++; + for (struct pbuf *q = p; q != NULL; q = q->next) rx_put(is, (const char *)q->payload, q->len); + } + pbuf_free(p); net_wake(); } @@ -498,8 +577,15 @@ inet_connect(void *h, uint32 addr, uint16 port) } } +// Read, and say where it came from. +// +// `addr` and `port` may be null, which is what read() and recv() pass: the +// source is then simply not reported. For a stream socket the answer is the +// peer it is connected to and is the same for every byte; for a datagram +// socket it belongs to the individual datagram, which is why UDP needs the +// queue above and TCP does not. int -inet_read(void *h, char *buf, int n) +inet_recvfrom(void *h, char *buf, int n, uint *addr, ushort *port) { struct inet_sock *is = h; @@ -507,14 +593,36 @@ inet_read(void *h, char *buf, int n) return 0; for (;;) { + int got = 0; + int have = 0; // something was delivered, even if it was zero bytes + int eof; + uint seen; + lwip_sys_lock(); - uint got = rx_take(is, buf, (uint)n); - int eof = is->peer_closed; - uint seen = net_seq(); + if (is->type == SOCK_DGRAM) { + // A zero-length datagram is a real thing a sender can send, and the + // right answer to it is 0 rather than "nothing arrived yet" -- which is + // why dq_take reports an empty queue as -1 and not as zero bytes. + int r = dq_take(is, buf, (uint)n, addr, port); + have = r >= 0; + got = r > 0 ? r : 0; + } else { + got = (int)rx_take(is, buf, (uint)n); + have = got > 0; + // A stream's source is the peer it is connected to: the same answer for + // every byte, which is why it is read from the pcb rather than stored. + if (have && addr && is->tpcb) { + *addr = ip4_addr_get_u32(ip_2_ip4(&is->tpcb->remote_ip)); + if (port) + *port = is->tpcb->remote_port; + } + } + eof = is->peer_closed; + seen = net_seq(); lwip_sys_unlock(); - if (got > 0) - return (int)got; + if (have) + return got; if (eof) return 0; // end of file, not an error if (myproc()->killed) @@ -524,7 +632,20 @@ inet_read(void *h, char *buf, int n) } int -inet_write(void *h, char *buf, int n) +inet_read(void *h, char *buf, int n) +{ + return inet_recvfrom(h, buf, n, 0, 0); +} + +// Write, optionally to a named destination. +// +// `addr` of zero means "wherever this socket is already pointed", which is what +// write() and send() pass. For TCP that is the only possibility and a +// destination is ignored rather than refused: the connection decides where the +// bytes go, and a program that passes one anyway -- many do, out of habit -- is +// better served by having it work. +int +inet_sendto(void *h, char *buf, int n, uint addr, ushort port) { struct inet_sock *is = h; int done = 0; @@ -540,10 +661,23 @@ inet_write(void *h, char *buf, int n) return -ENOBUFS; } pbuf_take(p, buf, (u16_t)n); - err_t e = udp_send(is->upcb, p); + + err_t e; + if (addr) { + ip_addr_t ip; + ip_addr_set_ip4_u32(&ip, addr); + e = udp_sendto(is->upcb, p, &ip, port); + } else { + // No destination and none remembered is a program error, not a network + // one: lwIP would send to 0.0.0.0 and the datagram would vanish. + e = udp_send(is->upcb, p); + } + pbuf_free(p); lwip_sys_unlock(); - return e == ERR_OK ? n : -EIO; + if (e == ERR_OK) + return n; + return e == ERR_RTE ? -EDESTADDRREQ : -EIO; } // TCP: the send buffer is finite, so a large write becomes several passes. @@ -592,6 +726,12 @@ inet_write(void *h, char *buf, int n) return done; } +int +inet_write(void *h, char *buf, int n) +{ + return inet_sendto(h, buf, n, 0, 0); +} + int inet_readable(void *h) { diff --git a/kernel/net/unix.c b/kernel/net/unix.c index 211539a..781309e 100644 --- a/kernel/net/unix.c +++ b/kernel/net/unix.c @@ -552,3 +552,38 @@ sock_domain(struct socket *s) { return s ? s->domain : 0; } + +// recvfrom and sendto, which differ from read and write only in carrying an +// address. Both live here rather than in sys_net.c for the same reason as +// sock_domain: the syscall layer must not know which side of the socket union +// is live. +// +// `port` comes back in host byte order, matching what sockbind_in and +// sockconnect_in take. The syscall layer converts on the way out, in the one +// place that also builds the sockaddr. +int +sockrecvfrom(struct socket *s, char *buf, int n, uint *addr, ushort *port) +{ + if(n < 0) + return -EINVAL; + if(s->domain == AF_INET) + return inet_recvfrom(s->inet, buf, n, addr, port); + + // A Unix socket has no address of this shape. Saying so by leaving the + // caller's address untouched, rather than inventing one, is what lets + // recvfrom report an address length of zero -- which is exactly what a + // program reading from an unnamed peer expects. + return sockread(s, buf, n); +} + +int +socksendto(struct socket *s, char *buf, int n, uint addr, ushort port) +{ + if(n < 0) + return -EINVAL; + if(s->domain == AF_INET) + return inet_sendto(s->inet, buf, n, addr, port); + if(addr) + return -EAFNOSUPPORT; // an AF_INET destination on an AF_UNIX socket + return sockwrite(s, buf, n); +} diff --git a/kernel/sys/sys_net.c b/kernel/sys/sys_net.c index 48a3500..bbc5750 100644 --- a/kernel/sys/sys_net.c +++ b/kernel/sys/sys_net.c @@ -317,47 +317,104 @@ SYSCALL_DEFINE(socketpair) return 0; } -// send/recv with no flags are read/write. Implementing them here rather than -// leaving them ENOSYS costs four lines and saves every ported program from -// having to be rewritten to use write(). +// Host byte order to network, for a port on its way back to userland. The +// inverse of port_from_net above, and the same two lines: a sockaddr always +// carries the port big-endian, whichever direction it is travelling. +static ushort +port_to_net(ushort p) +{ + return (ushort)(((p & 0xFF) << 8) | ((p >> 8) & 0xFF)); +} + +// send/recv with no flags are read/write, and sendto/recvfrom are those plus an +// address. Implementing them here rather than leaving them ENOSYS saves every +// ported program from being rewritten to use read() and write(). SYSCALL_DEFINE(sendto) { struct socket *s; - addr_t ubuf; - int len, flags, r; + addr_t ubuf, uaddr; + int len, flags, ulen, r; + uint ip = 0; + ushort port = 0; if ((r = argsock(0, &s)) < 0) return r; argaddr(1, &ubuf); argint(2, &len); argint(3, &flags); + argaddr(4, &uaddr); + argint(5, &ulen); if (len < 0) return -EINVAL; char *p = (char *)user_ptr(ubuf, (uint64)len); if (len && !p) return -EFAULT; - return sockwrite(s, p, len); + + // A null address is send(), not an error: the socket already knows where + // it is pointed, which for TCP is the only possibility anyway. + if (uaddr) { + if ((r = fetch_sin(uaddr, ulen, &ip, &port)) < 0) + return r; + port = port_from_net(port); + } + return socksendto(s, p, len, ip, port); } SYSCALL_DEFINE(recvfrom) { struct socket *s; - addr_t ubuf; + addr_t ubuf, uaddr, ulenp; int len, flags, r; + uint ip = 0; + ushort port = 0; if ((r = argsock(0, &s)) < 0) return r; argaddr(1, &ubuf); argint(2, &len); argint(3, &flags); + argaddr(4, &uaddr); + argaddr(5, &ulenp); if (len < 0) return -EINVAL; char *p = (char *)user_ptr(ubuf, (uint64)len); if (len && !p) return -EFAULT; - return sockread(s, p, len); + + r = sockrecvfrom(s, p, len, &ip, &port); + if (r < 0 || uaddr == 0) + return r; + + // The address is reported only for the family that has one to report. A + // Unix socket leaves ip at zero, and the honest answer there is a length of + // zero rather than a sockaddr full of nothing that a caller would read as + // 0.0.0.0:0 and believe. + int alen = 0; + if (sock_domain(s) == AF_INET) { + struct sockaddr_in sa; + memset(&sa, 0, sizeof(sa)); + sa.sin_family = AF_INET; + sa.sin_addr = ip; + sa.sin_port = port_to_net(port); + alen = (int)sizeof(sa); + + // Truncated rather than refused if the caller's buffer is smaller, + // which is what accept() and getsockname() do everywhere: the length + // written back tells them what they got. + int cap = alen; + if (ulenp && get_user(cap, ulenp) < 0) + return -EFAULT; + if (cap > alen) + cap = alen; + if (cap > 0 && copy_to_user(uaddr, &sa, (uint64)cap) < 0) + return -EFAULT; + } + + if (ulenp && copy_to_user(ulenp, &alen, sizeof(alen)) < 0) + return -EFAULT; + return r; } // shutdown(2). Half-close is not modelled -- there is one buffer per diff --git a/tools/windows/bootcheck.ps1 b/tools/windows/bootcheck.ps1 index 6440100..ffb90af 100644 --- a/tools/windows/bootcheck.ps1 +++ b/tools/windows/bootcheck.ps1 @@ -248,11 +248,17 @@ if ($Net) { $l = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0) $l.Start(); $script:netPort = $l.LocalEndpoint.Port; $l.Stop() + # A second port for UDP. Bound the same way, and separately: a free TCP + # port says nothing about the UDP port of the same number. + $u = [System.Net.Sockets.UdpClient]::new(0) + $script:netUdpPort = $u.Client.LocalEndPoint.Port; $u.Close() + # The interface first, so a failure below reads as "the address never # arrived" rather than "TCP is broken" -- two very different bugs that look # identical from a failed connect. $commands += 'cat /proc/netif' $commands += "nettest 10.0.2.2 $netPort" + $commands += "nettest -u 10.0.2.2 $netUdpPort" } if ($Ps) { $commands += 'ls /proc' @@ -370,6 +376,34 @@ $netJob = if ($Net) { } } else { $null } +# The UDP responder. A separate job because it is a separate exchange, and +# because a datagram socket has nothing to accept: it waits for one datagram +# and answers the address it came from, which is precisely what the guest is +# being asked to report back. +$netUdpJob = if ($Net) { + Start-Job -ArgumentList $netUdpPort -ScriptBlock { + param($port) + $u = [System.Net.Sockets.UdpClient]::new($port) + $u.Client.ReceiveTimeout = 900000 + $ep = [System.Net.IPEndPoint]::new([System.Net.IPAddress]::Any, 0) + $got = '' + try { + $data = $u.Receive([ref]$ep) + $got = [Text.Encoding]::ASCII.GetString($data) + $reply = [Text.Encoding]::ASCII.GetBytes("HELLO-UDP-FROM-HOST`n") + $u.Send($reply, $reply.Length, $ep) | Out-Null + # A second datagram, so the guest can show that it reads them one + # at a time. Two sends rather than one long one: the boundary is + # the property being tested, and it only exists if it was put there + # by the sender. + $second = [Text.Encoding]::ASCII.GetBytes("SECOND-DATAGRAM`n") + $u.Send($second, $second.Length, $ep) | Out-Null + } catch { } + $u.Close() + return $got + } +} else { $null } + $psi.Arguments = ($drives + $usbDevices + $netDevices + $monitor + @( '-m 1G -smp 1 -cpu qemu64,+rdtscp -machine pc-i440fx-9.2' '-display none' @@ -571,11 +605,17 @@ try { # is still sitting in accept, and the second case is a failure to report rather # than one to wait out. $netFromGuest = '' +$netUdpFromGuest = '' if ($Net -and $netJob) { Wait-Job $netJob -Timeout 20 | Out-Null $netFromGuest = (Receive-Job $netJob) -join '' Remove-Job $netJob -Force -ErrorAction SilentlyContinue } +if ($Net -and $netUdpJob) { + Wait-Job $netUdpJob -Timeout 20 | Out-Null + $netUdpFromGuest = (Receive-Job $netUdpJob) -join '' + Remove-Job $netUdpJob -Force -ErrorAction SilentlyContinue +} if (-not (Test-Path $log)) { throw "no serial log produced at $log" } $text = Get-Content $log -Raw @@ -760,6 +800,23 @@ if ($Net) { ExpectThat ($netFromGuest -match 'HELLO-FROM-SCHOS') ` 'the listener on this host received what the guest sent' ` "the listener read '$($netFromGuest -replace '\s+', ' ')'" + + # UDP, and specifically the source address. recvfrom filling that in is the + # thing being checked: over a stream the source is known before any byte + # arrives, so a kernel writing nothing at all would still look right there. + Expect 'nettest: udp OK' 'a UDP datagram went out and one came back' + Expect 'HELLO-UDP-FROM-HOST' 'the datagram carried the host''s bytes' + Expect "nettest: udp source 10\.0\.2\.2:$netUdpPort" ` + 'recvfrom reported the real sender, address and port' + ExpectThat ($netUdpFromGuest -match 'HELLO-UDP-FROM-SCHOS') ` + 'the responder on this host received the guest''s datagram' ` + "the responder read '$($netUdpFromGuest -replace '\s+', ' ')'" + # Message boundaries. The responder sends two datagrams of 20 and 16 bytes; + # a kernel that treated UDP as a byte stream would hand the first read all + # 36 at once, which is why the counts are asserted and not just the text. + Expect 'nettest: udp received 20 bytes' 'the first read returned exactly one datagram' + Expect 'nettest: udp second 16 bytes' 'and the second read returned the next one, whole' + Expect 'SECOND-DATAGRAM' 'the second datagram arrived intact' } if ($Ps) {