Re: [PATCH v2 06/12] packet: Don't hard code maximum packet size to UINT16_MAX

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 20 Dec 2024 19:35:29 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> We verify that every packet we store in a pool - and every partial packet
> we retreive from it has a length no longer than UINT16_MAX.  This
> originated in the older packet pool implementation which stored packet
> lengths in a uint16_t.  Now, that packets are represented by a struct
> iovec with its size_t length, this check serves only as a sanity / security
> check that we don't have some wildly out of range length due to a bug
> elsewhere.
> 
> However, UINT16_MAX (65535) isn't quite enough, because the "packet" as
> stored in the pool is in fact an entire frame including both L2 and any
> backend specific headers.  We can exceed this in passt mode, even with the
> default MTU: 65520 bytes of IP datagram + 14 bytes of Ethernet header +
> 4 bytes of qemu stream length header = 65538 bytes.
> 
> Introduce our own define for the maximum length of a packet in the pool and
> set it slightly larger, allowing 128 bytes for L2 and/or other backend
> specific headers.  We'll use different amounts of that depending on the
> tap backend, but since this is just a sanity check, the bound doesn't need
> to be 100% tight.

I couldn't find the time to check what's the maximum amount of bytes we
can get here depending on hypervisor and interface, but if this patch
fixes an actual issue as you seem to imply, actually checking that with
QEMU and muvm would be nice.

By the way, as you mention a specific calculation, does it really make
sense to use a "good enough" value here? Can we ever exceed 65538
bytes, or can we use that as limit? It would be good to find out, while
at it.

-- 
Stefano