net: add a new flag NET_FLAG_INCLUDE_VNET_HEADER by akerouanton · Pull Request #493 · containers/libkrun

akerouanton · 2025-12-22T06:05:15Z

This flag should be used to indicate to libkrun that downstream network backend wants to receive and transmit the virtio-net header along with Ethernet frames.

Network backends using this flag can then forward unmodified headers to another VM or build a sensible virtio_net_hdr (e.g. with GSO fields correctly set) such that receiving VM handles GSO'd frames properly.

mtjhrc

Thanks for PR, it certainly makes sense to expose the header to interested backends.
I am curious though, could you share more detail, what backend you would use with this?

Please fix the mentioned confusion between stream/datagram sockets and features/flags. Also I guess it would be nice to also implement this for stream sockets too, but not strictly necessary.

mtjhrc · 2025-12-22T14:29:43Z

src/libkrun/src/lib.rs

+    if (features & !(NET_ALL_FEATURES | NET_FLAG_INCLUDE_VNET_HEADER)) != 0 {
        return -libc::EINVAL;
    }


This check is incorrect on 2 levels:

as it stands this PR only adds support for this feature for unix datagrams, not streams (which this function is configuring) .

the NET_FLAG_INCLUDE_VNET_HEADER is a flag not a feature (as correctly in the krun_add_net_unixgram).

mtjhrc · 2025-12-22T15:44:58Z

src/libkrun/src/lib.rs

        return -libc::EINVAL;
    }

+    let include_vnet_header: bool = flags & NET_FLAG_INCLUDE_VNET_HEADER != 0;


As implied above, include_vnet_header, should always be false here, because it's not implemented for the stream socket.

mtjhrc · 2025-12-22T15:48:32Z

src/libkrun/src/lib.rs

    let backend = if let Some(path) = path {
        VirtioNetBackend::UnixgramPath(path, send_vfkit_magic)
    } else {
        VirtioNetBackend::UnixgramFd(fd)
    };


If you only implement the flag for the unixgram sockets, I think it would be better to put the bool inside the enum (like send_vfkit_magic is).

Note: I am not sure why send_vfkit_magic would not passed into VirtioNetBackend::UnixgramFd, that seems like a bug.

akerouanton · 2025-12-23T10:20:43Z

I am curious though, could you share more detail, what backend you would use with this?

I tried for a couple hours to get a kernel stacktrace to pinpoint exactly where's the issue and why this PR fixes it, but to no avail. Let me describe that with plain words instead, but let me know if you want something more concrete.

I don't plan to use this flag with any existing backend right now. I'm working on a container runtime that leverages https://github.com/containerd/nerdbox and has its own virtual switch that connects multiple VMs. Every VM gets one or more virtio-net devices, and each interface is connected to a bridge interface (i.e. ip link set master). That bridge has a veth connected to it too (and at some point, it'll get multiple of them but nerdbox doesn't support multiple-containers-per-VM yet).

With TSO enabled, VM-to-VM connections are working correctly — packets are GSO'd on the sending side, and GRO'd on the receiving side. However, container-to-container connections are broken. After some debugging, I realized that disabling GSO on the sending veth fixed the issue and that was probably caused by invalid GSO fields in virtio-net headers.

I believe that with the above topology GRO is supposed to happen at the veth level (although I might be wrong), and missing GSO fields would cause skbs to have some fields unset, preventing GRO to happen.

With this change, my virtual switch is forwarding virtio-net headers sent by the sending side without recomputing L4 checksums (which was required for VM-to-VM scenarios to work), and it fixes container-to-container communications too. I also think that this should help with host-to-[VM|container] communications as it'd allow to send large frames with partial checksums.

Please fix the mentioned confusion between stream/datagram sockets and features/flags. Also I guess it would be nice to also implement this for stream sockets too, but not strictly necessary.

Sorry for the nasty mistakes, let me fix them!

akerouanton · 2025-12-23T17:57:37Z

@mtjhrc I added include_vnet_header to all network backends but to keep the bespoke 'frame length' header used by the unixstream backend, I had to extend NetWorker's TX buffer by 4 bytes. The vnet header is now located at tx_frame_buf[4..14].

Unixstream either writes the frame length to 0..4 or 10..14 dependening on whether the vnet header is included. Other backends have been modified to ignore the first four bytes.

The Tap backend also takes an include_vnet_header to keep it aligned with other backends. But since vnet headers have always been included, there's no way to make that configurable without introducing a breaking change in the API. Maybe something to fix in a major API version, or via a new API method, or maybe it's not worth it (in that case I can remove my change if you prefer).

Anyway, this should be in a better shape. PTAL.

slp · 2026-01-09T10:50:35Z

@mtjhrc PTAAL

mtjhrc · 2026-01-20T09:28:28Z

Hi, sorry for the late reply.
I would have aproved and merged this, but this seemed to be buggy at least with with unixstream. Right now I'm working on #385, #405. I'll integrate this feature there, along with tests - the device side would be completely different - this removes the intermediary buffer entirely).

This flag should be used to indicate to libkrun that downstream network backend wants to receive and transmit the virtio-net header along with Ethernet frames. Network backends using this flag can then forward unmodified headers to another VM or build a sensible virtio_net_hdr (e.g. with GSO fields correctly set) such that receiving VM handles GSO'd frames properly. Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>

akerouanton · 2026-02-11T11:07:43Z

@mtjhrc @slp Do you plan to get #530 ready and merge it any time soon (i.e. before the next minor)? Otherwise, maybe we can consider merging this PR first and then get #530 in a subsequent release?

I would have aproved and merged this, but this seemed to be buggy at least with with unixstream.

I fixed that in a previous iteration, and fixed conflicts yesterday. I think it's good to go now.

slp · 2026-02-12T13:07:04Z

@mtjhrc @slp Do you plan to get #530 ready and merge it any time soon (i.e. before the next minor)? Otherwise, maybe we can consider merging this PR first and then get #530 in a subsequent release?

I think neither this one nor #530 qualify for a minor release. This will need to wait for 1.18, sorry.

akerouanton · 2026-02-12T14:29:11Z

To clarify, when I said next minor, I meant 1.18 not 1.17.x 🙂

mtjhrc · 2026-02-12T16:22:02Z

To clarify, when I said next minor, I meant 1.18 not 1.17.x 🙂

We should be able to get #530 before 1.18 anyway, but just in case:

Regarding this PR:
I think unixstream is still incorect (things around try_finish_write are suspicious, but I am removing the buffer as part of #530 anyway).

Since you care about the unixgram backend, I propose you drop the unixstream implementation from this PR after all and I will implement that as part of #530

The unixgram implementation looks OK.

slp · 2026-02-12T16:23:11Z

To clarify, when I said next minor, I meant 1.18 not 1.17.x 🙂

We should be able to get #530 before 1.18 anyway, but just in case:

I'd prefer not touching net code in 1.17, but it'll definitely go in 1.18.

mtjhrc · 2026-02-12T16:28:07Z

To clarify, when I said next minor, I meant 1.18 not 1.17.x 🙂

We should be able to get #530 before 1.18 anyway, but just in case:

I'd prefer not touching net code in 1.17, but it'll definitely go in 1.18.

I meant merged into main before 1.18 release, not released as 1.17.x 😅

akerouanton requested review from MatiasVara, jakecorrenti, mtjhrc, slp and tylerfanelli as code owners December 22, 2025 06:05

akerouanton mentioned this pull request Dec 22, 2025

vm networking: add flag vnet_hdr containerd/nerdbox#78

Draft

mtjhrc requested changes Dec 22, 2025

View reviewed changes

akerouanton force-pushed the pass-virtionet-hdr-to-backend branch 3 times, most recently from 420da29 to ab2fe8b Compare December 23, 2025 17:56

mtjhrc mentioned this pull request Feb 4, 2026

Rewrite virtio-net IO to be more efficient #530

Draft

3 tasks

akerouanton force-pushed the pass-virtionet-hdr-to-backend branch from ab2fe8b to fa669df Compare February 10, 2026 08:35

Conversation

akerouanton commented Dec 22, 2025

Uh oh!

mtjhrc left a comment

Choose a reason for hiding this comment

Uh oh!

mtjhrc Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

mtjhrc Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

mtjhrc Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

akerouanton commented Dec 23, 2025

Uh oh!

akerouanton commented Dec 23, 2025

Uh oh!

slp commented Jan 9, 2026

Uh oh!

mtjhrc commented Jan 20, 2026

Uh oh!

akerouanton commented Feb 11, 2026

Uh oh!

slp commented Feb 12, 2026

Uh oh!

akerouanton commented Feb 12, 2026

Uh oh!

mtjhrc commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slp commented Feb 12, 2026

Uh oh!

mtjhrc commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mtjhrc commented Feb 12, 2026 •

edited

Loading