Re: [RFC v3 4/8] virtio: Define the pasta vhost interface

["Ammar Yasser" <aerosound161@gmail.com>]

On Thu Aug 13, 2026 at 4:16 AM EEST, David Gibson wrote:
> On Wed, Aug 12, 2026 at 08:17:19PM +0300, Ammar Yasser wrote:
>> On Mon Aug 10, 2026 at 5:06 AM EEST, David Gibson wrote:
>> >>  
>> >>  /* Large enough for ~128 maximum size frames */
>> >> -#define PKT_BUF_BYTES		(8UL << 20)
>> >> +#define PKT_BUF_BYTES		((8UL << 20) + 1536)	/* 128 * sizeof(virtio_net_hdr_mrg_rxbuf) */
>> >
>> > I think the rationale for this change needs to be clearer (granted,
>> > the comment here beforehand is also kind of confusing).  IIRC - and
>> > based on the "~" in the comment, I don't think there's a strict
>> > requirement that this can hold 128 full frames - that's just setting a
>> > reasonable sense of scale, and then a round number was picked near it:
>> > ~64kiB * ~64 ~= 8MiB
>> >
>> > So, I'm not sure if this change is necessary - if it really is, we
>> > need a clearer analysis of why.
>> 
>> Because pkt_buf is now going to be the buffer where vhost guest->pasta
>> data but with the added size of the virtio_net_hdr_mrg_rxbuf for every
>> frame. So this is accounting for the worst case where the guest wants to
>> send a full 128 frames at maximum size at a time.
>
> Right, but that's not enough.  AFAICT pkt_buf is sized to allow
> *roughly* 128 full packets, but it doesn't strictly have to be able to
> contain that many.  If there's a reason it *must* have room for 128
> full frames with vhost-kernel, that needs to be pointed out
> explicitly.

Are you saying the size is not enough or the explanation is not ?
Also no, there's no explicit reason why it needs to have that many
packets in vhost-kernel. I saw that this was how pkt_buf was sized
before the vhost-kernel changes so i only added acounting for the virtio
net header

>> >> +#define VHOST_NDESCS (PKT_BUF_BYTES / 65520)
>> >
>> > I'm not sure 65520 is the right number here.  That's the max MTU at
>> > the IP level, but the packet buffer will also hold the 14 byte L2
>> > header.  I think you probably want one of the L2_MAX_LEN_* constants
>> > (or to define a new one for vhost-kernel).
>> 
>> My line of reasoning here is that "we typically expect, in the majority
>> of cases for an ethernet frame to span a single descriptor". evident by
>> how we eventually consume descriptors as we return the pointer into the
>> pkt_buf past the virtio_net header as the beginning of an ethernet
>> header. and this buffer should handle 128 frames (from the definition 
>> of PKT_BUF_BYTES) and so we want a denominator that yields a value as
>> close as possible to 128. Let me know if this isn't very sound. I will
>> investiagate the constants you mentioned anyways
>
> If the fundamental property you want is that you can fit 128
> descriptors, then you should just define NDESCS as 128, and derive the
> buffer size and other things from that.  If the basic property you
> want is something else, define that first and the rest in terms of it.

Fair enough, can do that