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

[Laurent Vivier <lvivier@redhat.com>]

On 14/11/2024 15:23, Stefano Brivio wrote:
> 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...).
> 

If I add a call to tap_sock_reset() in tcp_vu_data_from_sock(), I need to remove all the 
"const" from "struct ctx *" in the entire call chain. It's a lot. I'm not sure it is worth it.

Thanks,
Laurent