On 2024-12-11 04:32, Yunsheng Lin wrote:

On 2024/12/11 2:14, David Wei wrote:

quoted

On 2024-12-10 04:25, Yunsheng Lin wrote:

quoted

On 2024/12/4 12:10, David Wei wrote:

quoted

 	bnxt_copy_rx_ring(bp, rxr, clone);

@@ -15563,6 +15580,8 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
 	bnxt_hwrm_rx_agg_ring_free(bp, rxr, false);
 	rxr->rx_next_cons = 0;
 	page_pool_disable_direct_recycling(rxr->page_pool);
+	if (bnxt_separate_head_pool())
+		page_pool_disable_direct_recycling(rxr->head_pool);

Hi, David
As mentioned in [1], is the above page_pool_disable_direct_recycling()
really needed?

Is there any NAPI API called in the implementation of netdev_queue_mgmt_ops?
It doesn't seem obvious there is any NAPI API like napi_enable() &
____napi_schedule() that is called in bnxt_queue_start()/bnxt_queue_stop()/
bnxt_queue_mem_alloc()/bnxt_queue_mem_free() through code reading.

1. https://lore.kernel.org/all/c2b306af-4817-4169-814b-adbf25803919@huawei.com/ (local)

Hi Yunsheng, there are explicitly no napi_enable/disable() calls in the
bnxt implementation of netdev_queue_mgmt_ops due to ... let's say HW/FW
quirks. I looked back at my discussions w/ Broadcom, and IIU/RC
bnxt_hwrm_vnic_update() will prevent any work from coming into the rxq
that I'm trying to stop. Calling napi_disable() has unintended side
effects on the Tx side.

It seems that bnxt_hwrm_vnic_update() sends a VNIC_UPDATE cmd to disable
a VNIC? and a napi_disable() is not needed?

Correct.

Is it possible that there may
be some pending NAPI work is still being processed after bnxt_hwrm_vnic_update()
is called?

Possibly, I don't know the details of how the HW works.

At the time I just wanted something to work, and not having
napi_enable/disable() made it work. :) Looking back though it does seem
odd, so I'll try putting it back.

quoted

The intent of the call to page_pool_disable_direct_recycling() is to
prevent pages from the old page pool from being returned into the fast
cache. These pages must be returned via page_pool_return_page() so that
the it can eventually be freed in page_pool_release_retry().

I'm going to take a look at your discussions in [1] and respond there.

Thanks.