On Thu, Nov 28, 2024 at 01:57:34PM +0100, Laurent Vivier wrote:
> On 26/11/2024 06:24, David Gibson wrote:
> > > static int udp_vu_sock_recv(const struct ctx *c, int s, uint32_t events,
> > > +			    bool v6, ssize_t *dlen)
> > > +{
> > > +	struct vu_dev *vdev = c->vdev;
> > > +	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
> > > +	int iov_cnt, idx, iov_used;
> > > +	struct msghdr msg  = { 0 };
> > > +	size_t off, hdrlen;
> > > +
> > > +	ASSERT(!c->no_udp);
> > > +
> > > +	if (!(events & EPOLLIN))
> > > +		return 0;
> > > +
> > > +	/* compute L2 header length */
> > > +	hdrlen = udp_vu_hdrlen(v6);
> > > +
> > > +	vu_init_elem(elem, iov_vu, VIRTQUEUE_MAX_SIZE);
> > > +
> > > +	iov_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE,
> > > +			     IP_MAX_MTU - sizeof(struct udphdr) + hdrlen,
> > I don't think this calculation is quite right, though it's probably
> > safe.  At least for IPv4, IP_MAX_MTU includes the IP header itself,
> > but then you count that again in hdrlen.
> 
> I think it would be semantically more correct to use "ETH_MAX_MTU +
> sizeof(struct virtio_net_hdr_mrg_rxbuf)", but as ETH_MAX_MTU and IP_MAX_MTU
> are both defined to USHRT_MAX I'm not sure how to compute the segment
> size...

Huh, right.  I guess that means you can't quite put a maximum size IP
frame on ethernet, because you'll run out of Ethernet MTU before you
run out of IP MTU.

I think we should work based on the IP limits, not the L2 limit: our
operation means we're inherently connected to IP.  *Currently* we
always use Ethernet headers as well, but that could reasonably change
("tun" mode support, or other possible L2s).

So, that would give us:
	IP_MAX_MTU + ETH_HLEN + sizeof(virtio_net_hdr_mrg_rxbuf)

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson