[PATCH] net/ice: fix incorrect mbuf cleanup function

[PATCH] net/ice: fix incorrect mbuf cleanup function

[Bruce Richardson]

The mbuf cleanup function is not being called correctly on close of the
driver, due to the vector_tx flag not being set correctly on each Tx
queue. Normally, this causes no visible problems, because the default is
to have the vector flag unset, which means that, if vector Tx path is
chosen, we only free every second mbuf in the descriptor vector ring - a
subtle memory leak.

The reverse case - where we initially configure a port to use a vector
driver, and then later reconfigure it for scalar - causes a segfault on
close, as the vector cleanup is used on a scalar SW ring, and that
vector path treats the extra metadata in the SW ring as mbufs to be
freed.

This is due to an inconsistency across drivers on port start: in e.g.
the i40e driver, the Rx and Tx functions are chosen before the
individual queues are started, while in ice driver the order is
reversed.  Because the Tx function selection is done after the queues
are initialized, the initialized queues use an old value of the overall
flag indicating if a vector Tx path is being used.

Fix the issue by moving the Rx and Tx path selection before queue start.

Fixes: 552979dfb1c9 ("net/intel: create common Tx queue mbuf cleanup")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/intel/ice/ice_ethdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 0e092702cc..3cdfa16f77 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -3961,6 +3961,9 @@ ice_dev_start(struct rte_eth_dev *dev)
 	ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
 	ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
 
+	ice_set_rx_function(dev);
+	ice_set_tx_function(dev);
+
 	/* program Tx queues' context in hardware */
 	for (nb_txq = 0; nb_txq < data->nb_tx_queues; nb_txq++) {
 		ret = ice_tx_queue_start(dev, nb_txq);
@@ -3989,9 +3992,6 @@ ice_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	ice_set_rx_function(dev);
-	ice_set_tx_function(dev);
-
 	mask = RTE_ETH_VLAN_STRIP_MASK | RTE_ETH_VLAN_FILTER_MASK |
 			RTE_ETH_VLAN_EXTEND_MASK;
 	if (ice_is_dvm_ena(hw))
-- 
2.43.0

Re: [PATCH] net/ice: fix incorrect mbuf cleanup function

[Burakov, Anatoly]

On 21/02/2025 17:04, Bruce Richardson wrote:
> The mbuf cleanup function is not being called correctly on close of the
> driver, due to the vector_tx flag not being set correctly on each Tx
> queue. Normally, this causes no visible problems, because the default is
> to have the vector flag unset, which means that, if vector Tx path is
> chosen, we only free every second mbuf in the descriptor vector ring - a
> subtle memory leak.
> 
> The reverse case - where we initially configure a port to use a vector
> driver, and then later reconfigure it for scalar - causes a segfault on
> close, as the vector cleanup is used on a scalar SW ring, and that
> vector path treats the extra metadata in the SW ring as mbufs to be
> freed.
> 
> This is due to an inconsistency across drivers on port start: in e.g.
> the i40e driver, the Rx and Tx functions are chosen before the
> individual queues are started, while in ice driver the order is
> reversed.  Because the Tx function selection is done after the queues
> are initialized, the initialized queues use an old value of the overall
> flag indicating if a vector Tx path is being used.
> 
> Fix the issue by moving the Rx and Tx path selection before queue start.
> 
> Fixes: 552979dfb1c9 ("net/intel: create common Tx queue mbuf cleanup")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly