On 9/13/21 6:41 PM, Maxime Coquelin wrote: > > > On 8/18/21 4:13 PM, Andrew Rybchenko wrote: >> From: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru> >> >> Free Tx completed mbufs on device stop. Not completed Tx mbufs cannot be >> freed since they are still in use. >> >> Fixes: c1f86306a02 ("virtio: add new driver") >> Cc: sta...@dpdk.org >> >> Signed-off-by: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> >> --- >> drivers/net/virtio/virtio_ethdev.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/drivers/net/virtio/virtio_ethdev.c >> b/drivers/net/virtio/virtio_ethdev.c >> index e58085a2c9..ed3fefee7c 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -2393,6 +2393,34 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev >> *dev) >> PMD_INIT_LOG(DEBUG, "%d mbufs freed", mbuf_num); >> } >> >> +static void >> +virtio_tx_completed_cleanup(struct rte_eth_dev *dev) >> +{ >> + struct virtio_hw *hw = dev->data->dev_private; >> + struct virtqueue *vq; >> + int qidx; >> + void (*xmit_cleanup)(struct virtqueue *vq, uint16_t nb_used); >> + >> + if (virtio_with_packed_queue(hw)) { >> + if (hw->use_vec_tx) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder_packed; >> + else if (virtio_with_feature(hw, VIRTIO_F_IN_ORDER)) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder_packed; >> + else >> + xmit_cleanup = &virtio_xmit_cleanup_normal_packed; >> + } else { >> + if (hw->use_inorder_tx) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder; >> + else >> + xmit_cleanup = &virtio_xmit_cleanup; >> + } >> + >> + for (qidx = 0; qidx < hw->max_queue_pairs; qidx++) { >> + vq = hw->vqs[2 * qidx + VTNET_SQ_TQ_QUEUE_IDX]; > > Maybe add a check to ensure that vq is non-NULL since it is dereferenced > later without checking.
Good idea, I'll send v2. > >> + xmit_cleanup(vq, virtqueue_nused(vq)); >> + } >> +} >> + >> /* >> * Stop device: disable interrupt and mark link down >> */ >> @@ -2411,6 +2439,8 @@ virtio_dev_stop(struct rte_eth_dev *dev) >> goto out_unlock; >> hw->started = 0; >> >> + virtio_tx_completed_cleanup(dev); >> + >> if (intr_conf->lsc || intr_conf->rxq) { >> virtio_intr_disable(dev); >> >>