Rust code review and netdev

A fast-moving patch set—seemingly the norm for Linux networking development—seeks to add some Rust abstractions for physical layer (PHY) drivers. Lots of review has been done, and the patch set has been reworked frequently in response to those comments. Unfortunately, the Rust-for-Linux developers are having trouble keeping up with that pace. There is, it would appear, something of a disconnect between the two communities' development practices.

On October 26, Tomonori Fujita posted version 7 of his patches containing the infrastructure for creating some PHY drivers using Rust; it also includes a driver for the Asix AX88772A PHY. The first version was posted on October 2 and it was preceded by three RFCs starting on September 13. In response to the cover letter for version 7, Rust-for-Linux developer Miguel Ojeda suggested that Fujita slow the pace some:

This patch series has had 8 versions in a month. It would be better to wait more between revisions for this kind of patch series, especially when there is discussion still going on in the previous ones and it is a new "type" of code.

I understand you are doing it so that people can see the latest version (and thus focus on that), and that is helpful, but sending versions very quickly when the discussions are ongoing splits the discussions into several versions. That makes it more confusing for reviewers, and essentially discourages people from being able to follow everything.

But Andrew Lunn, who has been doing a lot of review of the patches, pointed out that the pace is what is expected for the Linux networking subsystem: "[...] netdev moves fast, review comments are expected within about 3 days". He said that new versions of a patch set should not be posted for at least 24 hours, but the fast pace is meant to drive a resolution sooner; "[...] posting a new version is a way to kick reviewers into action". But Rust-for-Linux developer Boqun Feng wondered if posting updated patch sets so frequently really meets that goal: "it's sometimes [frustrating] to see new version post but old comments were not resolved, it rather discourages reviewers".

Lunn also noted that networking patches do not require reviews in order to get merged; "If there is no feedback within three days, and it passes the CI [continuous-integration] tests, it likely will be merged." But Ojeda said that CI tests are not sufficient to determine if the abstractions are "well-designed or sound, which is the key property Rust abstractions need" (see this post for a description of "sound" in Rust terms); he hoped that there were humans in the loop. Lunn replied that humans ultimately decide whether to merge the code, but that API problems are just bugs like any other—they can be fixed later if any are found.

In another part of the thread, Ojeda described the differences he sees between the processes for the netdev community and that of the (nascent) Rust-for-Linux effort. There is a need for the project to do careful review of "a whole new subsystem API" in a new language, he said. Another difficulty is that the Rust-for-Linux project simply does not have enough reviewers and/or review bandwidth to keep up with the netdev pace. "So, if you appreciate/want/need our feedback, you will need to be a bit more patient."

Meanwhile, though, he also quoted parts of the netdev documentation that indicate posting a new revision is not the recommended path to speed things along. In particular: "Do not post a new version of the code if the discussion about the previous version is still ongoing, unless directly instructed by a reviewer." In fact, that is the norm in the rest of the kernel community: posting a new patch set before the discussion has died down is generally unwelcome.

Networking maintainer Jakub Kicinski acknowledged the Rust-for-Linux project's needs, but pointed out that the netdev process is meant to make it possible to handle an enormous patch volume. "Longer review cycles would make keeping track of patches and discussions unmanageable." He wondered if the Rust-for-Linux project would be less involved in patch review after the initial stages. Ojeda agreed that was the goal, but that the initial set of abstractions will require more review time:

In addition, for Rust, we are trying to get the very first abstractions that get into mainline as reasonably well-designed as we can (and ideally "sound"), so that they can serve as an example. This takes time, and typically several iterations.

There is also a need to spend some time on "getting people on the same page around what is expected from Rust code and abstractions", he said. There is more to it than just the design and logic in the code, as formatting is part of what needs to be hammered out. A "trivial example" of that is the maximum line length that Lunn had pointed out as a problem in his review. Ojeda said that the Rust-for-Linux project would like to have consistent formatting for its code throughout the kernel, hopefully maintained in an automated fashion, so those differences need to be ironed out kernel-wide if possible. The plan is to provide a solid foundation so that the networking developers "can actually handle any patches swiftly without having to wait on us"—but it is going to take some time to get there.

Lunn cautioned that it may be difficult to achieve the consistency the project is hoping for—at least for line lengths. The netdev community prefers 80-character lines, while other subsystems use 100-character lines and may not like the shorter lines. In the end, formatting is not really a technical problem, but a social issue, Lunn said:

In order to keep the netdev people happy, you are going to limit it to 80. But i would not be too surprised if another subsystem says the code would be more readable with 100, like the C code in our subsystem. We want Rust to be 100 as well.

Linux can be very fragmented like this across subsystems. Its just the way it is, and you might just have to fit in.

Ojeda seems adamant that the Rust code will be consistently formatted throughout the kernel, which is a departure from the (lack of) consistency of the existing C code. We will have to see how that plays out over time. The discussion highlights some of the problems that will need to be worked out when two disparate development communities "collide". The PHY abstractions and the driver using them would be the first Rust kernel feature that is visible to both driver writers and regular users, so it is no surprise that the developers want to take the time to get it right.

The Linux development pace is far faster than most other development projects, which may simply be too fast for the kernel Rust project—at least at this point. On the other hand, the Rust APIs are not something that have to be set in stone, as Lunn noted; they are internal to the kernel and, thus, can be changed as with other kernel APIs. One senses that the two projects will find a balance that works for both, perhaps assisted by conversations at the upcoming Linux Plumbers Conference in mid-November.