Re: [Intel-wired-lan] [PATCH v2] i40e: add an error handling path in i40e_xsk_pool_enable()

[Linkui Xiao <xiaolinkui@126.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]

Hi Haoxiang,

The kernel test robot reported the following build error on your v2:

  drivers/net/ethernet/intel/i40e/i40e_xsk.c:130:9: error: implicit declaration of function 'i40e_xsk_pool_disable'; did you mean 'i40e_xsk_pool_enable'?
  drivers/net/ethernet/intel/i40e/i40e_xsk.c:141:12: error: conflicting types for 'i40e_xsk_pool_disable'

The root cause is that i40e_xsk_pool_disable() is defined after i40e_xsk_pool_enable() in i40e_xsk.c [citation:4]. When the error path in i40e_xsk_pool_enable() calls i40e_xsk_pool_disable(), the compiler has not yet seen its prototype.

One way to fix this without reordering functions is to unwind the two operations that i40e_xsk_pool_enable() itself performed in the error path:

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..97d016be493d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -108,23 +108,31 @@ static int i40e_xsk_pool_enable(struct i40e_vsi *vsi,
        if (if_running) {
                err = i40e_queue_pair_disable(vsi, qid);
                if (err)
-                       return err;
+                       goto err_pool_unmap;

                err = i40e_realloc_rx_xdp_bi(vsi->rx_rings[qid], true);
                if (err)
-                       return err;
+                       goto err_pool_unmap;

                err = i40e_queue_pair_enable(vsi, qid);
                if (err)
-                       return err;
+                       goto err_realloc;

                /* Kick start the NAPI context so that receiving will start */
                err = i40e_xsk_wakeup(vsi->netdev, qid, XDP_WAKEUP_RX);
                if (err)
-                       return err;
+                       goto err_realloc;
        }

        return 0;
+
+err_realloc:
+       i40e_realloc_rx_xdp_bi(vsi->rx_rings[qid], false);
+err_pool_unmap:
+       clear_bit(qid, vsi->af_xdp_zc_qps);
+       xsk_pool_dma_unmap(pool, I40E_RX_DMA_ATTR);
+
+       return err;
 }

This mirrors the internal logic of i40e_xsk_pool_disable() [citation:12] — clear the zero-copy bit and DMA-unmap the pool — without calling a function defined later in the file. It also avoids the limbo-state concern you mentioned in your v2 changelog, since it does not go through the pool-association check in i40e_xsk_pool_disable().

Alternatively, a forward declaration of i40e_xsk_pool_disable() at the top of i40e_xsk.c would also resolve the build error.

Best regards,
Linkui Xiao