Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: add missing cpu_to_le32 in idpf_tx_splitq_build_flow_desc

Marcin Szycik <[email protected]>
Newsgroups org.kernel.vger.netdev,org.osuosl.intel-wired-lan
Message-ID <[email protected]>

On 03/08/2026 23:06, Willem de Bruijn wrote:
> From: Willem de Bruijn <[email protected]>
> 
> idpf_tx_splitq_build_flow_desc performs a 32-bit store to &cmd_dtype
> to set the 8-bit cmd_dtype and zero the adjacent 3-byte timestamp
> field in a single operation.
> 
> Descriptors are in little endian. Add missing cpu_to_le32 and cast to
> __le32 to ensure the fields are written correctly also on big endian
> platforms.
> 
> Fixes: 1a49cf814fe1 ("idpf: add Tx timestamp flows")
> Signed-off-by: Willem de Bruijn <[email protected]>
> Reviewed-by: Tony Nguyen <[email protected]>
> 
> ---
> 
> Changes
>   v1 -> v2
>   - add Fixes tag, Tony's Reviewed-by and Cc: [email protected]
>   v1: https://lore.kernel.org/netdev/[email protected]/
> ---
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index c724d429a7aa..91ca75e45463 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -2408,7 +2408,7 @@ void idpf_tx_splitq_build_flow_desc(union idpf_tx_flex_desc *desc,
>  				    struct idpf_tx_splitq_params *params,
>  				    u16 td_cmd, u16 size)
>  {
> -	*(u32 *)&desc->flow.qw1.cmd_dtype = (u8)(params->dtype | td_cmd);
> +	*(__le32 *)&desc->flow.qw1.cmd_dtype = cpu_to_le32((u8)(params->dtype | td_cmd));

While it technically works, I find it unreadable. Looking at this line
without reading commit msg, it looks like a bug where a 4-byte value is
written to u8 field. The intent to zero an adjacent, unrelated field is
not clear. Consider assigning these 4 fields manually, or adding a
comment/function doc.

Thanks,
Marcin

>  	desc->flow.qw1.rxr_bufsize = cpu_to_le16((u16)size);
>  	desc->flow.qw1.compl_tag = cpu_to_le16(params->compl_tag);
>  }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.