From: Simon Horman <horms@kernel.org>
To: michal.swiatkowski@linux.intel.com
Cc: 'Simon Horman' <horms@kernel.org>,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
jramaseu@redhat.com, anthony.l.nguyen@intel.com,
przemyslaw.kitszel@intel.com, aleksandr.loktionov@intel.com
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
Date: Thu, 30 Apr 2026 16:29:49 +0100 [thread overview]
Message-ID: <20260430152948.1683359-2-horms@kernel.org> (raw)
In-Reply-To: <20260428070647.777141-3-michal.swiatkowski@linux.intel.com>
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
---
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?
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?
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?
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: michal.swiatkowski@linux.intel.com
Cc: 'Simon Horman' <horms@kernel.org>,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
jramaseu@redhat.com, anthony.l.nguyen@intel.com,
przemyslaw.kitszel@intel.com, aleksandr.loktionov@intel.com
Subject: Re: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
Date: Thu, 30 Apr 2026 16:29:49 +0100 [thread overview]
Message-ID: <20260430152948.1683359-2-horms@kernel.org> (raw)
In-Reply-To: <20260428070647.777141-3-michal.swiatkowski@linux.intel.com>
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
---
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?
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?
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?
next prev parent reply other threads:[~2026-04-30 15:44 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-28 7:06 [Intel-wired-lan] [PATCH iwl-net v1 0/2] Rework ice Tx checksum Michal Swiatkowski
2026-04-28 7:06 ` Michal Swiatkowski
2026-04-28 7:06 ` [Intel-wired-lan] [PATCH iwl-net v1 1/2] ice: always do GCS if hardware supports it Michal Swiatkowski
2026-04-28 7:06 ` Michal Swiatkowski
2026-05-26 22:08 ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-26 22:08 ` Nowlin, Alexander
2026-04-28 7:06 ` [Intel-wired-lan] [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
2026-04-28 7:06 ` Michal Swiatkowski
2026-04-28 8:34 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-04-28 8:34 ` Loktionov, Aleksandr
2026-04-30 15:29 ` Simon Horman [this message]
2026-04-30 15:29 ` Simon Horman
2026-04-30 16:21 ` [Intel-wired-lan] " Simon Horman
2026-04-30 16:21 ` Simon Horman
2026-05-04 8:09 ` [Intel-wired-lan] " Michal Swiatkowski
2026-05-04 8:09 ` Michal Swiatkowski
2026-05-04 23:53 ` [Intel-wired-lan] " Jacob Keller
2026-05-05 4:35 ` Michal Swiatkowski
2026-05-05 5:16 ` Jacob Keller
2026-05-26 22:10 ` Nowlin, Alexander
2026-05-26 22:10 ` Nowlin, Alexander
2026-05-28 8:52 ` Loktionov, Aleksandr
2026-05-28 8:52 ` Loktionov, Aleksandr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260430152948.1683359-2-horms@kernel.org \
--to=horms@kernel.org \
--cc=aleksandr.loktionov@intel.com \
--cc=anthony.l.nguyen@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jramaseu@redhat.com \
--cc=michal.swiatkowski@linux.intel.com \
--cc=netdev@vger.kernel.org \
--cc=przemyslaw.kitszel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.