Kernel/USB+Net: Add a USB CDC-ECM Network Adapter Driver by Hendiadyoin1 · Pull Request #26471 · SerenityOS/serenity

Hendiadyoin1 · 2025-12-08T22:58:09Z

Kernel/xHCI: Explicitly allow 0-sized normal transfers

Kernel/USB: Allow fetching string descriptor

Kernel/USB: Add a basic CDC driver skeleton

This currently only dumps part of the descriptor information when a CDC
device is connected.

Kernel/Net: Add a USB CDC-ECM Network Adapter Driver

This crudely plugs into the new CDC driver framework and provides a
basic implementation for ECM network adapters.

To test this you need to force the run.py to use a usb-net device,
in-place of the default e1000 network device.

prepare_to_unmount() calls prepare_to_clear_last_mount() while holding this lock, which flushes the superblock in the case of ext2. We shouldn't hold a spinlock during such expensive operations, especially since writing the superblock to disk will likely cause us to block.

`initialize_device()` used to hold the slot state lock. Since it submits (blocking) control transfers, `handle_transfer_event()` can be scheduled on another core to handle the result of those transfers. But that function also tries to acquire the same lock, resulting in a deadlock. `initialize_device()` and `handle_transfer_event()` don't need concurrent access to the endpoint ring, so protecting it with a separate prevents the deadlock. `enqueue_transfer()` is split into a `_impl` version to avoid double-locking the endpoint ring.

We are blocking while holding these locks, so they shouldn't be spinlocks.

This member is locked during unmount(), which causes the superblock to be written do disk for ext2. We shouldn't hold a spinlock during such expensive operations, especially since writing the superblock to disk will likely cause us to block.

Hendiadyoin1 · 2025-12-10T21:16:45Z

Kernel/Bus/USB/Drivers/CDC/Driver.cpp

+        dmesgln("USB CDC:           MAC Address String Index: {}", i_macaddr);
+        auto maybe_mac_address = device.get_string_descriptor(i_macaddr);
+        if (maybe_mac_address.is_error()) {
+            dmesgln("USB CDC:           Failed to get MAC address string descriptor");
+        } else {
+            dmesgln("USB CDC:           MAC Address: {}", maybe_mac_address.value()->view());
+        }


I think just printing the mac address should be fine here?

Hendiadyoin1 · 2025-12-10T21:20:20Z

Kernel/Bus/USB/Drivers/CDC/Driver.cpp

+        if (interface.descriptor().interface_class_code == USB_CLASS_COMMUNICATIONS_AND_CDC_CONTROL) {
+            control_interface = interface;
+        } else if (interface.descriptor().interface_class_code == USB_CLASS_CDC_DATA) {
+            // FIXME: Think of a better way to select the data interface if multiple are present
+            //        ECM for example will always have an empty one and one with two endpoints
+            if (data_interface.has_value()) {
+                if (data_interface->endpoints().size() < interface.endpoints().size()) {
+                    data_interface = interface;
+                }
+            } else {
+                data_interface = interface;
+            }
+        }
+    }


So this needs a multi function fixme and we should be able to look for only the main one and get the related ones from union descriptors if they have them

Then possibly gather all related interfaces together into a "function" collection and query on that in driver selection

Hendiadyoin1 · 2025-12-10T21:25:00Z

Kernel/Net/USB/CDCECM.cpp

+    }
+    dmesgln("USB CDC-ECM: Using MAC address: {}", TRY(mac_address.to_string()));
+
+    TRY(device.set_configuration_and_interface(data));


We should actually delay this until the intialize call as this will trigger some events we want to listen for
Namely the link up and speed change ones

the config setting should probably stay though or be moved up

Hendiadyoin1 · 2025-12-10T21:26:13Z

Kernel/Net/USB/CDCECM.cpp

+    auto in_pipe = TRY(BulkInPipe::create(device.controller(), device, in_pipe_endpoint_number, in_max_packet_size));
+    auto out_pipe = TRY(BulkOutPipe::create(device.controller(), device, out_pipe_endpoint_number, out_max_packet_size));


These should be valid even when they are created before the interface-alternative change?

Hendiadyoin1 · 2025-12-10T21:28:28Z

Kernel/Net/USB/CDCECM.cpp

+    NonnullOwnPtr<BulkInPipe> in_pipe,
+    NonnullOwnPtr<BulkOutPipe> out_pipe,
+    u16 max_segment_size)
+    : NetworkAdapter("cdc-ecm"sv) // FIXME: Choose the propper name


Any suggestions what the correct name would be here?

Kernel code should use KString instead. Compared to AK::String, KString is a very simple string class that just wraps a char array and doesn't have any fancy features such as reference counting.

Kernel code should use KString instead. Compared to AK::ByteString, KString is a very simple string class that just wraps a char array and doesn't have any fancy features such as reference counting. Additionally, AK::ByteString isn't OOM-safe at all currently.

We currently don't support loadable modules, so all callers of this function use `MUST()` on the result for now.

Vector::append() will be disallowed in the kernel by the next commit.

This makes it not as easy to forgot to handle OOMs in the kernel. This commit replaces most usages of this function with `try_append(...).release_value_but_fixme_should_propagate_errors()`. But in some cases, using the `TRY` macro or `unchecked_append()` is already possible. In places where allocations should not fail or an OOM would be fatal anyways, `MUST(try_append(...))` should be used explicitly.

We now store each segment's encoded data on the encoding context. This will be useful for implementing writing files in the random access organization, and it will hopefully also be useful in cases where we need to partially decode the file we're about to write (for example when refining whole text or halftone regions, or when refining the current page). This requires a few more copies than before, but it's small amounts of data. `compile.sh` time does not change measurably with this. No behavior change.

Files in random access organization first store all segment headers, followed by all segment data. (In contrast, the usual sequential organization writes each segment's header and data in one piece.) For that reason, files in random access organization *must* end with an end_of_file segment, while it's optional for files in sequential organization. Almost all jbig2 decoders in existence can only read jbig2 data when it's part of a PDF. JBIG2 data in a PDF is in the "embedded" organization, which is like the sequential organization without the file header. That means it's a bit difficult to test if random access files are correct. Having said that, `jbig2dec` decodes the files we're writing fine.

No behavior change.

This was blocked on not having test files. We're about to teach the writer to generate test files and to add test files, so we can add support for this now :^)

No behavior change.

...and add some spec comments. No behavior change.

The page refinement code will have to apply a crop to the page buffer. With this, that's easy to do. No behavior change.

Encoding a generic refinement region that refines the page contents requires that the writer has access to the current page contents. For some region types (text, halftone), computing the output bitmap isn't trivial, and handling all the composition operators and so on is also not trivial. Reimplementing all this in the writer doesn't seem ideal. So instead, when encountering a generic refinement region refining the page contents in the writer, collect the encoded segment data for all segments for the current page (and for the "global page" 0) that are in front of the refinement region, and call into the *loader* to decode the data so far, and then refine the bitmap returned by the loader. This requires storing the encoded data for each segment, but we do that already for implementing writing files in random access organization. It also requires making the list of segments available on JBIG2EncodingContext.

The decode ok in Preview.app, Acrobat Reader, poppler, and HEAD PDFium.

There are actually a couple of issues here: 1. We are not properly perfect-forwarding the iterable to the Enumerator member. We are using the class template as the constructor type, but we would actually have to do something like this to achieve perfect forwarding: template <typname Iter = Iterable> Enumerator(Iter&&) 2. The begin / end methods on Enumerator (although they return by const- ref) are making copies during for-each loops. The compiler basically generates this when we call enumerate: for (auto it = Enumerator::begin(); it != Enumerator::end(); ++it) The creation of `it` above actually creates a copy of the returned Enumerator instance. To avoid all of this, let's create an intermediate structure to act as the enumerated iterator. This structure does not hold the iterable and thus is fine to copy. We can then let the compiler handle forwarding the iterable to the Enumerator.

JBIG2ImageDecoderPlugin::create_embedded_jbig2_decoder() takes a list of segment data spans and the number of an intermediate segment, and returns the image data of that intermediate segment.

No behavior change.

jbig2enc, which is used by Google Books, sometimes writes files that are technically invalid. Allow that in permissive mode. To be able to check the referred-to segment's types, switch the loop from iterating over header.referred_to_segment_numbers to iterating over referred_to_segments directly (possible after the third commit of SerenityOS#26191, 72cd151). Fixes SerenityOS#26405.

Since it's easy to do now, only allow references to dead pattern dictionary segments in Power JBIG2 files, not to all segments. That's enough to still be able to decode all Power JBIG2 files. (Except for files 042_13.jb2, 042_14.jb2, which didn't decode before either, and which don't decode in any viewer that I tried.)

This adds writer support for refinement of a symbol dictionary entry that is itself a strip refinement. This case is a bit awkward for the writer: To write the refinement, we need the bitmap that's being refined. But since that's a strip refine, we don't have that bitmap, we only have the parts it consists of. As SerenityOS#26411 explains, the approach of calling into the loader to do the strip decoding doesn't work here: We need the decoded strip refinement while writing the strip itself. So we'd have to send a partially encoded strip to the loader, which isn't something we can currently do. But since strip refinements only use a limited subset of full text strips (always a background color of 0, always a reference corner of topleft, always a composition operator of "or"), reimplementing the loader's logic for this case, while conceptually not nice, is not a lot of code. So let's just do that. Using this, also add a test for this feature. I believe this tests the last missing refinement case :^) The test uses regular symbols for the eyes (symbols 0 and 1) and the mouth (symbol 6). The nose, 100x100 pixels, is conceptually split into 60x60, 40x60, 60x40, 40x40 tiles. Since symbols are sorted by increasing height, the latter two are symbols 2 and 3, the former are symbols 4 and 5. Symbol 3 isn't actually the bottom right part of the nose, but contains the pixel data of an eye. This is for testing refinement below. To test strip background color filling, the 60x60 tile is actually 59x60 pixels. And to test refinement, the top left tile is actually solid white instead of the top left part of the nose. The first refinement symbol (id 7) refines the top left tile to a 59x60 tile that's the top left corner of the nose, but shifted several pixels to the right. The second refinement symbol (id 8) is a strip refinement. It puts symbols 7 and 5 next to each other, to produce the top half of the nose (with the left part being shifted to the side). The third refinement symbol (id 9) is a simple refinement of symbol 8 refines it to the actual top of the nose (to fix the shift, and to test refinement of strips in a symbol dictionary). The fourth refinement symbol (id 10) is a strip refinement that puts symbol 9 in the first strip, with an offset of 1, and symbols 2 and 3 in a second strip. This means symbol 10 is almost the nose, but its bottom right contains the pixel data of an eye, since that's what symbol 3 contained. Finally, the text region refines symbol 10 to the actual nose pixels, to test refinement of strips from a text region. Whew! Since the text region refines to the nose, it's useful to locally remove that refinement to test that the reference bitmap looks as expected. Similarly, it's useful to locally make the final strip refinement put symbol 8 instead of symbol 9 in the first strip, to make sure the input to symbol 9 looks as expected. The test file decodes fine in all viewers I've tried (PDFium, Preview.app, pdf.js, mutool). (This case is much more awkward for the writer than for the loader, after all.)

The VirtIO Console uses a RingBuffer for transmit/receive operations, which assumes buffers are reclaimed in FIFO order. However, according to the VirtIO 1.2 specification, devices are not required to process descriptors in submission order unless VIRTIO_F_IN_ORDER is negotiated. When using QEMU 10.1+ with qemu-vdagent enabled, the virtio-console device may return buffers out-of-order, causing the RingBuffer's reclaim_space() to fail its FIFO assertion and trigger a kernel panic. This patch requires the VIRTIO_F_IN_ORDER feature to be available before initializing the Console device. If the feature is not supported, initialization fails gracefully with ENOTSUP and a clear error message. Fixes SerenityOS#26210

Replace plain size_t with Atomic<size_t> for mouse and keyboard minor number counters. This prevents race conditions during hot-plugging of input devices, where concurrent calls to generate_minor_device_number could result in duplicate minor numbers being assigned. The fetch_add operation ensures each device gets a unique minor number even when multiple devices are being initialized concurrently.

When a read or write syscall is interrupted by a signal, it returns -1 with errno set to EINTR. Following POSIX conventions, we should retry the syscall in this case rather than propagating the error. This matches the existing behavior in Core::File::close() which already handles EINTR by retrying in a loop. Fixes SerenityOS#26500

Previously, there were two slightly different implementations of this in the Kernel and in LibCrypto. This patch removes those, and replaces them with an implementation that seeks to combine the best aspects of both while not being a drop-in replacement for either. Two new tests are also included.

Fix common spelling mistakes: - "recieved" -> "received" - "recieves" -> "receives" - "reciever" -> "receiver" - "seperately" -> "separately"

Return a clear error message instead of TODO() panic when attempting to create XZ-compressed archives since compression is not yet implemented.

This is not the correct way to negotiate virtio features and causes this panic when running on a machine with a virtio-serial device, such as the default non-CI machine: "ASSERTION FAILED: m_did_accept_features.was_set()" Additionally, I don't think this is the correct way to fix this, as QEMU never seems to have advertised VIRTIO_F_IN_ORDER. A correct fix would be to handle descriptors being processed out of order correctly. This reverts commit 0d9a095.

No real behavior change. This makes delta width positive, which lets us encode the file with huffman encoding too. This file doesn't use huffman encoding, but it allows us to make the huffman version identical to this file (except for the "use huffman coding" flag).

This test is identical to bitmap-symbol-symbolrefine-textrefine.json, except with "uses_huffman_encoding": true, in the "flags" section of seach segment. While the arithmetically-coded version of this file decodes fine everywhere, the huffman-coded version decodes only correctly in Acrobat Reader, and almost correctly in Preview.app. PDFium, pdf.js, and mutool can't currently decode it.

On average, this patch reduces the error in luminosity between an input image and its dithered equivalent. This is done by correcting an off-by- one error in the threshold computation and also by making sure the range of mapped gray values to matrix pattern is of equal size for all ranges. This means that all values between 0 and 255 / (N * N + 1) (51 for Bayer2x2) will result in a black pixel while only a value of 0 would previously do it. The test works by first generating gray bitmap, then applying Bayer dithering on them and finally compare the luminosity of the two.

This currently only dumps part of the descriptor information when a CDC device is connected.

This crudely plugs into the new CDC driver framework and provides a basic implementation for ECM network adapters. To test this you need to force the `run.py` to use a `usb-net` device, in-place of the default e1000 network device.

… has started. This makes the USB CDC ECM driver work in the cases where USB discovery takes longer, or devices are plugged in later.

stale · 2026-01-24T18:30:43Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

github-actions · 2026-02-17T02:13:55Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

github-actions · 2026-02-24T02:14:09Z

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

spholz added 7 commits November 15, 2025 15:23

Kernel/xHCI: Replace some spinlocks with mutexes

a22dfa0

We are blocking while holding these locks, so they shouldn't be spinlocks.

Kernel/xHCI: Migrate m_command_completion_queue to WaitQueue

965a5b7

Kernel/xHCI: Migrate m_event_queue to WaitQueue

4ea10bb

Kernel/xHCI: Migrate SyncPendingTransfer::wait_queue to WaitQueue

d71ab17

github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Dec 8, 2025

Hendiadyoin1 marked this pull request as draft December 10, 2025 21:15

github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Dec 10, 2025

Hendiadyoin1 commented Dec 10, 2025

View reviewed changes

Hendiadyoin1 force-pushed the usb-cdc-ecm branch from aee2774 to 6be85af Compare December 13, 2025 21:29

spholz and others added 18 commits January 1, 2026 17:16

AK+Kernel: Don't allow AK::String in the kernel

a267901

Kernel code should use KString instead. Compared to AK::String, KString is a very simple string class that just wraps a char array and doesn't have any fancy features such as reference counting.

Kernel: Propagate errors in USBManagement::register_driver()

2323179

We currently don't support loadable modules, so all callers of this function use `MUST()` on the result for now.

AK: Guard some APIs that use Vector::append() behind #ifndef KERNEL

ca4e519

Vector::append() will be disallowed in the kernel by the next commit.

Tests/LibGfx: Add a JBIG2 file in random access organization

b6a3550

LibGfx/JBIG2Shared: Give RegionSegmentInformationField rect() accessor

fc06741

No behavior change.

LibGfx/JBIG2Loader: Add support for refinement regions refining the page

d77e42b

This was blocked on not having test files. We're about to teach the writer to generate test files and to add test files, so we can add support for this now :^)

LibGfx/JBIG2Loader: Make decode_embedded() return BilevelImage

00d4bb2

No behavior change.

LibGfx/JBIG2Writer: Put reference_bitmap computation into IIFE

02aa8d2

...and add some spec comments. No behavior change.

LibGfx/JBIG2Writer: Make refinement reference image a BilevelSubImage

3297442

The page refinement code will have to apply a crop to the page buffer. With this, that's easy to do. No behavior change.

Tests/LibGfx: Add JBIG2 tests for refinement regions refining the page

f88a74e

The decode ok in Preview.app, Acrobat Reader, poppler, and HEAD PDFium.

LibGfx/JBIG2Loader: Add method that returns intermediate region data

45a6195

JBIG2ImageDecoderPlugin::create_embedded_jbig2_decoder() takes a list of segment data spans and the number of an intermediate segment, and returns the image data of that intermediate segment.

LibGfx/JBIG2Writer: Make a lambda return NonnullRefPtr

0c4930f

No behavior change.

nico and others added 21 commits January 1, 2026 17:16

Everywhere: Fix spelling errors in comments

f7c00e0

Fix common spelling mistakes: - "recieved" -> "received" - "recieves" -> "receives" - "reciever" -> "receiver" - "seperately" -> "separately"

tar: Return error instead of panic for XZ compression

b25acde

Return a clear error message instead of TODO() panic when attempting to create XZ-compressed archives since compression is not yet implemented.

Tests/LibGfx: Test JBIG2 16-bit and 32-bit referred-to segment numbers

cb89db9

Kernel/xHCI: Explicitly allow 0-sized normal transfers

26efe19

Kernel/USB: Allow fetching string descriptor

cb341b1

Kernel/USB: Add a basic CDC driver skeleton

aaaa0bb

This currently only dumps part of the descriptor information when a CDC device is connected.

Kernel/Net: Add a USB CDC-ECM Network Adapter Driver

4b51ff2

This crudely plugs into the new CDC driver framework and provides a basic implementation for ECM network adapters. To test this you need to force the `run.py` to use a `usb-net` device, in-place of the default e1000 network device.

Wip: Refactor

5db876a

Kernel/Net: Initialize Network adapters outside of collection lock

9b9bbf5

Kernel/Net: Hackily allow registering NetworkAdapters after NetworkTask…

c6a5c05

… has started. This makes the USB CDC ECM driver work in the cases where USB discovery takes longer, or devices are plugged in later.

Hendiadyoin1 force-pushed the usb-cdc-ecm branch from 6be85af to c6a5c05 Compare January 2, 2026 16:20

stale bot added the stale label Jan 24, 2026

Hendiadyoin1 added ⛔️ pr-is-blocked PR is blocked by something outside of the author's control, protected from stalebot and removed stale labels Jan 26, 2026

github-actions bot added the stale label Feb 17, 2026

github-actions bot closed this Feb 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kernel/USB+Net: Add a USB CDC-ECM Network Adapter Driver#26471

Kernel/USB+Net: Add a USB CDC-ECM Network Adapter Driver#26471
Hendiadyoin1 wants to merge 236 commits intoSerenityOS:masterfrom
Hendiadyoin1:usb-cdc-ecm

Hendiadyoin1 commented Dec 8, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

Hendiadyoin1 Dec 10, 2025

Uh oh!

stale bot commented Jan 24, 2026

Uh oh!

github-actions bot commented Feb 17, 2026

Uh oh!

github-actions bot commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants

		auto in_pipe = TRY(BulkInPipe::create(device.controller(), device, in_pipe_endpoint_number, in_max_packet_size));
		auto out_pipe = TRY(BulkOutPipe::create(device.controller(), device, out_pipe_endpoint_number, out_max_packet_size));

Conversation

Hendiadyoin1 commented Dec 8, 2025

Kernel/xHCI: Explicitly allow 0-sized normal transfers

Kernel/USB: Allow fetching string descriptor

Kernel/USB: Add a basic CDC driver skeleton

Kernel/Net: Add a USB CDC-ECM Network Adapter Driver

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

stale bot commented Jan 24, 2026

Uh oh!

github-actions bot commented Feb 17, 2026

Uh oh!

github-actions bot commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants