linux-kernel - Re: [PATCH V2,net-next, 1/2] net: mana: Add support for coalesced RX packets on CQE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260109175610.0eb69acb@kernel.org>
Date: Fri, 9 Jan 2026 17:56:10 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Haiyang Zhang <haiyangz@...ux.microsoft.com>
Cc: linux-hyperv@...r.kernel.org, netdev@...r.kernel.org, "K. Y. Srinivasan"
 <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu
 <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, Long Li
 <longli@...rosoft.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo
 Abeni <pabeni@...hat.com>, Konstantin Taranov <kotaranov@...rosoft.com>,
 Simon Horman <horms@...nel.org>, Erni Sri Satya Vennela
 <ernis@...ux.microsoft.com>, Shradha Gupta
 <shradhagupta@...ux.microsoft.com>, Saurabh Sengar
 <ssengar@...ux.microsoft.com>, Aditya Garg
 <gargaditya@...ux.microsoft.com>, Dipayaan Roy
 <dipayanroy@...ux.microsoft.com>, Shiraz Saleem
 <shirazsaleem@...rosoft.com>, linux-kernel@...r.kernel.org,
 linux-rdma@...r.kernel.org, paulros@...rosoft.com
Subject: Re: [PATCH V2,net-next, 1/2] net: mana: Add support for coalesced
 RX packets on CQE

On Tue,  6 Jan 2026 12:46:46 -0800 Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@...rosoft.com>
> 
> Our NIC can have up to 4 RX packets on 1 CQE. To support this feature,
> check and process the type CQE_RX_COALESCED_4. The default setting is
> disabled, to avoid possible regression on latency.
> 
> And add ethtool handler to switch this feature. To turn it on, run:
>   ethtool -C <nic> rx-frames 4
> To turn it off:
>   ethtool -C <nic> rx-frames 1

Exposing just rx frame count, and only two values is quite unusual.
Please explain in more detail the coalescing logic of the device.

> @@ -2079,14 +2081,10 @@ static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
>  		return;
>  	}
>  
> -	pktlen = oob->ppi[0].pkt_len;
> -
> -	if (pktlen == 0) {
> -		/* data packets should never have packetlength of zero */
> -		netdev_err(ndev, "RX pkt len=0, rq=%u, cq=%u, rxobj=0x%llx\n",
> -			   rxq->gdma_id, cq->gdma_id, rxq->rxobj);
> +nextpkt:
> +	pktlen = oob->ppi[i].pkt_len;
> +	if (pktlen == 0)
>  		return;
> -	}
>  
>  	curr = rxq->buf_index;
>  	rxbuf_oob = &rxq->rx_oobs[curr];
> @@ -2097,12 +2095,15 @@ static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
>  	/* Unsuccessful refill will have old_buf == NULL.
>  	 * In this case, mana_rx_skb() will drop the packet.
>  	 */
> -	mana_rx_skb(old_buf, old_fp, oob, rxq);
> +	mana_rx_skb(old_buf, old_fp, oob, rxq, i);
>  
>  drop:
>  	mana_move_wq_tail(rxq->gdma_rq, rxbuf_oob->wqe_inf.wqe_size_in_bu);
>  
>  	mana_post_pkt_rxq(rxq);
> +
> +	if (coalesced && (++i < MANA_RXCOMP_OOB_NUM_PPI))
> +		goto nextpkt;

Please code this up as a loop. Using gotos for control flow other than
to jump to error handling epilogues is a poor coding practice (see the
kernel coding style).

> +static int mana_set_coalesce(struct net_device *ndev,
> +			     struct ethtool_coalesce *ec,
> +			     struct kernel_ethtool_coalesce *kernel_coal,
> +			     struct netlink_ext_ack *extack)
> +{
> +	struct mana_port_context *apc = netdev_priv(ndev);
> +	u8 saved_cqe_coalescing_enable;
> +	int err;
> +
> +	if (ec->rx_max_coalesced_frames != 1 &&
> +	    ec->rx_max_coalesced_frames != MANA_RXCOMP_OOB_NUM_PPI) {
> +		NL_SET_ERR_MSG_FMT(extack,
> +				   "rx-frames must be 1 or %u, got %u",
> +				   MANA_RXCOMP_OOB_NUM_PPI,
> +				   ec->rx_max_coalesced_frames);
> +		return -EINVAL;
> +	}
> +
> +	saved_cqe_coalescing_enable = apc->cqe_coalescing_enable;
> +	apc->cqe_coalescing_enable =
> +		ec->rx_max_coalesced_frames == MANA_RXCOMP_OOB_NUM_PPI;
> +
> +	if (!apc->port_is_up)
> +		return 0;
> +
> +	err = mana_config_rss(apc, TRI_STATE_TRUE, false, false);
> +

unnecessary empty line

> +	if (err) {
> +		netdev_err(ndev, "Set rx-frames to %u failed:%d\n",
> +			   ec->rx_max_coalesced_frames, err);
> +		NL_SET_ERR_MSG_FMT(extack, "Set rx-frames to %u failed",
> +				   ec->rx_max_coalesced_frames);

These messages are both pointless. If HW communication has failed
presumably there will already be an error in the logs. The extack
gives the user no information they wouldn't already have.

> +		apc->cqe_coalescing_enable = saved_cqe_coalescing_enable;
> +	}
> +
> +	return err;
> +}