On 14/11/2024 15:23, Stefano Brivio wrote:
On Thu, 14 Nov 2024 11:20:09 +0100 Laurent Vivier
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).
So what you propose is to remove the "if (!vq->vring.avail) return;"? Thanks Laurent