Re: QEMU net code and padding of short packets

On Tue, 21 Jul 2026 at 12:34, Bin Meng <[email protected]> wrote:
>
> Hi Peter,
>
> On Tue, Jul 21, 2026 at 12:22 AM Peter Maydell <[email protected]> wrote:
> >
> > On Mon, 29 Jun 2026 at 17:31, Peter Maydell <[email protected]> wrote:
> > > I'm also looking at a variant of this involving vlan headers:
> > > https://gitlab.com/qemu-project/qemu/-/work_items/3518
> > >
> > > In commit 63b901bfd30 you removed the code from rtl8139.c
> > > which handled padding out short frames in the device's
> > > receive path, because we now do the padding at the other end.
> > > However, in this case the padding code in the device was
> > > also handling a case where there's an 802.1Q vlan tag: it makes
> > > the buffer at least MIN_BUF_SIZE + VLAN_HLEN == 64 bytes, with
> > > the comment "Include some tailroom in case a vlan tag is
> > > later removed". Then later we use this to avoid the copy
> > > from the buffer reading off the end, but now the copy can
> > > read 4 bytes of data off the end of the packet into guest
> > > memory.
> > >
> > > How should we handle this? We could:
> > >  - reinstate the rtl8139 padding code
> > >  - make the generic padding code allow enough extra space
> > >    for the 802.1Q vlan tag (so devices can assume that the
> > >    frame is at least its minimum size even if there's a
> > >    vlan tag)
> > >  - have rtl8139 special case this
> >
> > Ping? How do you think we should handle the "short packet
> > with a vlan header" case? I think I lean towards making
> > the generic code deal with it.

> For the VLAN case, after looking at it more closely, I
> agree that the generic padding code should handle it. A single-tagged
> Ethernet frame has a minimum length of ETH_ZLEN + VLAN_HLEN == 64
> bytes without FCS, so padding it to 64 is not merely providing private
> tailroom for rtl8139. Ideally the helper could derive the minimum from
> the L2 header length, which would also cover double-tagged frames.

Having looked at the 802.1Q spec, I found that it actually says
that a single-tagged ethernet frame's minimum length is still
60 bytes without FCS. So I now think the problem is that the rtl8139
code is unnecessarily trying to pad out the input tagged frame's
payload more than it needs to, and the fix is just to stop doing that.

-- PMM