Re: [PATCH iwl-next 1/8] e1000e: add jumbo Rx CRC stripping

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260830232146.36948-2-tactii@gmail.com?part=1
---
e1000e: add jumbo Rx CRC stripping
    
When the hardware is configured to not strip the FCS (CrcStripping=0),
the jumbo Rx path would deliver frames with the FCS still attached.
Follow the standard path's convention by stripping the FCS at EOP and
leaving it out of the byte count.

> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 844f31ab37ad4..599600ad695cd 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -1631,13 +1631,23 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_ring *rx_ring, int *work_done,
>  			}
>  		}
>  
> +		/* strip the Ethernet CRC; it may span fragments */
> +		if (!(adapter->flags2 & FLAG2_CRC_STRIPPING) &&
> +		    !(netdev->features & NETIF_F_RXFCS))
> +			pskb_trim(skb, skb->len - 4);
> +
>  		/* Receive Checksum Offload */
>  		e1000_rx_checksum(adapter, staterr, skb);
>  
>  		e1000_rx_hash(netdev, rx_desc->wb.lower.hi_dword.rss, skb);
>  
> -		/* probably a little skewed due to removing CRC */
>  		total_rx_bytes += skb->len;
> +		/* If configured to store CRC, keep the FCS bytes out of the
> +		 * total_rx_bytes counter
> +		 */
> +		if (!(adapter->flags2 & FLAG2_CRC_STRIPPING) &&
> +		    (netdev->features & NETIF_F_RXFCS))
> +			total_rx_bytes -= 4;

[Severity: Medium]
Could these lockless evaluations of adapter->flags2 and netdev->features get
out of sync if userspace changes features via ethtool while a packet is being
processed?

If a concurrent change occurs between the first check and this second check,
it appears it could lead to an inconsistent state where a packet's payload is
incorrectly truncated by 4 bytes, or where the byte statistics are overcounted
or undercounted by 4 bytes.

>  		total_rx_packets++;
>  
>  		/* eth type trans needs skb->data to point to something */