RE: [EXTERNAL] Re: [PATCH net-next v2] net: mana: Add Interrupt Moderation support

[Haiyang Zhang]

> -----Original Message-----
> From: Paolo Abeni <pabeni@xxxxxxxxxx>
> Sent: Tuesday, June 9, 2026 9:49 AM
> To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxxxxxxxx>; linux-
> hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; KY Srinivasan
> <kys@xxxxxxxxxxxxx>; Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; Wei Liu
> <wei.liu@xxxxxxxxxx>; Dexuan Cui <DECUI@xxxxxxxxxxxxx>; Long Li
> <longli@xxxxxxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>; David S.
> Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>;
> Simon Horman <horms@xxxxxxxxxx>; Shradha Gupta
> <shradhagupta@xxxxxxxxxxxxxxxxxxx>; Erni Sri Satya Vennela
> <ernis@xxxxxxxxxxxxxxxxxxx>; Dipayaan Roy
> <dipayanroy@xxxxxxxxxxxxxxxxxxx>; Aditya Garg
> <gargaditya@xxxxxxxxxxxxxxxxxxx>; Kees Cook <kees@xxxxxxxxxx>; Breno
> Leitao <leitao@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx
> Cc: Paul Rosswurm <paulros@xxxxxxxxxxxxx>
> Subject: [EXTERNAL] Re: [PATCH net-next v2] net: mana: Add Interrupt
> Moderation support
> 
> On 6/5/26 1:41 AM, Haiyang Zhang wrote:
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index db14357d3732..b1e0c444f414 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -1551,6 +1551,9 @@ int mana_create_wq_obj(struct mana_port_context
> *apc,
> >
> >  	mana_gd_init_req_hdr(&req.hdr, MANA_CREATE_WQ_OBJ,
> >  			     sizeof(req), sizeof(resp));
> > +
> > +	req.hdr.req.msg_version = GDMA_MESSAGE_V3;
> > +	req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
> 
> Sashiko noted the above cold break initialization on older firmware:

Our firmware is forward compatible with newer message versions, so the 
old firmware still properly handles this message, just the new feature 
fields are ignored, and queue creation will be successful.
And if the DIM capability bit is zero from FW, driver will keep the DIM
feature to be off and unchangeable.


> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsashiko.
> dev%2F%23%2Fpatchset%2F20260604234211.2056341-1-
> haiyangz%2540linux.microsoft.com&data=05%7C02%7Chaiyangz%40microsoft.com%7
> C9cc8d2c7aa7f472ff8e308dec62def04%7C72f988bf86f141af91ab2d7cd011db47%7C1%7
> C0%7C639166097783522606%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIl
> YiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> 7C%7C&sdata=TB7q6EhtR6HJ02Q4f767sXUCmYZyGr3wH1Sz3vLPWfA%3D&reserved=0
> 
> [...]
> > +static void mana_update_rx_dim(struct mana_cq *cq)
> > +{
> > +	struct mana_port_context *apc = netdev_priv(cq->rxq->ndev);
> > +	struct mana_rxq *rxq = cq->rxq;
> > +	struct dim_sample dim_sample = {};
> 
> Minor nit: please fix the variable declaration order above. Other
> occurrences below.

Done in the new version.

> 
> [...]
> > @@ -440,17 +474,94 @@ static int mana_set_coalesce(struct net_device
> *ndev,
> >  		return -EINVAL;
> >  	}
> >
> > -	saved_cqe_coalescing_enable = apc->cqe_coalescing_enable;
> > +	if (ec->rx_coalesce_usecs > MANA_INTR_MODR_USEC_MAX ||
> > +	    ec->tx_coalesce_usecs > MANA_INTR_MODR_USEC_MAX) {
> > +		NL_SET_ERR_MSG_FMT(extack,
> > +				   "coalesce usecs must be <= %lu",
> > +				   MANA_INTR_MODR_USEC_MAX);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ec->rx_max_coalesced_frames > MANA_INTR_MODR_COMP_MAX ||
> > +	    ec->tx_max_coalesced_frames > MANA_INTR_MODR_COMP_MAX) {
> > +		NL_SET_ERR_MSG_FMT(extack,
> > +				   "coalesce frames must be <= %lu",
> > +				   MANA_INTR_MODR_COMP_MAX);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ec->rx_coalesce_usecs != apc->intr_modr_rx_usec ||
> > +	    ec->rx_max_coalesced_frames != apc->intr_modr_rx_comp ||
> > +	    ec->tx_coalesce_usecs != apc->intr_modr_tx_usec ||
> > +	    ec->tx_max_coalesced_frames != apc->intr_modr_tx_comp)
> > +		modr_changed = true;
> > +
> > +	saved.intr_modr_rx_usec = apc->intr_modr_rx_usec;
> > +	saved.intr_modr_rx_comp = apc->intr_modr_rx_comp;
> > +	saved.intr_modr_tx_usec = apc->intr_modr_tx_usec;
> > +	saved.intr_modr_tx_comp = apc->intr_modr_tx_comp;
> > +
> > +	apc->intr_modr_rx_usec = ec->rx_coalesce_usecs;
> > +	apc->intr_modr_rx_comp = ec->rx_max_coalesced_frames;
> > +	apc->intr_modr_tx_usec = ec->tx_coalesce_usecs;
> > +	apc->intr_modr_tx_comp = ec->tx_max_coalesced_frames;
> > +
> > +	if (!!ec->use_adaptive_rx_coalesce != apc->rx_dim_enabled ||
> > +	    !!ec->use_adaptive_tx_coalesce != apc->tx_dim_enabled)
> > +		dim_changed = true;
> > +
> > +	saved.rx_dim_enabled = apc->rx_dim_enabled;
> > +	saved.tx_dim_enabled = apc->tx_dim_enabled;
> > +	apc->rx_dim_enabled = !!ec->use_adaptive_rx_coalesce;
> > +	apc->tx_dim_enabled = !!ec->use_adaptive_tx_coalesce;
> > +
> > +	saved.cqe_coalescing_enable = apc->cqe_coalescing_enable;
> >  	apc->cqe_coalescing_enable =
> >  		kernel_coal->rx_cqe_frames == MANA_RXCOMP_OOB_NUM_PPI;
> >
> >  	if (!apc->port_is_up)
> >  		return 0;
> >
> > -	err = mana_config_rss(apc, TRI_STATE_TRUE, false, false);
> > -	if (err)
> > -		apc->cqe_coalescing_enable = saved_cqe_coalescing_enable;
> > +	if (apc->cqe_coalescing_enable != saved.cqe_coalescing_enable &&
> > +	    !modr_changed && !dim_changed) {
> > +		/* If only CQE coalescing setting is changed, we can just
> update
> > +		 * RSS configuration.
> > +		 */
> > +		err = mana_config_rss(apc, TRI_STATE_TRUE, false, false);
> > +		if (err) {
> > +			netdev_err(ndev, "Change CQE coalescing failed: %d\n",
> > +				   err);
> > +			apc->cqe_coalescing_enable =
> > +				saved.cqe_coalescing_enable;
> > +			return err;
> > +		}
> > +		return 0;
> > +	}
> > +
> > +	if (modr_changed || dim_changed) {
> > +		err = mana_detach(ndev, false);
> > +		if (err) {
> > +			netdev_err(ndev, "mana_detach failed: %d\n", err);
> > +			goto restore_modr;
> > +		}
> > +
> > +		err = mana_attach(ndev);
> > +		if (err) {
> > +			netdev_err(ndev, "mana_attach failed: %d\n", err);
> > +			goto restore_modr;
> > +		}
> 
> You should try hard to avoid this sequence: if mana_attach fails,
> mana_set_coalesce() will leave the NIC unexpectedly down.

I have updated the patch to use doorbell for this setting change without
re-attach, and will submit the new version soon.

Thanks,
- Haiyang