Re: [PATCH v8 7/8] vhost-user: add vhost-user

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 14 Nov 2024 11:20:09 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> On 17/10/2024 02:10, Stefano Brivio wrote:
> >> +		if (frame_size == 0)
> >> +			first = &iov_vu[i + 1];
> >> +
> >> +		if (iov_vu[i + 1].iov_len > (size_t)len)
> >> +			iov_vu[i + 1].iov_len = len;
> >> +
> >> +		len -= iov_vu[i + 1].iov_len;
> >> +		iov_used++;
> >> +
> >> +		frame_size += iov_vu[i + 1].iov_len;
> >> +		num_buffers++;
> >> +
> >> +		if (frame_size >= mss || len == 0 ||
> >> +		    i + 1 == iov_cnt || !vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
> >> +			if (i + 1 == iov_cnt)
> >> +				check = NULL;
> >> +
> >> +			/* restore first iovec base: point to vnet header */
> >> +			vu_set_vnethdr(vdev, first, num_buffers, l2_hdrlen);
> >> +
> >> +			tcp_vu_prepare(c, conn, first, frame_size, &check);
> >> +			if (*c->pcap)  {
> >> +				tcp_vu_update_check(tapside, first, num_buffers);
> >> +				pcap_iov(first, num_buffers,
> >> +					 sizeof(struct virtio_net_hdr_mrg_rxbuf));
> >> +			}
> >> +
> >> +			conn->seq_to_tap += frame_size;  
> > We always increase this, even if, later...
> >   
> >> +
> >> +			frame_size = 0;
> >> +			num_buffers = 0;
> >> +		}
> >> +	}
> >> +
> >> +	/* release unused buffers */
> >> +	vu_queue_rewind(vq, iov_cnt - iov_used);
> >> +
> >> +	/* send packets */
> >> +	vu_flush(vdev, vq, elem, iov_used);  
> > we fail to send packets, that is, even if vu_queue_fill_by_index()
> > returns early because (!vq->vring.avail).  
> 
> vring.avail is a pointer to a structure. vring.avail is NULL if there is something wrong 
> during the initialization. It's imported code, I think it's only a sanity check.
> So in theory vu_flush() cannot fail.

Oh, I see now. I actually think it's preferable to crash in that
(theoretically impossible) case, without even an ASSERT() (we would
dereference a NULL pointer, eventually, even if not here).

-- 
Stefano