From 20cc673a41f754bccc789435de118380088f553a Mon Sep 17 00:00:00 2001 From: Nexoniarz Date: Sat, 15 Aug 2026 16:42:34 +0200 Subject: [PATCH] net: a network *class*, so the stack stops knowing what an e1000 is The TCP/IP stack reached straight into the first driver anyone wrote. net.c called e1000_present(), the lwIP glue was named netif_e1000.c and called e1000_transmit(), and include/defs.h declared the driver's internals to the whole kernel. The second NIC driver would therefore have arrived with a near-identical copy of the glue file and a patch to the protocol stack, which is the arrangement include/device.h was written to prevent -- the e1000 even said so, with `.ops = 0, // no class vtable yet; there is no net class`. There is now. A driver publishes a struct netdev_ops -- transmit, receive, mac, link_up -- and is finished: it never mentions lwIP, and nothing above it learns its name. The display class was already this shape; this is the same shape for networking, and the two now read the same way in kernel/dev/device.c. What that made possible immediately: - One netif per bound device instead of one per driver. lwIP's glue is netif_eth.c, it takes a struct device, and it holds up to NETIF_MAX interfaces. net.c attaches every network device it finds and starts DHCP on each, because which interface the network answers on is not ours to decide. - /proc/netif lists all of them, one block of fields each, with the interface name and the driver that claimed it. - The ARP self-test moved from the e1000 to the class. Nothing in it was chip-specific, and it is the first thing anyone will want to run against the next driver. Still `echo n > /proc/sysrq`. - Private headers live with their driver: e1000.h was in include/, where exactly one file could use it. The driver build now adds the driver's own directory to the include path. The e1000 also drives more than one card. Its probe() refused the second outright, because there was a single static instance -- so a machine with two of them drove one and silently ignored the other. Instances are an array now, and the instance is hung on dev->priv by a new optional `attached` hook that the bus calls once the device exists. probe() runs before device_add(), so there was previously nowhere to put it; that is the whole reason the hook exists rather than a line inside probe. QEMU gets an explicit NIC on every target (QEMU_NET). The default device happens to be an e1000 on this machine type today, which is exactly the sort of thing that stays true until a QEMU release changes it and the network stops being exercised without anyone noticing. Verified: - bootcheck -Net passes, 48 [ok], no failures -- including the moved self-test and two new checks that /proc/netif names both the interface and the driver behind it, which together say the stack went through the class rather than through a driver it was compiled to know. - Two NICs attached by hand (-device e1000 twice, second with its own MAC): eth0 and eth1 both attach, each reports its own MAC in /proc/netif, and each gets its own DHCP lease. Before this commit the same machine produced one interface. - make schfs-check still passes in full. One trap worth recording: moving a header broke the build for anyone with stale .d files -- make stopped with "No rule to make target 'include/e1000.h'". CFLAGS gains -MP, which emits a phony target per header so a moved or deleted one can never wedge the build again. Co-Authored-By: Claude Opus 5 --- Makefile | 41 +++- docs/drivers.md | 25 ++ docs/roadmap.md | 12 + include/defs.h | 25 +- include/netdev.h | 63 +++++ include/param.h | 6 + include/pci.h | 9 + kernel/dev/device.c | 159 ++++++++++++ kernel/dev/pci.c | 5 + kernel/dev/procfs.c | 3 +- kernel/drivers/net/e1000/e1000.c | 227 +++++++----------- {include => kernel/drivers/net/e1000}/e1000.h | 0 kernel/net/lwip/netif_e1000.c | 152 ------------ kernel/net/lwip/netif_eth.c | 215 +++++++++++++++++ kernel/net/net.c | 149 ++++++++---- tools/windows/bootcheck.ps1 | 13 +- 16 files changed, 733 insertions(+), 371 deletions(-) create mode 100644 include/netdev.h rename {include => kernel/drivers/net/e1000}/e1000.h (100%) delete mode 100644 kernel/net/lwip/netif_e1000.c create mode 100644 kernel/net/lwip/netif_eth.c diff --git a/Makefile b/Makefile index 7ae7c5f..f7b075e 100644 --- a/Makefile +++ b/Makefile @@ -79,7 +79,7 @@ KERN_NET += $(addprefix $(BUILD)/lwip/core/, \ KERN_NET += $(addprefix $(BUILD)/lwip/core/ipv4/, \ acd.o dhcp.o etharp.o icmp.o ip4.o ip4_addr.o ip4_frag.o) KERN_NET += $(addprefix $(BUILD)/lwip/netif/, ethernet.o) -KERN_NET += $(addprefix $(BUILD)/kernel/net/lwip/, sys_arch.o netif_e1000.o) +KERN_NET += $(addprefix $(BUILD)/kernel/net/lwip/, sys_arch.o netif_eth.o) KERN_PROC = $(addprefix $(BUILD)/kernel/proc/, \ proc.o) @@ -173,6 +173,22 @@ endif # virtio-gpu wins the display class, stdvga carries the splash and text console. QEMU_DISPLAY = -vga std -device virtio-gpu-pci +# A network card, on every QEMU target. +# +# Spelled out rather than left to QEMU's default NIC. The default exists and is +# an e1000 on this machine type today, which is exactly the sort of thing that +# is true until a QEMU release changes it -- and then the network silently stops +# being tested rather than failing loudly. Naming the device also means `make +# qemu` and `bootcheck -Net` are running the same hardware. +# +# User-mode networking: the guest gets 10.0.2.15 by DHCP, the host is 10.0.2.2, +# and no privileges are needed. A bridge or a tap would need root and would put +# the guest on the real network, which is not what a development boot wants. +# +# To point the driver at different hardware once there is a second one, override +# it: make qemu QEMU_NET='-netdev user,id=net0 -device rtl8139,netdev=net0' +QEMU_NET = -netdev user,id=net0 -device e1000,netdev=net0 + # An absolute pointing device, on an xHCI controller. # # A PS/2 mouse is relative, so inside a VM the guest cursor and the host's @@ -325,7 +341,12 @@ XFLAGS += -ftls-model=local-exec # answer that cannot change during a build. NO_SSP := $(shell $(CC) -fno-stack-protector -E -x c /dev/null >/dev/null 2>&1 && echo -fno-stack-protector) -CFLAGS = -fno-pic -static -fno-builtin -fno-strict-aliasing -Wall -MD -ggdb -fno-omit-frame-pointer +# -MP with -MD: emit a phony target for every header, so that *moving* or +# deleting one does not wedge the build. Without it the stale .d still names the +# old path and make stops with "No rule to make target 'include/e1000.h'" -- +# which happened the day that header moved next to its driver, and the only +# cure was knowing to delete the .d by hand. +CFLAGS = -fno-pic -static -fno-builtin -fno-strict-aliasing -Wall -MD -MP -ggdb -fno-omit-frame-pointer CFLAGS += -ffreestanding -fno-common -nostdlib -I./include $(XFLAGS) $(OPT) CFLAGS += $(NO_SSP) $(CFLAGS_EXTRA) @@ -516,7 +537,7 @@ ovmf: $(OVMF_CODE) $(OVMF_VARS) all: $(OVMF_CODE) $(OVMF_VARS) qemu-uefi: esp $(BUILD)/schfs.img $(OVMF_CODE) $(OVMF_VARS) - $(QEMU) -machine pc-i440fx-9.2 -drive if=pflash,format=raw,unit=0,readonly=on,file=$(OVMF_CODE) -drive if=pflash,format=raw,unit=1,file=$(OVMF_VARS) -drive file=fat:rw:$(BUILD)/esp,index=0,media=disk,format=raw -drive file=$(BUILD)/schfs.img,format=raw,index=1,media=disk -m 1G -smp 1 $(QEMU_ACCEL) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) + $(QEMU) -machine pc-i440fx-9.2 -drive if=pflash,format=raw,unit=0,readonly=on,file=$(OVMF_CODE) -drive if=pflash,format=raw,unit=1,file=$(OVMF_VARS) -drive file=fat:rw:$(BUILD)/esp,index=0,media=disk,format=raw -drive file=$(BUILD)/schfs.img,format=raw,index=1,media=disk -m 1G -smp 1 $(QEMU_ACCEL) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) $(QEMU_NET) # Build the ramdisk-root variant. This is a target rather than just # "make RAMROOT=1" because toggling RAMROOT changes CFLAGS (-DRAMDISK_ROOT), @@ -579,7 +600,7 @@ schos.iso: $(BUILD)/kernel/kernel $(BUILD)/uefi/BOOTX64.EFI sysroot-install # all, so a kernel that still secretly depends on one fails here rather than # in someone's optical drive. qemu-iso: schos.iso - $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_ACCEL) \ + $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_ACCEL) $(QEMU_NET) \ -cdrom schos.iso -boot d \ -m 1G -smp 2 -machine pc-i440fx-9.2 @@ -680,9 +701,13 @@ $(BUILD)/kernel/dev/%.o: kernel/dev/%.c # `$(dir $@)` rather than a fixed path: drivers are grouped into one directory # per device class (gpu/, net/, bus/, usb/), so the output directory varies with # the target and a hardcoded mkdir only creates the top one. +# -I on the driver's own directory, so a driver can keep its register +# definitions next to its code instead of in include/. include/ is for headers +# the rest of the kernel needs; a chip's register map is not one of those, and +# e1000.h sat there for a year being included by exactly one file. $(BUILD)/kernel/drivers/%.o: kernel/drivers/%.c @mkdir -p $(dir $@) - $(CC) $(CFLAGS) -c $< -o $@ + $(CC) $(CFLAGS) -I$(dir $<) -c $< -o $@ $(BUILD)/kernel/fs/%.o: kernel/fs/%.c @mkdir -p $(BUILD)/kernel/fs @@ -977,11 +1002,11 @@ OVMF_DRIVES = -drive if=pflash,format=raw,unit=0,readonly=on,file=$(OVMF_CODE) \ # insisting on it, which is what you want when the point is to find out why the # host has no /dev/kvm rather than to quietly get a slow machine. qemu-kvm: esp $(BUILD)/schfs.img $(OVMF_CODE) $(OVMF_VARS) - $(QEMU) -enable-kvm -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) -cpu host \ + $(QEMU) -enable-kvm -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) $(QEMU_NET) -cpu host \ $(OVMF_DRIVES) -m 1G -smp 2 -machine pc-i440fx-9.2 qemu: esp $(BUILD)/schfs.img $(OVMF_CODE) $(OVMF_VARS) - $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) $(QEMU_ACCEL) \ + $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_INPUT) $(QEMU_ACCEL) $(QEMU_NET) \ $(OVMF_DRIVES) -m 1G -smp 2 -machine pc-i440fx-9.2 # The root filesystem. Built from the staged sysroot; the kernel mounts it as @@ -1001,7 +1026,7 @@ $(BUILD)/schfs.img: $(MKSCHFS) $(BUILD)/sysroot.stamp # # Once up: cat /proc/schfs qemu-schfs: esp $(BUILD)/schfs.img $(OVMF_CODE) $(OVMF_VARS) - $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_ACCEL) \ + $(QEMU) -serial mon:stdio $(QEMU_DISPLAY) $(QEMU_ACCEL) $(QEMU_NET) \ $(OVMF_DRIVES) -m 1G -smp 2 -machine pc-i440fx-9.2 # ----------------------------------------------------------------------- diff --git a/docs/drivers.md b/docs/drivers.md index 78e1095..541a761 100644 --- a/docs/drivers.md +++ b/docs/drivers.md @@ -116,11 +116,36 @@ existing mechanisms for keeping that true mechanically rather than by intention. 2. Declare it in `kernel/drivers/registry.c` and add it to the array. 3. Add the object to `KERN_DRIVERS` in the `Makefile`. +4. Fill in the **class vtable** for what it is, and point `.ops` at it. That is + the whole of what the rest of the kernel sees: + + | class | vtable | consumer | + |---|---|---| + | `DEVCLASS_DISPLAY` | `struct display_ops` ([display.h](../include/display.h)) | the compositor, the console | + | `DEVCLASS_NET` | `struct netdev_ops` ([netdev.h](../include/netdev.h)) | the TCP/IP stack | + + A driver that fills these in never mentions its consumer, and its consumer + never mentions it. Getting this wrong has a characteristic shape: the e1000 + shipped with `.ops = 0` and the network stack called `e1000_transmit()` + directly, so `kernel/net/` was full of one driver's name and the second NIC + driver would have had to edit the protocol stack to add itself. + +Private headers go **next to the driver**, not in `include/`. The build adds +the driver's own directory to the include path, so `e1000.h` — a register map +that exactly one file has any use for — lives in `kernel/drivers/net/e1000/`. +`include/` is for headers the rest of the kernel needs. Registration in the device model is automatic — `pci_bind()` and `usb_bind_all()` publish whatever they claim, so a driver cannot forget to. Set `devclass` on a USB driver, or the device lands in `other`. +A driver that can drive **more than one card at once** keeps its instances in +an array and implements `.attached`, which the bus calls once the `struct +device` exists; hang the instance on `dev->priv` there and read it back in +every op. It cannot be done inside `probe()`, because the device is published +afterwards. Skipping this is how a driver ends up refusing the second card, as +the e1000 did until it was asked to drive two. + The best source for a new driver is the **specification**, not another kernel: xHCI, AHCI, virtio, NVMe and the Intel datasheets are public documents with no licence on what you write from them. `kernel/drivers/bus/xhci/xhci.c` came from the spec, and so did diff --git a/docs/roadmap.md b/docs/roadmap.md index 252452e..16ae784 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -167,9 +167,21 @@ TCP/IP, so they are the right thing to build the socket layer's shape around. to them, and both ends are asserted -- one side alone cannot tell a working stack from one that completes a handshake and drops every byte. 45 [ok] with it on. +- [x] The **netdev class**, 2026-08-15. The stack reached straight into the + e1000: `net.c` called `e1000_present()`, the lwIP glue was called + `netif_e1000.c`, and a second NIC driver would have meant editing the + protocol stack to add itself. A driver now publishes a + `struct netdev_ops` (include/netdev.h) and nothing above it learns its + name; lwIP gets one netif per bound device, `/proc/netif` lists them all, + and the e1000 drives up to four cards rather than refusing the second. + Verified with two NICs attached: `eth0` and `eth1`, each with its own MAC + and its own DHCP lease. - [ ] `accept()` does not report the peer's address, and there is no `getsockname`/`getpeername`. Nothing has needed them yet; a server that wants to log who connected will. +- [ ] No routing table. With two interfaces up, the first is the default route + and the second is reachable only by an address on its own subnet. That is + a rule rather than an accident, but it is not routing. - [ ] Receive is still polled. The network thread looks at the NIC every timer tick when the link is idle, so an arriving frame waits up to 10 ms before anything sees it. Waking that thread from the e1000 interrupt is the real diff --git a/include/defs.h b/include/defs.h index bef234c..84fbe27 100644 --- a/include/defs.h +++ b/include/defs.h @@ -13,6 +13,7 @@ struct stat; struct superblock; struct trapframe; struct dirent; +struct device; // entryother.S / syscall.S void syscall_entry(void); @@ -529,12 +530,10 @@ int fb_ioctl(int request, addr_t arg); // IDE device number. Writes bypass every filesystem above them. int diskread(struct inode*, uint64, char*, int); int diskwrite(struct inode*, uint64, char*, int); -// drivers/net/e1000 -- raw Ethernet. No protocol stack above it yet, so these -// are the whole interface: queue a frame, take a frame, and the MAC. -int e1000_transmit(const void *frame, int len); -int e1000_receive(void *out, int cap); -const unsigned char *e1000_mac(void); -int e1000_present(void); +// Network *drivers* are not declared here at all. They are reached through the +// netdev class in include/netdev.h -- a driver publishes a vtable and nothing +// above it ever learns its name. Declaring e1000_transmit() here was what let +// the protocol stack call it directly. // net/net.c -- the TCP/IP stack (lwIP). netinit starts the network thread; // net_is_up reports whether an address has been obtained. @@ -542,15 +541,17 @@ void netinit(void); int net_is_up(void); int net_info(char *buf, int cap); -// net/lwip/ -- the port layer and the interface, called only from net.c. +// net/lwip/ -- the port layer and the Ethernet interfaces, called only from +// net.c. One netif per bound network device; the index is that interface's. void lwip_sys_init(void); void lwip_sys_lock(void); void lwip_sys_unlock(void); -int net_e1000_attach(void); -int net_e1000_poll(void); -void net_e1000_counts(uint64 *rx, uint64 *tx); -struct netif *net_e1000_netif(void); -void e1000_selftest(void); +int net_eth_attach(struct device *); +int net_eth_poll(void); +int net_eth_count(void); +void net_eth_counts(int i, uint64 *rx, uint64 *tx); +struct netif *net_eth_netif(int i); +struct device *net_eth_device(int i); // registry.c void platform_register_devices(void); diff --git a/include/netdev.h b/include/netdev.h new file mode 100644 index 0000000..c59ad0f --- /dev/null +++ b/include/netdev.h @@ -0,0 +1,63 @@ +#pragma once + +// The network class: what the TCP/IP stack talks to instead of a driver. +// +// The stack should ask for "the network interfaces" and get whatever is bound +// -- today an e1000, tomorrow a virtio-net or an rtl8139 -- exactly as a +// compositor asks for "the display" and does not care which driver answers. +// Without this the first driver's name spreads: kernel/net/net.c called +// e1000_present(), the lwIP glue was named netif_e1000.c and called +// e1000_transmit(), and adding a second NIC would have meant editing the +// protocol stack to add it. That is the mistake include/device.h was written +// to prevent, made once anyway because the net class did not exist yet -- +// kernel/drivers/net/e1000/e1000.c even said so, with `.ops = 0, // no class +// vtable yet`. +// +// A driver publishes one of these and is then finished with the network stack +// forever: it never mentions lwIP, and the stack never mentions it. +// +// Found with device_find_class(DEVCLASS_NET) or, when there may be more than +// one, netdev_at(); see include/device.h. + +#include "device.h" + +struct netdev_ops { + // Queue one frame for transmission. Returns 0, or -1 if the ring is full -- + // which is congestion, not failure, and the caller is expected to retry. + int (*transmit)(struct device *, const void *frame, int len); + + // Take one received frame if there is one. Returns its length, or 0 when + // nothing is waiting. Never blocks: the caller decides how to wait. + int (*receive)(struct device *, void *out, int cap); + + // The station address, six bytes. Every Ethernet interface has one and the + // stack needs it before it can send anything, so this is not optional. + const unsigned char *(*mac)(struct device *); + + // Whether the link is up. Optional -- a driver that cannot tell leaves this + // null and is treated as connected, which is the assumption that was made + // before any driver could report it. + int (*link_up)(struct device *); +}; + +// The i-th bound network device, or 0 past the end. Dense over interfaces +// that have both a driver and a vtable, so the stack can simply count up. +struct device *netdev_at(int i); +int netdev_count(void); + +// Wrappers with the null checks in one place, so no caller is written around +// a vtable that may be absent or partly filled in. +int netdev_transmit(struct device *, const void *frame, int len); +int netdev_receive(struct device *, void *out, int cap); +const unsigned char *netdev_mac(struct device *); +int netdev_link_up(struct device *); + +// Send an ARP request by hand and wait for the reply. `echo n > /proc/sysrq`. +// +// It proves the whole driver path at once -- descriptor formats, the DD +// handshake, tail bookkeeping, DMA addresses, the receive ring hand-back and +// the MAC filter -- because an ARP reply is addressed to this station and does +// not arrive if the filter is wrong. Written for the e1000 before there was a +// stack to test it with; it belongs to the class rather than to that driver, +// since it is the check every new NIC driver will want first. +void netdev_selftest(void); diff --git a/include/param.h b/include/param.h index 7e85e38..fbbd75d 100644 --- a/include/param.h +++ b/include/param.h @@ -37,3 +37,9 @@ // with tens of megabytes; 8 MB would not be. #define NBUF 256 #define FSSIZE 150000 // size of file system in blocks + +// Network interfaces the stack will attach. Two is already an ordinary machine +// -- a laptop with Ethernet plus a virtual adapter -- and each one costs a +// struct netif, so the limit is about keeping a fixed array honest rather than +// about memory. +#define NETIF_MAX 4 diff --git a/include/pci.h b/include/pci.h index be4a6e5..b34f67a 100644 --- a/include/pci.h +++ b/include/pci.h @@ -107,4 +107,13 @@ struct pci_driver { // Return 0 to claim the device. A driver that returns non-zero leaves it // unclaimed for anything else that matches. int (*probe)(struct pci_dev *); + // Called right after the device is published, for a driver that supports + // more than one instance and needs to say which of its own is which. + // Optional -- a single-instance driver has nothing to attach. + // + // It exists because probe() runs *before* device_add(), so a driver has + // nowhere to put its per-instance pointer at the point it would want to. + // Without this the second card of the same model is either refused or + // silently shares the first one's registers. + void (*attached)(struct pci_dev *, struct device *); }; diff --git a/kernel/dev/device.c b/kernel/dev/device.c index bb5469e..9c4b300 100644 --- a/kernel/dev/device.c +++ b/kernel/dev/device.c @@ -15,6 +15,7 @@ #include "param.h" #include "device.h" #include "display.h" +#include "netdev.h" #include "theme.h" #include "text.h" @@ -306,3 +307,161 @@ text_selftest(void) display_present(0, 0, m.width, m.height); cprintf("text: drew a specimen, \"%s\" measures %d px\n", msg, w); } + +// --------------------------------------------------------------------------- +// The network class. See include/netdev.h. +// +// Plural where the display class is singular: a machine with two NICs is +// ordinary, and "the network" is not a thing a stack can ask for the way a +// compositor asks for "the display". So this walks, and the caller decides +// what to do with each interface it finds. + +struct device * +netdev_at(int i) +{ + int seen = 0; + + for (int k = 0; k < device_count(); k++) { + struct device *d = device_at(k); + if (!d || d->class != DEVCLASS_NET || !d->driver || !d->ops) + continue; + if (seen++ == i) + return d; + } + return 0; +} + +int +netdev_count(void) +{ + int n = 0; + + while (netdev_at(n)) + n++; + return n; +} + +int +netdev_transmit(struct device *d, const void *frame, int len) +{ + if (!d || !d->ops || !((struct netdev_ops *)d->ops)->transmit) + return -1; + return ((struct netdev_ops *)d->ops)->transmit(d, frame, len); +} + +int +netdev_receive(struct device *d, void *out, int cap) +{ + if (!d || !d->ops || !((struct netdev_ops *)d->ops)->receive) + return 0; + return ((struct netdev_ops *)d->ops)->receive(d, out, cap); +} + +const unsigned char * +netdev_mac(struct device *d) +{ + if (!d || !d->ops || !((struct netdev_ops *)d->ops)->mac) + return 0; + return ((struct netdev_ops *)d->ops)->mac(d); +} + +int +netdev_link_up(struct device *d) +{ + // A driver that cannot tell is treated as up. Reporting "down" for a link + // whose state is simply unknown would stop the stack on hardware that works. + if (!d || !d->ops || !((struct netdev_ops *)d->ops)->link_up) + return 1; + return ((struct netdev_ops *)d->ops)->link_up(d); +} + +static void +put_be16(uint8 *p, uint16 v) +{ + p[0] = (uint8)(v >> 8); + p[1] = (uint8)v; +} + +// The ARP probe. Written for the e1000 when it was the only driver and there +// was no stack to test it with; it belongs here because it is the first thing +// anyone will want to run against the *next* driver, and none of it is +// specific to a chip. +// +// Under QEMU's user-mode networking the guest is 10.0.2.15 and the gateway is +// 10.0.2.2. SLIRP is silent until spoken to, which is why this has to transmit +// before it can expect to receive anything. +void +netdev_selftest(void) +{ + struct device *d = netdev_at(0); + uint8 frame[42]; + uint8 reply[2048]; + + if (!d) { + cprintf("netdev: no interface\n"); + return; + } + + const unsigned char *mac = netdev_mac(d); + if (!mac) { + cprintf("netdev: %s has no station address\n", d->name); + return; + } + + static const uint8 me[4] = { 10, 0, 2, 15 }; // QEMU SLIRP guest address + static const uint8 them[4] = { 10, 0, 2, 2 }; // and its gateway + + memset(frame, 0, sizeof(frame)); + for (int i = 0; i < 6; i++) + frame[i] = 0xFF; // broadcast + for (int i = 0; i < 6; i++) + frame[6 + i] = mac[i]; + put_be16(&frame[12], 0x0806); // ARP + + put_be16(&frame[14], 1); // Ethernet + put_be16(&frame[16], 0x0800); // IPv4 + frame[18] = 6; // hardware address length + frame[19] = 4; // protocol address length + put_be16(&frame[20], 1); // request + for (int i = 0; i < 6; i++) + frame[22 + i] = mac[i]; // sender hardware address + for (int i = 0; i < 4; i++) + frame[28 + i] = me[i]; // sender protocol address + // target hardware address stays zero -- that is what is being asked + for (int i = 0; i < 4; i++) + frame[38 + i] = them[i]; + + cprintf("netdev: %s asking who has %d.%d.%d.%d\n", + d->name, them[0], them[1], them[2], them[3]); + if (netdev_transmit(d, frame, sizeof(frame)) != 0) { + cprintf("netdev: %s transmit ring full\n", d->name); + return; + } + + // Bounded poll. This runs from a sysrq write and there is nothing to sleep + // on that would be woken by a raw frame, so spinning briefly is honest. + // + // It will also swallow whatever else is on the wire, including the stack's + // own traffic if it is running -- which is why a non-ARP frame is reported + // rather than treated as a failure. + for (int spin = 0; spin < 20000000; spin++) { + int n = netdev_receive(d, reply, sizeof(reply)); + if (n <= 0) + continue; + + cprintf("netdev: %d bytes from %x:%x:%x:%x:%x:%x type %x\n", + n, reply[6], reply[7], reply[8], reply[9], reply[10], reply[11], + ((uint32)reply[12] << 8) | reply[13]); + + if (n >= 42 && reply[12] == 0x08 && reply[13] == 0x06 && + reply[20] == 0 && reply[21] == 2) { + cprintf("netdev: ARP reply -- %d.%d.%d.%d is at %x:%x:%x:%x:%x:%x\n", + reply[28], reply[29], reply[30], reply[31], + reply[22], reply[23], reply[24], reply[25], + reply[26], reply[27]); + cprintf("netdev: %s self-test passed\n", d->name); + return; + } + } + cprintf("netdev: %s no reply\n", d->name); +} diff --git a/kernel/dev/pci.c b/kernel/dev/pci.c index b8f8d78..fb517eb 100644 --- a/kernel/dev/pci.c +++ b/kernel/dev/pci.c @@ -386,6 +386,11 @@ pci_bind(const struct pci_driver *drv) dev->product = d->device; dev->b = d->bus; dev->s = d->slot; dev->f = d->func; pci_describe(d, dev->desc, sizeof(dev->desc)); + // Now, and only now, can a multi-instance driver say which of its + // own this device is: the object it would hang that on did not + // exist until the line above. + if (drv->attached) + drv->attached(d, dev); } } break; diff --git a/kernel/dev/procfs.c b/kernel/dev/procfs.c index 8cc356c..18039b9 100644 --- a/kernel/dev/procfs.c +++ b/kernel/dev/procfs.c @@ -21,6 +21,7 @@ #include "x86.h" #include "device.h" #include "display.h" +#include "netdev.h" #include "param.h" #include "memlayout.h" #include "mmu.h" @@ -444,7 +445,7 @@ proc_sysrq_cmd(char cmd) display_selftest(); break; case 'n': - e1000_selftest(); + netdev_selftest(); break; case 't': text_selftest(); diff --git a/kernel/drivers/net/e1000/e1000.c b/kernel/drivers/net/e1000/e1000.c index be35986..ff6b251 100644 --- a/kernel/drivers/net/e1000/e1000.c +++ b/kernel/drivers/net/e1000/e1000.c @@ -1,10 +1,11 @@ // Intel 8254x gigabit Ethernet, written from Intel's public datasheet (317453). // -// The first network device. There is no IP stack yet, so what this provides is -// the layer under one: a MAC address, a transmit path, and a receive ring that -// fills with frames. That is deliberately the right order -- an IP stack -// written before a driver has nothing to be tested against, and every bug in it -// looks like a driver bug. +// What it provides the rest of the system is a struct netdev_ops and nothing +// else: a station address, a transmit path and a receive ring. It does not +// know that lwIP exists, and lwIP does not know that it does -- see +// include/netdev.h for why that boundary is where it is. Adding a second NIC +// means writing another driver that fills in the same four function pointers, +// and touching no file outside its own directory. // // Bring-up follows the manual's initialisation sequence (section 14.3-14.5): // reset, read the MAC out of EEPROM, clear the multicast table, set up the @@ -24,6 +25,7 @@ #include "memlayout.h" #include "pci.h" #include "device.h" +#include "netdev.h" #include "dma.h" #include "irq.h" #include "e1000.h" @@ -42,10 +44,29 @@ struct e1000 { int ready; uint64 rx_packets, tx_packets, rx_dropped; + // Which PCI device this instance drives. Kept so that attached() can match + // the device the bus just published against the instance probe() set up -- + // the two calls are separate, and nothing else connects them. + struct pci_dev *pdev; + struct spinlock lock; }; -static struct e1000 nic; +// More than one, because a machine really can have two of the same card and +// this driver used to refuse the second outright. Four is the same limit the +// stack puts on interfaces; each unused entry costs a few dozen bytes, and the +// rings behind a used one come from the DMA arena at probe time. +#define E1000_MAX_NICS 4 + +static struct e1000 nics[E1000_MAX_NICS]; + +// The instance behind a device, or 0. Every op goes through this rather than a +// file-static pointer, which is what makes two cards possible at all. +static struct e1000 * +nic_of(struct device *dev) +{ + return dev ? (struct e1000 *)dev->priv : 0; +} static inline uint32 rd(struct e1000 *e, int off) { return *(volatile uint32 *)(e->regs + off); } static inline void wr(struct e1000 *e, int off, uint32 v) { *(volatile uint32 *)(e->regs + off) = v; } @@ -156,12 +177,13 @@ rings_setup(struct e1000 *e) // Copies into the descriptor's own buffer rather than mapping the caller's. // A zero-copy path wants the caller to guarantee the memory stays put and is // physically contiguous, which nothing above this can promise yet. -int -e1000_transmit(const void *frame, int len) +// +static int +e1000_transmit(struct device *dev, const void *frame, int len) { - struct e1000 *e = &nic; + struct e1000 *e = nic_of(dev); - if (!e->ready || len <= 0 || len > E1000_BUFSZ) + if (!e || !e->ready || len <= 0 || len > E1000_BUFSZ) return -1; acquire(&e->lock); @@ -190,12 +212,12 @@ e1000_transmit(const void *frame, int len) // wakes a nonexistent protocol stack is harder to reason about than one that is // asked. The interrupt handler below exists to keep the cause register clear // and to count; when an IP stack arrives, this is where the hand-off changes. -int -e1000_receive(void *out, int cap) +static int +e1000_receive(struct device *dev, void *out, int cap) { - struct e1000 *e = &nic; + struct e1000 *e = nic_of(dev); - if (!e->ready) + if (!e || !e->ready) return 0; acquire(&e->lock); @@ -231,37 +253,24 @@ e1000_intr(void *arg) (void)cause; } -int -e1000_info(char *buf, int cap) -{ - struct e1000 *e = &nic; - int n = 0; - - if (!e->ready) - return 0; - - static const char hex[] = "0123456789abcdef"; - n += 0; - for (int i = 0; i < 6 && n + 3 < cap; i++) { - buf[n++] = hex[e->mac[i] >> 4]; - buf[n++] = hex[e->mac[i] & 0xF]; - if (i < 5) - buf[n++] = ':'; - } - if (n < cap) buf[n++] = '\n'; - return n; -} - static int e1000_probe(struct pci_dev *d) { - struct e1000 *e = &nic; + struct e1000 *e = 0; - if (e->ready) + // A free slot, rather than "the one instance, if it is still free". The old + // spelling is why a machine with two of these cards drove only the first. + for (int i = 0; i < E1000_MAX_NICS; i++) + if (!nics[i].ready) { + e = &nics[i]; + break; + } + if (!e) return -1; if (!d->bar[0].base) return -1; + e->pdev = d; initlock(&e->lock, "e1000"); pci_enable(d, 0x0006); // memory space + bus master // ioremap rather than P2V: firmware may place a 64-bit BAR above the 4 GB the @@ -353,117 +362,45 @@ static const struct pci_match e1000_match[] = { }; #undef M +static const unsigned char * +e1000_mac_op(struct device *dev) +{ + struct e1000 *e = nic_of(dev); + return (e && e->ready) ? e->mac : 0; +} + +static int +e1000_link_up(struct device *dev) +{ + struct e1000 *e = nic_of(dev); + return e && e->ready && (rd(e, E1000_STATUS) & E1000_STATUS_LU) != 0; +} + +// Hang the instance on the device the bus has just published. See the comment +// on `attached` in include/pci.h for why this cannot happen inside probe(). +static void +e1000_attached(struct pci_dev *d, struct device *dev) +{ + for (int i = 0; i < E1000_MAX_NICS; i++) + if (nics[i].ready && nics[i].pdev == d) { + dev->priv = &nics[i]; + return; + } +} + +// The whole of what the network stack sees of this driver. +static const struct netdev_ops e1000_netdev_ops = { + .transmit = e1000_transmit, + .receive = e1000_receive, + .mac = e1000_mac_op, + .link_up = e1000_link_up, +}; + const struct pci_driver e1000_driver = { .name = "e1000", .devname = "eth", .match = e1000_match, - .ops = 0, // no class vtable yet; there is no net class + .ops = &e1000_netdev_ops, .probe = e1000_probe, + .attached = e1000_attached, }; - -// --- self-test: does this thing actually move packets? --------------------- -// -// The transmit and receive paths were written together and neither had ever -// carried a frame. That is exactly the state in which a driver looks finished -// and is not: the rings are set up, the registers read back plausibly, and the -// first thing to discover otherwise would have been a protocol stack failing -// for reasons indistinguishable from its own bugs. -// -// So: build an ARP request by hand, send it, and wait for the reply. It proves -// the whole path at once -- descriptor format, DD handshake, tail bookkeeping, -// DMA addresses, the receive ring handback, and the MAC filter -- because an -// ARP reply is addressed to *us* and will not arrive if the filter is wrong. -// -// Under QEMU's user-mode networking the guest is 10.0.2.15 and the gateway is -// 10.0.2.2. SLIRP is silent until spoken to, which is why this has to transmit -// before it can expect to receive. -// -// `echo n > /proc/sysrq`. - -static void -put_be16(uint8 *p, uint16 v) -{ - p[0] = (uint8)(v >> 8); - p[1] = (uint8)v; -} - -// Is there a NIC at all? Asked by netinit() before it starts a thread whose -// entire job would be to poll a device that is not there. -int -e1000_present(void) -{ - return nic.ready; -} - -const unsigned char * -e1000_mac(void) -{ - return nic.mac; -} - -void -e1000_selftest(void) -{ - struct e1000 *e = &nic; - uint8 frame[42]; - uint8 reply[E1000_BUFSZ]; - - if (!e->ready) { - cprintf("e1000: no device\n"); - return; - } - - static const uint8 me[4] = { 10, 0, 2, 15 }; // QEMU SLIRP guest address - static const uint8 them[4] = { 10, 0, 2, 2 }; // and its gateway - - memset(frame, 0, sizeof(frame)); - for (int i = 0; i < 6; i++) - frame[i] = 0xFF; // broadcast - for (int i = 0; i < 6; i++) - frame[6 + i] = e->mac[i]; - put_be16(&frame[12], 0x0806); // ARP - - put_be16(&frame[14], 1); // Ethernet - put_be16(&frame[16], 0x0800); // IPv4 - frame[18] = 6; // hardware address length - frame[19] = 4; // protocol address length - put_be16(&frame[20], 1); // request - for (int i = 0; i < 6; i++) - frame[22 + i] = e->mac[i]; // sender hardware address - for (int i = 0; i < 4; i++) - frame[28 + i] = me[i]; // sender protocol address - // target hardware address stays zero -- that is what is being asked - for (int i = 0; i < 4; i++) - frame[38 + i] = them[i]; - - cprintf("e1000: asking who has %d.%d.%d.%d\n", them[0], them[1], them[2], them[3]); - if (e1000_transmit(frame, sizeof(frame)) != 0) { - cprintf("e1000: transmit ring full\n"); - return; - } - - // Bounded poll. There is no interrupt path to sleep on yet and this runs from - // a sysrq write, so spinning briefly is the honest way to wait. - for (int spin = 0; spin < 20000000; spin++) { - int n = e1000_receive(reply, sizeof(reply)); - if (n <= 0) - continue; - - cprintf("e1000: %d bytes from %x:%x:%x:%x:%x:%x type %x\n", - n, reply[6], reply[7], reply[8], reply[9], reply[10], reply[11], - ((uint32)reply[12] << 8) | reply[13]); - - if (n >= 42 && reply[12] == 0x08 && reply[13] == 0x06 && - reply[20] == 0 && reply[21] == 2) { - cprintf("e1000: ARP reply -- %d.%d.%d.%d is at %x:%x:%x:%x:%x:%x\n", - reply[28], reply[29], reply[30], reply[31], - reply[22], reply[23], reply[24], reply[25], - reply[26], reply[27]); - cprintf("e1000: self-test passed, %d tx %d rx\n", - (int)e->tx_packets, (int)e->rx_packets); - return; - } - } - cprintf("e1000: no reply -- %d tx, %d rx\n", - (int)e->tx_packets, (int)e->rx_packets); -} diff --git a/include/e1000.h b/kernel/drivers/net/e1000/e1000.h similarity index 100% rename from include/e1000.h rename to kernel/drivers/net/e1000/e1000.h diff --git a/kernel/net/lwip/netif_e1000.c b/kernel/net/lwip/netif_e1000.c deleted file mode 100644 index 6faee36..0000000 --- a/kernel/net/lwip/netif_e1000.c +++ /dev/null @@ -1,152 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// -// The lwIP network interface backed by the e1000 driver. -// -// Two directions and almost nothing else: a linkoutput that flattens a pbuf -// chain into a frame and hands it to the driver, and a poll that takes frames -// off the driver and hands them up. Everything about IP, ARP and TCP lives in -// lwIP; everything about descriptors and registers lives in the driver; this -// file is the seam. - -#include "types.h" -#include "defs.h" -#include "param.h" - -#include "lwip/opt.h" -#include "lwip/netif.h" -#include "lwip/pbuf.h" -#include "lwip/etharp.h" -#include "lwip/ethip6.h" -#include "netif/ethernet.h" - -#define ETH_MTU 1500 -#define ETH_FRAMESZ 1518 // 1500 payload + 14 header + 4 FCS allowance - -static struct netif e1000_netif; - -// Frames in and out, for /proc/netif. Counted here rather than in the driver -// because this is the seam lwIP actually uses: a frame the driver received and -// this file dropped never reached the stack, and a counter kept one layer down -// would say otherwise. Only the network thread touches them, so no lock. -static uint64 rx_frames, tx_frames; - -void -net_e1000_counts(uint64 *rx, uint64 *tx) -{ - *rx = rx_frames; - *tx = tx_frames; -} - -// One staging buffer, not one per call: a frame is 1518 bytes and the kernel -// stack is 4096 in total, so a local of this size on a path that is several -// frames deep is exactly the overflow the kstack canary exists to catch. -// Safe as a single static because every caller holds the lwIP lock. -static char txframe[ETH_FRAMESZ]; -static char rxframe[ETH_FRAMESZ]; - -// Send one packet. lwIP hands over a pbuf chain; the driver wants one flat -// buffer, so this walks the chain into txframe. -static err_t -e1000_linkoutput(struct netif *netif, struct pbuf *p) -{ - (void)netif; - - if (p->tot_len > sizeof(txframe)) - return ERR_IF; - - u16_t off = 0; - for (struct pbuf *q = p; q != NULL; q = q->next) { - if (off + q->len > sizeof(txframe)) - return ERR_IF; - memmove(txframe + off, q->payload, q->len); - off += q->len; - } - - // A full transmit ring is congestion, not failure. Reporting ERR_IF would - // make lwIP drop the segment and wait for a retransmission timer; ERR_WOULDBLOCK - // lets it try again, which for TCP is the difference between a stall of - // milliseconds and one of seconds. - if (e1000_transmit(txframe, off) != 0) - return ERR_WOULDBLOCK; - - tx_frames++; - return ERR_OK; -} - -// Bring the interface up. Called once, by the network thread. -static err_t -e1000_netif_init(struct netif *netif) -{ - const unsigned char *mac = e1000_mac(); - - netif->name[0] = 'e'; - netif->name[1] = 'n'; - netif->output = etharp_output; // IP -> ARP -> linkoutput - netif->linkoutput = e1000_linkoutput; - netif->mtu = ETH_MTU; - netif->hwaddr_len = 6; - for (int i = 0; i < 6; i++) - netif->hwaddr[i] = mac[i]; - netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | NETIF_FLAG_LINK_UP; - return ERR_OK; -} - -struct netif * -net_e1000_netif(void) -{ - return &e1000_netif; -} - -// Attach the interface with no address yet -- DHCP fills it in. -int -net_e1000_attach(void) -{ - ip4_addr_t any; - IP4_ADDR(&any, 0, 0, 0, 0); - - if (netif_add(&e1000_netif, &any, &any, &any, NULL, - e1000_netif_init, netif_input) == NULL) - return -1; - - netif_set_default(&e1000_netif); - netif_set_up(&e1000_netif); - netif_set_link_up(&e1000_netif); - return 0; -} - -// Take whatever the driver has and push it into lwIP. Returns how many frames -// moved, so the caller can tell a busy poll from an idle one. -// -// Bounded rather than draining: a receive loop with no limit starves everything -// else on a busy network, and the caller comes straight back anyway. -int -net_e1000_poll(void) -{ - int moved = 0; - - for (int i = 0; i < 32; i++) { - int len = e1000_receive(rxframe, sizeof(rxframe)); - if (len <= 0) - break; - - // PBUF_POOL, so this comes from lwIP's own pool rather than a heap it - // does not have. A null return means the pool is exhausted, which is a - // dropped packet -- entirely normal under load, and what the protocol - // above is designed to survive. - struct pbuf *p = pbuf_alloc(PBUF_RAW, (u16_t)len, PBUF_POOL); - if (p == NULL) - break; - - pbuf_take(p, rxframe, (u16_t)len); - - // netif->input is netif_input, set at netif_add: it dispatches on the - // Ethernet type. Ownership of the pbuf passes with the call, including - // when it fails, so there is nothing to free here. - if (e1000_netif.input(p, &e1000_netif) != ERR_OK) - pbuf_free(p); - - rx_frames++; - moved++; - } - return moved; -} diff --git a/kernel/net/lwip/netif_eth.c b/kernel/net/lwip/netif_eth.c new file mode 100644 index 0000000..99ab94d --- /dev/null +++ b/kernel/net/lwip/netif_eth.c @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// lwIP network interfaces over the netdev class. +// +// The seam, and it is deliberately driver-agnostic: everything about IP, ARP +// and TCP lives in lwIP, everything about descriptors and registers lives in a +// driver, and this file knows only that a network device can transmit a frame, +// receive a frame and state its MAC address. It was `netif_e1000.c` and called +// `e1000_transmit` directly, which meant the second NIC driver would have +// arrived with a second near-identical copy of this file. See include/netdev.h. +// +// One netif per bound interface, up to NETIF_MAX. Two NICs is an ordinary +// machine -- a laptop with Ethernet and the virtual one QEMU adds is already +// two -- and the limit exists only because a fixed array is simpler than +// allocation for something enumerated once at boot. + +#include "types.h" +#include "defs.h" +#include "param.h" +#include "device.h" +#include "netdev.h" + +#include "lwip/opt.h" +#include "lwip/netif.h" +#include "lwip/pbuf.h" +#include "lwip/etharp.h" +#include "lwip/ethip6.h" +#include "netif/ethernet.h" + +#define ETH_MTU 1500 +#define ETH_FRAMESZ 1518 // 1500 payload + 14 header + 4 FCS allowance + +struct eth_if { + struct netif netif; + struct device *dev; + // Frames in and out, for /proc/netif. Counted at this seam rather than in + // the driver on purpose: a frame the driver received and this file dropped + // never reached the stack, and a counter one layer down would say it did. + uint64 rx_frames, tx_frames; + int used; +}; + +static struct eth_if ifs[NETIF_MAX]; +static int nifs; + +// One staging buffer each, not one per call: a frame is 1518 bytes and the +// kernel stack is 4096 in total, so a local of this size on a path that is +// several frames deep is exactly the overflow the kstack canary exists to +// catch. Safe as a single static because every caller holds the lwIP lock -- +// which is also why one pair serves every interface. +static char txframe[ETH_FRAMESZ]; +static char rxframe[ETH_FRAMESZ]; + +// Send one packet. lwIP hands over a pbuf chain; a driver wants one flat +// buffer, so this walks the chain into txframe. +static err_t +eth_linkoutput(struct netif *netif, struct pbuf *p) +{ + struct eth_if *e = netif->state; + + if (p->tot_len > sizeof(txframe)) + return ERR_IF; + + u16_t off = 0; + for (struct pbuf *q = p; q != NULL; q = q->next) { + if (off + q->len > sizeof(txframe)) + return ERR_IF; + memmove(txframe + off, q->payload, q->len); + off += q->len; + } + + // A full transmit ring is congestion, not failure. Reporting ERR_IF would + // make lwIP drop the segment and wait for a retransmission timer; + // ERR_WOULDBLOCK lets it try again, which for TCP is the difference between + // a stall of milliseconds and one of seconds. + if (netdev_transmit(e->dev, txframe, off) != 0) + return ERR_WOULDBLOCK; + + e->tx_frames++; + return ERR_OK; +} + +// Bring one interface up. Called by netif_add, once per device. +static err_t +eth_netif_init(struct netif *netif) +{ + struct eth_if *e = netif->state; + const unsigned char *mac = netdev_mac(e->dev); + + netif->name[0] = 'e'; + netif->name[1] = 'n'; + netif->output = etharp_output; // IP -> ARP -> linkoutput + netif->linkoutput = eth_linkoutput; + netif->mtu = ETH_MTU; + netif->hwaddr_len = 6; + for (int i = 0; i < 6; i++) + netif->hwaddr[i] = mac ? mac[i] : 0; + netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | NETIF_FLAG_LINK_UP; + return ERR_OK; +} + +// Attach one network device. Returns its interface index, or -1. +// +// No address: DHCP fills that in, and an interface that came up with a +// hardcoded one would be wrong on every network but the one it was written on. +int +net_eth_attach(struct device *dev) +{ + ip4_addr_t any; + + if (nifs >= NETIF_MAX || dev == 0) + return -1; + + struct eth_if *e = &ifs[nifs]; + e->dev = dev; + e->used = 1; + + IP4_ADDR(&any, 0, 0, 0, 0); + if (netif_add(&e->netif, &any, &any, &any, e, + eth_netif_init, netif_input) == NULL) { + e->used = 0; + return -1; + } + + // The first one is the default route. With one interface that is the only + // possible answer; with two it is a policy decision that belongs to a + // routing table this kernel does not have yet, and picking the first is at + // least a rule rather than an accident of enumeration order. + if (nifs == 0) + netif_set_default(&e->netif); + + netif_set_up(&e->netif); + if (netdev_link_up(dev)) + netif_set_link_up(&e->netif); + else + netif_set_link_down(&e->netif); + + return nifs++; +} + +int +net_eth_count(void) +{ + return nifs; +} + +struct netif * +net_eth_netif(int i) +{ + if (i < 0 || i >= nifs) + return 0; + return &ifs[i].netif; +} + +struct device * +net_eth_device(int i) +{ + if (i < 0 || i >= nifs) + return 0; + return ifs[i].dev; +} + +void +net_eth_counts(int i, uint64 *rx, uint64 *tx) +{ + if (i < 0 || i >= nifs) { + *rx = *tx = 0; + return; + } + *rx = ifs[i].rx_frames; + *tx = ifs[i].tx_frames; +} + +// Take whatever the drivers have and push it into lwIP. Returns how many +// frames moved across all interfaces, so the caller can tell a busy poll from +// an idle one. +// +// Bounded per interface rather than draining: a receive loop with no limit +// starves everything else on a busy network, and the caller comes straight +// back anyway. +int +net_eth_poll(void) +{ + int moved = 0; + + for (int k = 0; k < nifs; k++) { + struct eth_if *e = &ifs[k]; + + for (int i = 0; i < 32; i++) { + int len = netdev_receive(e->dev, rxframe, sizeof(rxframe)); + if (len <= 0) + break; + + // PBUF_POOL, so this comes from lwIP's own pool rather than a heap + // it does not have. A null return means the pool is exhausted, + // which is a dropped packet -- entirely normal under load, and what + // the protocol above is designed to survive. + struct pbuf *p = pbuf_alloc(PBUF_RAW, (u16_t)len, PBUF_POOL); + if (p == NULL) + break; + + pbuf_take(p, rxframe, (u16_t)len); + + // netif->input is netif_input, set at netif_add: it dispatches on + // the Ethernet type. Ownership of the pbuf passes with the call, + // including when it fails, so there is nothing to free here. + if (e->netif.input(p, &e->netif) != ERR_OK) + pbuf_free(p); + + e->rx_frames++; + moved++; + } + } + return moved; +} diff --git a/kernel/net/net.c b/kernel/net/net.c index 6020481..2073a9f 100644 --- a/kernel/net/net.c +++ b/kernel/net/net.c @@ -4,11 +4,15 @@ // // lwIP with NO_SYS=1 has no threads of its own, so something has to feed it // received frames and run its timers. That something is a kernel thread rather -// than the e1000 interrupt handler, and the reason is the lock: a call into +// than a driver's interrupt handler, and the reason is the lock: a call into // lwIP can reach the driver, the driver can sleep, and an interrupt handler // cannot. So the interrupt keeps doing what it always did -- clear the cause // register -- and this thread polls. // +// Nothing here names a driver. Interfaces come from the netdev class +// (include/netdev.h): every bound network device is attached, and a machine +// with two NICs gets two interfaces without a line changing in this file. +// // Polling costs a wakeup every few milliseconds when the link is idle. The // alternative, waking this thread from the interrupt, is a worthwhile change // later; it is deliberately not the first version, because a receive path that @@ -20,6 +24,8 @@ #include "spinlock.h" #include "sleeplock.h" #include "proc.h" +#include "device.h" +#include "netdev.h" #include "lwip/opt.h" #include "lwip/init.h" @@ -66,42 +72,63 @@ net_thread(void) lwip_sys_lock(); lwip_init(); - if (net_e1000_attach() != 0) { + + // Every bound network device becomes an interface. DHCP is started on each + // of them: which one answers is the network's business, not ours, and an + // interface left unconfigured because it happened to be second would be a + // rule nobody could see. + int attached = 0; + for (int i = 0; i < netdev_count(); i++) { + struct device *d = netdev_at(i); + int idx = net_eth_attach(d); + if (idx < 0) + continue; + dhcp_start(net_eth_netif(idx)); + attached++; + cprintf("net: %s attached as interface %d\n", d->name, idx); + } + + if (attached == 0) { lwip_sys_unlock(); - cprintf("net: no e1000; the network thread has nothing to drive\n"); + cprintf("net: no network device; the thread has nothing to drive\n"); // Not a panic and not an exit: a kernel thread must never return, and // a machine with no NIC is an ordinary machine. for (;;) wait_tick(); } - struct netif *nif = net_e1000_netif(); - dhcp_start(nif); lwip_sys_unlock(); - cprintf("net: lwIP %d.%d.%d up, requesting an address\n", - LWIP_VERSION_MAJOR, LWIP_VERSION_MINOR, LWIP_VERSION_REVISION); + cprintf("net: lwIP %d.%d.%d up on %d interface%s, requesting an address\n", + LWIP_VERSION_MAJOR, LWIP_VERSION_MINOR, LWIP_VERSION_REVISION, + attached, attached == 1 ? "" : "s"); - int announced = 0; + // One bit per interface, so a second one binding later is announced too. + int announced[NETIF_MAX] = { 0 }; for (;;) { int moved; lwip_sys_lock(); - moved = net_e1000_poll(); + moved = net_eth_poll(); sys_check_timeouts(); - // Report the address once, when it first arrives. Anyone who has spent + // Report each address once, when it first arrives. Anyone who has spent // an evening wondering whether the guest has an address at all wants // this on the console rather than in a file they have to know about. - if (!announced && !ip4_addr_isany_val(*netif_ip4_addr(nif))) { + for (int i = 0; i < attached; i++) { + struct netif *nif = net_eth_netif(i); + if (announced[i] || ip4_addr_isany_val(*netif_ip4_addr(nif))) + continue; + const ip4_addr_t *a = netif_ip4_addr(nif); const ip4_addr_t *m = netif_ip4_netmask(nif); const ip4_addr_t *gw = netif_ip4_gw(nif); - cprintf("net: %d.%d.%d.%d/%d.%d.%d.%d via %d.%d.%d.%d\n", + cprintf("net: %s %d.%d.%d.%d/%d.%d.%d.%d via %d.%d.%d.%d\n", + net_eth_device(i)->name, ip4_addr1(a), ip4_addr2(a), ip4_addr3(a), ip4_addr4(a), ip4_addr1(m), ip4_addr2(m), ip4_addr3(m), ip4_addr4(m), ip4_addr1(gw), ip4_addr2(gw), ip4_addr3(gw), ip4_addr4(gw)); - announced = 1; + announced[i] = 1; net_bound = 1; } lwip_sys_unlock(); @@ -183,65 +210,83 @@ put_field(char *out, const char *name) return n; } +// One block of fields per interface, separated by a blank line. A reader that +// can parse one can parse any number, which is the whole reason it is not a +// single flat list of values: with two NICs a flat list has no way to say which +// address belongs to which. int net_info(char *buf, int cap) { int len = 0; - if (cap < 512) + // Room for the largest block, so the loop below never needs a bounds check + // in the middle of writing one. + if (cap < 256 * (NETIF_MAX + 1)) return 0; - if (!net_started) { + if (!net_started || net_eth_count() == 0) { len += put_field(buf + len, "state"); - len += put_str(buf + len, e1000_present() ? "down\n" : "no interface\n"); + len += put_str(buf + len, netdev_count() ? "down\n" : "no interface\n"); return len; } - struct netif *nif = net_e1000_netif(); + for (int i = 0; i < net_eth_count(); i++) { + struct netif *nif = net_eth_netif(i); + struct device *dev = net_eth_device(i); - lwip_sys_lock(); + if (i) + buf[len++] = '\n'; - len += put_field(buf + len, "interface"); - buf[len++] = nif->name[0]; - buf[len++] = nif->name[1]; - len += put_u(buf + len, nif->num); - buf[len++] = '\n'; + lwip_sys_lock(); - len += put_field(buf + len, "mac"); - len += put_mac(buf + len, nif->hwaddr); - buf[len++] = '\n'; + len += put_field(buf + len, "interface"); + len += put_str(buf + len, dev->name); + buf[len++] = '\n'; - len += put_field(buf + len, "state"); - len += put_str(buf + len, net_bound ? "up\n" : "no address\n"); + len += put_field(buf + len, "driver"); + len += put_str(buf + len, dev->driver ? dev->driver : "-"); + buf[len++] = '\n'; - len += put_field(buf + len, "address"); - len += put_ip4(buf + len, netif_ip4_addr(nif)); - buf[len++] = '\n'; + len += put_field(buf + len, "mac"); + len += put_mac(buf + len, nif->hwaddr); + buf[len++] = '\n'; - len += put_field(buf + len, "netmask"); - len += put_ip4(buf + len, netif_ip4_netmask(nif)); - buf[len++] = '\n'; + // Per interface, not per system: with two NICs one can be configured + // while the other is still asking. + len += put_field(buf + len, "state"); + len += put_str(buf + len, + ip4_addr_isany_val(*netif_ip4_addr(nif)) ? "no address\n" + : "up\n"); - len += put_field(buf + len, "gateway"); - len += put_ip4(buf + len, netif_ip4_gw(nif)); - buf[len++] = '\n'; + len += put_field(buf + len, "address"); + len += put_ip4(buf + len, netif_ip4_addr(nif)); + buf[len++] = '\n'; - len += put_field(buf + len, "mtu"); - len += put_u(buf + len, nif->mtu); - buf[len++] = '\n'; + len += put_field(buf + len, "netmask"); + len += put_ip4(buf + len, netif_ip4_netmask(nif)); + buf[len++] = '\n'; - lwip_sys_unlock(); + len += put_field(buf + len, "gateway"); + len += put_ip4(buf + len, netif_ip4_gw(nif)); + buf[len++] = '\n'; - uint64 rx, tx; - net_e1000_counts(&rx, &tx); + len += put_field(buf + len, "mtu"); + len += put_u(buf + len, nif->mtu); + buf[len++] = '\n'; - len += put_field(buf + len, "rx_frames"); - len += put_u(buf + len, rx); - buf[len++] = '\n'; + lwip_sys_unlock(); - len += put_field(buf + len, "tx_frames"); - len += put_u(buf + len, tx); - buf[len++] = '\n'; + uint64 rx, tx; + net_eth_counts(i, &rx, &tx); + + len += put_field(buf + len, "rx_frames"); + len += put_u(buf + len, rx); + buf[len++] = '\n'; + + len += put_field(buf + len, "tx_frames"); + len += put_u(buf + len, tx); + buf[len++] = '\n'; + } return len; } @@ -258,9 +303,9 @@ netinit(void) // depends on that reasoning holding forever is not worth the two lines // saved. inet_init(); - if (!e1000_present()) { - // Silent: a machine without an e1000 is not misconfigured, and this - // runs on every boot. + if (netdev_count() == 0) { + // Silent: a machine with no network device is not misconfigured, and + // this runs on every boot. return; } net_started = 1; diff --git a/tools/windows/bootcheck.ps1 b/tools/windows/bootcheck.ps1 index ffb90af..32645cb 100644 --- a/tools/windows/bootcheck.ps1 +++ b/tools/windows/bootcheck.ps1 @@ -257,6 +257,11 @@ if ($Net) { # arrived" rather than "TCP is broken" -- two very different bugs that look # identical from a failed connect. $commands += 'cat /proc/netif' + # The raw driver path, underneath lwIP: an ARP request built by hand and + # the reply read back off the receive ring. It lives on the netdev class + # now rather than inside the e1000, so this is also the check that moving + # it did not break it. + $commands += 'echo n > /proc/sysrq' $commands += "nettest 10.0.2.2 $netPort" $commands += "nettest -u 10.0.2.2 $netUdpPort" } @@ -782,7 +787,12 @@ if ($Net) { # 10.0.2.15 is the address QEMU's user-mode networking hands out, so this # is DHCP genuinely completing -- discover, offer, request, ack -- and not # a hardcoded guess: nothing in the guest knows that number in advance. - Expect 'net: 10\.0\.2\.15/' 'DHCP obtained an address from SLIRP' + Expect 'net: eth\d 10\.0\.2\.15/' 'DHCP obtained an address from SLIRP' + # The interface is named for the device, and the device names the driver + # that claimed it. Together they say the stack went through the netdev + # class rather than calling a driver it was compiled to know about. + Expect '(?m)^interface\s+eth\d' '/proc/netif names the bound interface' + Expect '(?m)^driver\s+e1000' 'and the driver behind it' Expect '(?m)^state\s+up' '/proc/netif reports the interface up' Expect '(?m)^address\s+10\.0\.2\.15' '/proc/netif reports the address it was given' Expect '(?m)^gateway\s+10\.0\.2\.2' '/proc/netif reports the gateway' @@ -790,6 +800,7 @@ if ($Net) { # zero here means the counters are wrong rather than the network. Expect '(?m)^rx_frames\s+[1-9]' 'frames arrived and reached the stack' Expect '(?m)^tx_frames\s+[1-9]' 'frames were transmitted' + Expect 'netdev: ARP reply' 'the raw driver path carries a frame both ways' # And now real traffic. Each of these is a different part of the exchange, # so a failure says which one: the handshake, the guest's receive path, and