Skip to content

net_tap: hand a received VLAN tag to frontends that take it out of band - #4935

Open
Robert Nowotny (bitranox) wants to merge 4 commits into
microsoft:mainfrom
bitranox:vlan-pr-3
Open

Robert Nowotny (bitranox) wants to merge 4 commits into
microsoft:mainfrom
bitranox:vlan-pr-3

Conversation

@bitranox

@bitranox Robert Nowotny (bitranox) commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

A tap read holds the tag in the frame whatever the sender did, and net_tap passes it on that way. A netvsc guest reads the tag from the VLAN PPI and a MANA guest reads it from the completion, so neither gets what it expects. Size is the worse half: a full-size tagged frame is 1518 bytes, both frontends report buffers sized for the MTU, and net_tap drops anything over capacity. Smaller frames get through, so a ping can work on a VLAN where a full-size frame cannot.

Measured with a Linux guest on a VLAN device over MANA, on a downstream tree with the same capacity check: a 1514-byte tagged frame was answered 5 of 5 times, and a 1518-byte one was handed over by the tap and answered 0 of 5. netvsp has the same check by the code on main. I did not measure netvsc on main, because that tree has a receive path main lacks, which changes the result.

RxOffloadSupport gets a vlan_strip field. netvsp and gdma set it, and virtio-net leaves it clear because a virtio guest reads the tag from the frame. With it set, net_tap moves an outer 0x8100 tag into RxMetadata.vlan before the capacity check, which is why this lives in the backend: a frontend cannot strip in time. The field does not reach TUNSETOFFLOAD.

Two limits stay. An outer 0x88a8 tag stays in the frame because neither guest contract has a form for it, so a full-size 802.1ad frame still does not fit. And a MANA completion has only a 12-bit VLAN id, so PCP and DEI do not reach a MANA guest. That is the completion format, the same as on hardware.

Independent of the two transmit PRs (#4933, #4934).

Testing: cargo test -p net_tap --lib, cargo test -p net_tap --test tap_tests -- --test-threads=1, cargo test -p netvsp, cargo test -p gdma, cargo test -p net_mana, cargo test -p virtio_net. The new trials inject tagged frames into a real tap and read what the queue delivers: a full-size frame with the tag out of band and inline, 0x88a8, frames of 16, 17 and 18 bytes, and a partial checksum. Separate tests in netvsp and net_mana check what each frontend asks the backend for. The strip comes from our downstream networking tree and was tested there: a Linux guest on a VLAN device over netvsc and over a MANA VF receives full-size tagged frames, and a full-size 802.1ad frame still does not fit, as described above. I have not run this against a Windows guest. Re-tested on 2026-11-05, after our downstream tree caught up with main: a Linux guest on a VLAN device over netvsc and over a MANA VF still receives tagged TCP, UDP and ICMP, including full-size frames.

A tap read holds the tag in the frame whatever the sender did, and net_tap passed
it on like that. A netvsc or MANA guest reads the tag from the VLAN PPI or the
completion, and a full-size tagged frame is 4 bytes over a buffer sized for the
MTU, so net_tap drops it.

RxOffloadSupport gets a vlan field, set by netvsp and gdma. With it set, net_tap
moves an outer 0x8100 tag into RxMetadata.vlan before the capacity check. A
partial checksum start moves back by 4. An outer 0x88a8 tag stays in the frame,
as neither guest contract has a form for it.

The tests inject tagged frames into a real tap and read what the queue delivers,
and check what netvsp and gdma ask the backend for.
Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:53
@github-actions github-actions Bot added the unsafe Related to unsafe code label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

VLAN stripping leaves the TCP GSO header length unchanged, causing delivered metadata to count four payload bytes as header, including for DATA_VALID frames.

1 open issue
What changed in this PR

This PR lets the TAP backend deliver received 802.1Q VLAN tags out of band to netvsp and MANA, allowing full-size tagged frames to fit their receive buffers. It is independent of the linked transmit fixes.

Changes:

  • Add a receive VLAN capability, enabled by netvsp and GDMA but disabled for virtio-net.
  • Strip supported VLAN tags before TAP’s capacity check and populate receive metadata.
  • Add TAP receive tests and frontend capability checks.
File Description
vm/​devices/​virtio/​virtio_net/​src/​lib.rs Keep received VLAN tags inline.
vm/​devices/​net/​netvsp/​src/​test.rs Verify requests for out-of-band VLAN tags.
vm/​devices/​net/​netvsp/​src/​lib.rs Enable receive VLAN metadata on queues.
vm/​devices/​net/​net_tap/​tests/​tap_tests.rs Test VLAN stripping, capacity limits, and checksum offsets.
vm/​devices/​net/​net_tap/​src/​lib.rs Extract VLAN tags before checking receive capacity.
vm/​devices/​net/​net_mana/​src/​test.rs Verify GDMA requests out-of-band VLAN tags.
vm/​devices/​net/​net_consomme/​src/​tests.rs Update receive capability fixtures.
vm/​devices/​net/​net_backend_core/​src/​lib.rs Define the receive VLAN capability.
vm/​devices/​net/​gdma/​src/​bnic.rs Request VLAN metadata for MANA completions.

Review effort: Balanced

Comment thread vm/devices/net/net_tap/src/lib.rs Resolved
Stripping the tag rebased csum_start but left hdr_len, so a stripped
GSO frame reported a header 4 bytes longer than it is. hdr_len now
moves too, for NEEDS_CSUM and DATA_VALID frames alike.
Copilot AI balanced review requested due to automatic review settings November 5, 2026 01:02
This reverts commit 9150d7a. The tap's hdr_len never counts a tag that
is out of the skb head: virtio_net_hdr_from_skb sets it to skb_headlen()
and adds vlan_hlen only to csum_start, and tun passes vlan_hlen only for
a tag in skb->vlan_tci, which is where received traffic carries it. So
taking 4 off hdr_len makes it short on real frames. It only counts the
tag when a frame is written to the tap with the tag inline, as a packet
socket does.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the out-of-band VLAN receive changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 5, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified for the receive VLAN handling changes.

No open issues 🎉

Review effort: Balanced

@github-actions

Copy link
Copy Markdown

The old name read as "VLAN is supported". The flag asks the backend to
take a received 802.1Q tag out of the frame and report it in
RxMetadata::vlan, so name it for that.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the VLAN receive-handling changes.

No open issues 🎉

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants