Re: [PATCH net-next v28 4/8] eea: create/destroy rx,tx queues for netdevice open and stop

[Paolo Abeni <pabeni@redhat.com>]

On 3/9/26 9:51 AM, Xuan Zhuo wrote:
> On Thu, 5 Mar 2026 11:16:32 +0100, Paolo Abeni <pabeni@redhat.com> wrote:
>> On 3/2/26 3:46 AM, Xuan Zhuo wrote:
>>> +static int eea_netdev_open(struct net_device *netdev)
>>> +{
>>> +	struct eea_net *enet = netdev_priv(netdev);
>>> +	struct eea_net_init_ctx ctx;
>>> +	int err;
>>> +
>>> +	if (enet->link_err) {
>>> +		netdev_err(netdev, "netdev open err, because link error: %d\n",
>>> +			   enet->link_err);
>>> +		return -EBUSY;
>>> +	}
>>> +
>>> +	eea_init_ctx(enet, &ctx);
>>> +
>>> +	err = eea_alloc_rxtx_q_mem(&ctx);
>>> +	if (err)
>>> +		goto err_done;
>>> +
>>> +	eea_bind_q_and_cfg(enet, &ctx);
>>> +
>>> +	err = eea_update_queues(enet);
>>> +	if (err)
>>> +		goto err_free_q;
>>> +
>>> +	err = eea_hw_active_ring(enet);
>>> +	if (err)
>>> +		goto err_free_q;
>>> +
>>> +	eea_start_rxtx(enet);
>>> +
>>> +	return 0;
>>> +
>>> +err_free_q:
>>> +	eea_unbind_q_and_cfg(enet, &ctx);
>>> +	eea_free_rxtx_q_mem(&ctx);
>>> +
>>> +err_done:
>>> +	return err;
>>> +}
>>> +
>>> +/* resources: ring, buffers, irq */
>>> +int eea_reset_hw_resources(struct eea_net *enet, struct eea_net_init_ctx *ctx)
>>> +{
>>> +	struct eea_net_init_ctx ctx_old = {0};
>>> +	int err, error;
>>> +
>>> +	if (!netif_running(enet->netdev) || !enet->started) {
>>> +		enet->cfg = ctx->cfg;
>>> +		return 0;
>>> +	}
>>> +
>>> +	err = eea_alloc_rxtx_q_mem(ctx);
>>> +	if (err) {
>>> +		netdev_warn(enet->netdev,
>>> +			    "eea reset: alloc q failed. stop reset. err %d\n",
>>> +			    err);
>>> +		return err;
>>> +	}
>>> +
>>> +	eea_stop_rxtx(enet->netdev);
>>> +	eea_hw_unactive_ring(enet);
>>> +
>>> +	eea_unbind_q_and_cfg(enet, &ctx_old);
>>> +	eea_bind_q_and_cfg(enet, ctx);
>>> +
>>> +	err = eea_update_queues(enet);
>>> +	if (err) {
>>> +		netdev_err(enet->netdev,
>>> +			   "eea reset: set real num queues failed. err %d\n",
>>> +			   err);
>>> +		goto err_bind_old;
>>> +	}
>>> +
>>> +	err = eea_hw_active_ring(enet);
>>
>> AFAICS the need for admins queue destroy and recreation on cfg change is
>> what prevents a safe configuration swap (together with irq allocation,
>> see below). Could you instead keep the admin queue always alive i.e.
>> with maximum number of erings configured, and just enable/them as needded?
>>
>> If not, please document explicitly the constraint in the commit message.
> 
> Generally, during I/O queue reconstruction, the admin queue is not rebuilt.
> However, in HA scenarios—which represent a special case—
> the DPU notifies the driver to request re-initialization. In this specific
> situation, the admin queue will also be rebuilt.

The above means that in HA scenarios the driver can't ensure to restore
the status correctly right?

At very least this info should be explicitly mentioned in the commit
message.

> 
>>
>> [...]
>>> @@ -140,14 +591,65 @@ static struct eea_net *eea_netdev_alloc(struct eea_device *edev, u32 pairs)
>>>  	enet->edev = edev;
>>>  	edev->enet = enet;
>>>
>>> +	err = eea_alloc_irq_blks(enet, pairs);
>>> +	if (err) {
>>
>> I'm likely lost, but AFAICS eea_alloc_irq_blks() is invoked only here,
>> and allocates edev->rx_num interrupts. rx_num is the number of RX
>> channel at device probe time.
>>
>> set_channels can later change such value, i.e. increasing it. But I
>> don't see other irqs allocated.
> 
> 
> No, set_channels just change enet.cfg.rx_num, edev->rx_num is readonly.

Ah, I (now) see that set_channel max is cfg_hw.rx_ring_num, which in
turn is equal to edev->rx_num.

It would be useful to explicitly mention in the commit message or in the
init code that the driver always per allocate the max possible number of
IRQs handlers.

/P