First, a correction to my previous message: the AF_PACKET injection in
that run never actually worked. I took the "ifindex" from the ifreq
returned by TUNSETIFF, but TUNSETIFF fills in ifr_flags there (the
values 4098/20482 I printed were BROADCAST|MULTICAST|VNET_HDR flag
bits), so every sendto failed with ENODEV and no packet was ever
injected. The all-0xAA buffer in that run only shows that no packet
was processed at all, and the "payload starts at offset 24" claim was
wrong. Sorry for the noise.
I fixed the injection (SIOCGIFINDEX by name) and re-ran both
configurations on 7.3.0-rc7 with a 64-byte broadcast frame injected on
the tap. With IFF_VNET_HDR enabled (plus TUNSETVNETHDRSZ(24)) the
result is worse than stale bytes: the packet is consumed from the tun
queue and then silently discarded, so the guest sees nothing at all:
tunnel mode (no IFF_VNET_HDR), sendto 64 bytes ok:
hdr: 00 00 00 00 00 00 00 00 00 00 01 00
aa aa aa aa aa aa aa aa aa aa aa aa <- stale 12..23
ff ff ff ff ff ff 02 02 ... <- our frame at 24
tunnel-vnet-hdr mode (IFF_VNET_HDR | TUNSETVNETHDRSZ(24)),
same 64-byte sendto ok:
hdr: aa aa aa aa ... (all 32 bytes untouched)
The guest buffer is never touched in the second run. The packet was
read out of the tun queue and then dropped.
The reason is a peek/read size disagreement:
- peek_head_len() returns head->len, i.e. the raw skb->len. The tun
vnet header is not in the skb; tun_put_user() adds vnet_hdr_sz
bytes at read time.
- With VHOST_NET_F_VIRTIO_NET_HDR negotiated, sock_hlen = 0, so
handle_rx() computes sock_len = peek = skb->len (64) and calls
recvmsg(sock, &msg, 64, ...) with a msg_iter sized for
vhost_len = 64 + vhost_hlen(24) = 88.
- tun_do_read() delivers skb->len + vnet_hdr_sz = 64 + 24 = 88 bytes
and returns 88.
- handle_rx() then sees err (88) != sock_len (64) and takes the
"Discarded rx packet" path: vhost_discard_vq_desc() rolls back the
descriptor, but the skb has already been dequeued from the tun
receive queue. The frame is gone.
So for a tun backend, the three combinations are:
VHOST_NET_F_VIRTIO_NET_HDR | IFF_VNET_HDR | result
---------------------------+--------------+--------------------------
off | on | ok (classic: tun header
| | passed through)
on | off | header written by vhost,
| | bytes 12..23 stale (the
| | patch under discussion)
on | on | every packet consumed and
| | dropped, guest RX blackout
VHOST_NET_F_VIRTIO_NET_HDR means "vhost supplies the header, the socket
data has none", so it is only consistent with a backend whose
vnet_hdr_sz is 0. When the backend has vnet_hdr_sz > 0 the peek and
the read disagree and handle_rx() eats every frame.
Given that, I don't think disabling "vhost + !IFF_VNET_HDR" is the
right fix: that is exactly the combination VHOST_NET_F_VIRTIO_NET_HDR
is for, and it works apart from the 12 stale bytes my patch addresses.
The combination that is plainly broken is "vhost supplies the header"
together with a backend that supplies one (IFF_VNET_HDR set), which
today results in a silent total RX blackout.
So I'd suggest one of:
1. Reject VHOST_NET_F_VIRTIO_NET_HDR (or refuse the backend attach)
when the backend's vnet_hdr_sz is nonzero, so both broken
combinations become impossible; keep my patch for the stale bytes
in the remaining valid combination. The backend header size could
be queried at attach time the same way TUNSETIFF-derived sockets
are handled today.
2. Keep all combinations and fix both symptoms: my patch for the
stale bytes, plus making the peek path account for the backend
header so peek and read agree.
Happy to test whichever you prefer.
Best,
Guikun Yang
