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

[Stefano Brivio <sbrivio@redhat.com>]

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

> On 17/10/2024 02:10, Stefano Brivio wrote:
> >> +/**
> >> + * tcp_vu_prepare() - Prepare the packet header
> >> + * @c:		Execution context
> >> + * @conn:	Connection pointer
> >> + * @first:	Pointer to the array of IO vectors
> >> + * @dlen:	Packet data length
> >> + * @check:	Checksum, if already known
> >> + */
> >> +static void tcp_vu_prepare(const struct ctx *c,
> >> +			   struct tcp_tap_conn *conn, struct iovec *first,
> >> +			   size_t dlen, const uint16_t **check)
> >> +{
> >> +	const struct flowside *toside = TAPFLOW(conn);
> >> +	char *base = first->iov_base;
> >> +	struct ethhdr *eh;
> >> +
> >> +	/* we guess the first iovec provided by the guest can embed
> >> +	 * all the headers needed by L2 frame
> >> +	 */  
> > What happens if it doesn't (buggy guest)? Do we have a way to make sure
> > it's the case? I guess it's more straightforward to do this in
> > tcp_vu_data_from_sock() where we check and set iov_len (even though the
> > implication of VIRTIO_NET_F_MRG_RXBUF isn't totally clear to me).  
> 
> According to spec, minimum size of a buffer is 1526 bytes
> (https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-2340003)
> 
> So if the guest is buggy, we will write in the guest memory out of the (buggy) provided 
> buffer, and we can crash the guest. But it's what happens to a buggy guest.
> 
> We can't fix the guest, IMO passt should crash in this case (add an ASSERT()?).

I think we should rather call tap_sock_reset() (see tap_passt_input())
so that the guest has a chance to reconnect (you implemented this in
QEMU...).

-- 
Stefano