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/[email protected]?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 */
