Re: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6

[Simon Horman <horms@kernel.org>]

On Thu, Apr 30, 2026 at 04:29:49PM +0100, Simon Horman wrote:
> From: '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

Sorry, the email I am responding to was supposed to come with
some commentary - it seems my method for adding it is broken :(
So I'm adding it below (as best as I can remember what it was).

> ---
> ice: use NETIF_F_HW_CSUM instead of IP/IPV6
> 
> This commit replaces the IP and IPV6 checksum offload flags with HW_CSUM
> in the ice driver to allow hardware checksumming for IPv6 packets with
> extension headers. It also removes the mutual exclusivity check between
> TSO and GCS.
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> > index 5f92377d4dfc..33f252a6a22f 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > @@ -3491,9 +3491,8 @@ void ice_set_netdev_features(struct net_device *netdev)
> >  			NETIF_F_RXHASH;
> >  
> >  	csumo_features = NETIF_F_RXCSUM	  |
> > -			 NETIF_F_IP_CSUM  |
> >  			 NETIF_F_SCTP_CRC |
> > -			 NETIF_F_IPV6_CSUM;
> > +			 NETIF_F_HW_CSUM;
> 
> By advertising NETIF_F_HW_CSUM, the network stack assumes the driver can handle
> checksums for any protocol and passes non-IP packets (e.g., from AF_PACKET
> injected frames) to the driver with CHECKSUM_PARTIAL.
> 
> Since ice_tx_csum() strictly parses the L3 header and expects IPv4 or IPv6,
> returning -1 for other protocols, will this regression cause non-IP packets to
> be silently dropped instead of falling back to software checksums in the kernel?

The problem above seems to be a legitimate concern, if a corner case.
It seems to also exist in i40e.

> Additionally, skb_csum_hwoffload_help() relies on NETIF_F_IPV6_CSUM to
> safely fall back to software checksums when IPv6 extension headers are
> present.
> 
> Does using NETIF_F_HW_CSUM bypass this safeguard, potentially causing
> legacy hardware (like E810) or E830 without GCS descriptors to compute
> incorrect pseudo-headers and silently corrupt IPv6 packets with extension
> headers?

The issue above seems to be a false positive as the ice driver
doesn't rely on the behaviour described. Rather ice_tx_csum()
uses ipv6_skip_exthdr() to correctly handle extension headers.

> 
> Finally, for protocols other than TCP, UDP, or SCTP, the driver falls back to
> calling skb_checksum_help(skb) in the default case of ice_tx_csum().
> 
> However, the driver appears to ignore the return value of skb_checksum_help().
> 
> If skb_checksum_help() fails (e.g., due to -ENOMEM during pskb_expand_head()),
> could this result in the packet being transmitted with an uncomputed checksum?

The problem above seems real to me, but it also seems to be pre-existing.
So I don't think it should delay progress of this patch.

If it is a problem, it also seems to be present in i40e.