> -----Original Message-----
> From: Paolo Abeni <[email protected]>
> Sent: Tuesday, June 9, 2026 9:49 AM
> To: Haiyang Zhang <[email protected]>; linux-
> [email protected]; [email protected]; KY Srinivasan
> <[email protected]>; Haiyang Zhang <[email protected]>; Wei Liu
> <[email protected]>; Dexuan Cui <[email protected]>; Long Li
> <[email protected]>; Andrew Lunn <[email protected]>; David S.
> Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Konstantin Taranov <[email protected]>;
> Simon Horman <[email protected]>; Shradha Gupta
> <[email protected]>; Erni Sri Satya Vennela
> <[email protected]>; Dipayaan Roy
> <[email protected]>; Aditya Garg
> <[email protected]>; Kees Cook <[email protected]>; Breno
> Leitao <[email protected]>; [email protected]; linux-
> [email protected]
> Cc: Paul Rosswurm <[email protected]>
> 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
