On 17/10/2024 02:10, Stefano Brivio wrote: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.+ 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).We had this same issue on the non-vhost-user path until commit a469fc393fa1 ("tcp, tap: Don't increase tap-side sequence counter for dropped frames") (completely reworked with time). There, it was pretty bad with small (default) values for wmem_max and rmem_max. Now, I_guess_ with vhost-user it won't be so easy to hit that, because virtqueue buffers are (altogether) bigger, so we can probably fix this later, but if it's not exceedingly complicated, we should consider fixing it now. If we hit something like that, the behaviour is pretty bad, with constant retransmissions and stalls. The mapping between queued frames and connections is done in tcp_data_to_tap(), where tcp4_frame_conns[] and tcp6_frame_conns[] items are set to the current (highest) value of tcp4_payload_used and tcp6_payload_used. Then, if we fail to transmit some frames, tcp_revert_seq() uses those arrays to revert the seq_to_tap values. I guess you could make vu_queue_fill_by_index() return an error, propagate it, and make vu_flush() call something like tcp_revert_seq() in case.Thanks, Laurent