Re: [PATCH net-next] net: mana: Extend RX CQE coalescing up to 8 packets

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:
- [High] The `mana_rxcomp_perpkt_info` union defines the 8-packet mode layo=
ut in a way that incorrectly rearranges fields, contradicting the commit me=
ssage and potentially causing the driver to read bogus packet lengths and h=
ashes.
- [Medium] The driver accepts `rx-frames=3D8` via ethtool even when the har=
dware does not support it (`!gc->cqe8_coalescing_sup`), silently falling ba=
ck to 4 packets instead of returning an error.
--

--- Patch [1]: [PATCH net-next] net: mana: Extend RX CQE coalescing up to 8=
 packets ---
commit 2083fcc78b90120f37ea18fdedab5ea909bd1068
Author: Haiyang Zhang <[email protected]>

net: mana: Extend RX CQE coalescing up to 8 packets

To support up to 8 packets per CQE, put two packet lengths and hash values
into one PPI entry by using the reserved fields. Update ethtool handlers to
set this feature and per queue stat to show the coalesced CQE counters.
This feature is supported on NIC hardware showing the relevant PF flag.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers=
/net/ethernet/microsoft/mana/mana_ethtool.c
> index 9e31e2595ae32..8743f7edecbb8 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c

[ ... ]

> @@ -514,8 +518,15 @@ static int mana_set_coalesce(struct net_device *ndev,
>  	saved.tx_dim_enabled =3D apc->tx_dim_enabled;
> =20
>  	saved.cqe_coalescing_enable =3D apc->cqe_coalescing_enable;
> +	saved.cqe8_coalescing_enable =3D apc->cqe8_coalescing_enable;
>  	apc->cqe_coalescing_enable =3D
> -		kernel_coal->rx_cqe_frames =3D=3D MANA_RXCOMP_OOB_NUM_PPI;
> +		kernel_coal->rx_cqe_frames >=3D MANA_RXCOMP_OOB_NUM_PPI;
> +
> +	if (gc->cqe8_coalescing_sup)
> +		apc->cqe8_coalescing_enable =3D
> +			kernel_coal->rx_cqe_frames =3D=3D MANA_CQE_COAL_PKTS_8;
> +	else if (kernel_coal->rx_cqe_frames =3D=3D MANA_CQE_COAL_PKTS_8)
> +		netdev_info(ndev, "Fall back to RX-CQE x4 coalescing\n");

[Severity: Medium]
When the hardware does not support 8-packet coalescing, this code logs an
info message and falls back to 4 packets while still returning success (0).

Should this instead return an error code like -EINVAL or -EOPNOTSUPP?
Silently changing the requested parameter might break userspace configurati=
on
scripts that expect ethtool to reject unsupported values.

> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 4d041fb8437f5..3ca1399a2fafa 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h

[ ... ]

> @@ -238,12 +242,25 @@ struct mana_cqe_header {
>  #define MANA_HASH_L4                                                    =
     \
>  	(NDIS_HASH_TCP_IPV4 | NDIS_HASH_UDP_IPV4 | NDIS_HASH_TCP_IPV6 |      \
>  	 NDIS_HASH_UDP_IPV6 | NDIS_HASH_TCP_IPV6_EX | NDIS_HASH_UDP_IPV6_EX)
> +#define MANA_HASH_ENABLE_SUPPORTED \
> +	(NDIS_HASH_IPV4 | NDIS_HASH_TCP_IPV4 | NDIS_HASH_UDP_IPV4 | \
> +	 NDIS_HASH_IPV6 | NDIS_HASH_TCP_IPV6 | NDIS_HASH_UDP_IPV6)
> +
> +union mana_rxcomp_perpkt_info {
> +	struct {
> +		u32 pkt_len : 16;
> +		u32 reserved1 : 16;
> +		u32 reserved2;
> +		u32 pkt_hash;
> +	};
> =20
> -struct mana_rxcomp_perpkt_info {
> -	u32 pkt_len	: 16;
> -	u32 reserved1	: 16;
> -	u32 reserved2;
> -	u32 pkt_hash;
> +	/* Up to two pkts per PPI entry */
> +	struct {
> +		u32 pkt_hash0;
> +		u16 pkt_len0;
> +		u16 pkt_len1;
> +		u32 pkt_hash1;
> +	};
>  }; /* HW DATA */

[Severity: High]
The commit message mentions using the reserved fields for the second packet,
but does this new structure layout match the original offsets?

In the original structure, pkt_len is at offset 0. However, the new 8-packet
struct places pkt_hash0 at offset 0.

On little-endian systems, this will overwrite the original pkt_len position
and move pkt_len0 and pkt_len1 to offsets 4 and 6. If the hardware still
populates the first packet's length at offset 0, could the driver misinterp=
ret
the first 4 bytes as a hash and parse bogus packet lengths, potentially
causing 8-packet coalesced packets to be dropped as oversized?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260729225235.3573=
[email protected]?part=3D1