* [PATCH iwl-net v1 0/2] Rework ice Tx checksum
@ 2026-04-28 7:06 Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 1/2] ice: always do GCS if hardware supports it Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
0 siblings, 2 replies; 10+ messages in thread
From: Michal Swiatkowski @ 2026-04-28 7:06 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, jramaseu, anthony.l.nguyen, przemyslaw.kitszel,
aleksandr.loktionov, Michal Swiatkowski
Hi,
Jakub find the problem with how ice handles Tx checksum offload [1].
The issue is that IP_CSUM/IPV6_CSUM should support only basic packet
types. Kernel assumes that if there are extensions headers and there is
no HW_CSUM software fallback should be applied. Here is a patch that
introduced that [2].
However, software fallback can't be applied when TSO offload happens.
That is the situation that this patchset is fixing.
When TSO is enabled and IP_CSUM/IPV6_CSUM is on, kernel is assuming
hardware will be able to calculate the checksum, but later on the
extension headers are checked and if there is no HW_CSUM the
skb_bad_offload() is called.
Following the documentation ice driver is capable of HW_CSUM support, so
switch from IP_CSUM/IPV6_CSUM to HW_CSUM. Other Intel drivers (even
previous like i40e, ixgbe) also supports HW_CSUM.
The HW_CSUM was used to indicate that GCS (Intel name for checksumming
offload using additional descriptor) is support, which is variation of
HW_CSUM. Even without GCS th HW_CSUM is supported. First patch is
switching to use custom Tx ring flags to allow HW_CSUM usage.
[1] https://lore.kernel.org/netdev/20260310150557.1138437-1-jramaseu@redhat.com/
[2] https://lore.kernel.org/all/5fbeecfc311ea182aa1d1c771725ab8b4cac515e.1729778144.git.benoit.monin@gmx.fr/
Michal Swiatkowski (2):
ice: always do GCS if hardware supports it
ice: use NETIF_F_HW_CSUM instead of IP/IPV6
drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++++
drivers/net/ethernet/intel/ice/ice_main.c | 21 +--------------------
drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx.h | 1 +
4 files changed, 7 insertions(+), 21 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH iwl-net v1 1/2] ice: always do GCS if hardware supports it
2026-04-28 7:06 [PATCH iwl-net v1 0/2] Rework ice Tx checksum Michal Swiatkowski
@ 2026-04-28 7:06 ` Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
1 sibling, 0 replies; 10+ messages in thread
From: Michal Swiatkowski @ 2026-04-28 7:06 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, jramaseu, anthony.l.nguyen, przemyslaw.kitszel,
aleksandr.loktionov, Michal Swiatkowski
There is no need to check for NETIF_HW_CSUM. If the code reach
calculating checksum it means that correct checksum flags are set,
because kernel is checking that when setting ip->summed.
Instead of netdev feature flag use Tx ring flag to check if the hardware
can use special descriptor for checksum calculating.
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++++
drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx.h | 1 +
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 837b71b7b2b7..033fabc22f58 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1415,6 +1415,10 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
set_bit(ICE_TX_RING_FLAGS_VLAN_L2TAG2, ring->flags);
else
set_bit(ICE_TX_RING_FLAGS_VLAN_L2TAG1, ring->flags);
+
+ if (ice_is_feature_supported(pf, ICE_F_GCS))
+ set_bit(ICE_TX_RING_FLAGS_GCS, ring->flags);
+
WRITE_ONCE(vsi->tx_rings[i], ring);
}
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 4ca1a0602307..25b19a5d817e 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -1743,7 +1743,7 @@ int ice_tx_csum(struct ice_tx_buf *first, struct ice_tx_offload_params *off)
l3_len = l4.hdr - ip.hdr;
offset |= (l3_len / 4) << ICE_TX_DESC_LEN_IPLEN_S;
- if ((tx_ring->netdev->features & NETIF_F_HW_CSUM) &&
+ if (test_bit(ICE_TX_RING_FLAGS_GCS, tx_ring->flags) &&
!(first->tx_flags & ICE_TX_FLAGS_TSO) &&
!skb_csum_is_sctp(skb)) {
/* Set GCS */
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index 5e517f219379..15dbd5100912 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -217,6 +217,7 @@ enum ice_tx_ring_flags {
ICE_TX_RING_FLAGS_VLAN_L2TAG1,
ICE_TX_RING_FLAGS_VLAN_L2TAG2,
ICE_TX_RING_FLAGS_TXTIME,
+ ICE_TX_RING_FLAGS_GCS,
ICE_TX_RING_FLAGS_NBITS,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-28 7:06 [PATCH iwl-net v1 0/2] Rework ice Tx checksum Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 1/2] ice: always do GCS if hardware supports it Michal Swiatkowski
@ 2026-04-28 7:06 ` Michal Swiatkowski
2026-04-28 8:34 ` Loktionov, Aleksandr
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Michal Swiatkowski @ 2026-04-28 7:06 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, jramaseu, anthony.l.nguyen, przemyslaw.kitszel,
aleksandr.loktionov, Michal Swiatkowski
The hardware is capable of calculating checksum for IPV6 packets with
extension header. To not drop such packets switch from IP/IPV6 checksum
to HW_CSUM.
HW_CSUM is also used in previous generation (i40e).
Previously HW_CSUM was used to indicate that hardware supports general
checksum. Drop it assuming that if the hardware supports it, it is used.
Disabling offload for E830 in case of TSO isn't needed anymore as the
check for TSO is done in Tx path just before preparation of the special
GCS descriptor.
The commit from Fixes didn't introduce a bug, it just shown that the
driver is doing sth wrong with the checksum features.
Suggested-by: Jakub Ramaseuski <jramaseu@redhat.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Fixes: 04c20a9356f2 ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension")
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 15550216fbf0..0f2f949af536 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3500,9 +3500,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;
vlano_features = NETIF_F_HW_VLAN_CTAG_FILTER |
NETIF_F_HW_VLAN_CTAG_TX |
@@ -3564,12 +3563,6 @@ void ice_set_netdev_features(struct net_device *netdev)
/* Allow core to manage IRQs affinity */
netif_set_affinity_auto(netdev);
- /* Mutual exclusivity for TSO and GCS is enforced by the set features
- * ndo callback.
- */
- if (ice_is_feature_supported(pf, ICE_F_GCS))
- netdev->hw_features |= NETIF_F_HW_CSUM;
-
netif_set_tso_max_size(netdev, ICE_MAX_TSO_SIZE);
}
@@ -6489,18 +6482,6 @@ ice_set_features(struct net_device *netdev, netdev_features_t features)
if (changed & NETIF_F_LOOPBACK)
ret = ice_set_loopback(vsi, !!(features & NETIF_F_LOOPBACK));
- /* Due to E830 hardware limitations, TSO (NETIF_F_ALL_TSO) with GCS
- * (NETIF_F_HW_CSUM) is not supported.
- */
- if (ice_is_feature_supported(pf, ICE_F_GCS) &&
- ((features & NETIF_F_HW_CSUM) && (features & NETIF_F_ALL_TSO))) {
- if (netdev->features & NETIF_F_HW_CSUM)
- dev_err(ice_pf_to_dev(pf), "To enable TSO, you must first disable HW checksum.\n");
- else
- dev_err(ice_pf_to_dev(pf), "To enable HW checksum, you must first disable TSO.\n");
- return -EIO;
- }
-
return ret;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
@ 2026-04-28 8:34 ` Loktionov, Aleksandr
2026-04-30 15:29 ` Simon Horman
2026-05-04 23:53 ` [Intel-wired-lan] " Jacob Keller
2 siblings, 0 replies; 10+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-28 8:34 UTC (permalink / raw)
To: Michal Swiatkowski, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, jramaseu@redhat.com, Nguyen, Anthony L,
Kitszel, Przemyslaw
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Tuesday, April 28, 2026 9:07 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; jramaseu@redhat.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com>
> Subject: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of
> IP/IPV6
>
> The hardware is capable of calculating checksum for IPV6 packets with
> extension header. To not drop such packets switch from IP/IPV6
> checksum to HW_CSUM.
I'd recommend "To not drop" -> "To avoid dropping"
>
> HW_CSUM is also used in previous generation (i40e).
>
> Previously HW_CSUM was used to indicate that hardware supports general
> checksum. Drop it assuming that if the hardware supports it, it is
> used.
>
> Disabling offload for E830 in case of TSO isn't needed anymore as the
> check for TSO is done in Tx path just before preparation of the
> special GCS descriptor.
>
> The commit from Fixes didn't introduce a bug, it just shown that the
> driver is doing sth wrong with the checksum features.
>
Except commit message nits
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Suggested-by: Jakub Ramaseuski <jramaseu@redhat.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Fixes: 04c20a9356f2 ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6
> header contains extension")
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 15550216fbf0..0f2f949af536 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3500,9 +3500,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;
>
> vlano_features = NETIF_F_HW_VLAN_CTAG_FILTER |
> NETIF_F_HW_VLAN_CTAG_TX |
> @@ -3564,12 +3563,6 @@ void ice_set_netdev_features(struct net_device
> *netdev)
> /* Allow core to manage IRQs affinity */
> netif_set_affinity_auto(netdev);
>
> - /* Mutual exclusivity for TSO and GCS is enforced by the set
> features
> - * ndo callback.
> - */
> - if (ice_is_feature_supported(pf, ICE_F_GCS))
> - netdev->hw_features |= NETIF_F_HW_CSUM;
> -
> netif_set_tso_max_size(netdev, ICE_MAX_TSO_SIZE); }
>
> @@ -6489,18 +6482,6 @@ ice_set_features(struct net_device *netdev,
> netdev_features_t features)
> if (changed & NETIF_F_LOOPBACK)
> ret = ice_set_loopback(vsi, !!(features &
> NETIF_F_LOOPBACK));
>
> - /* Due to E830 hardware limitations, TSO (NETIF_F_ALL_TSO) with
> GCS
> - * (NETIF_F_HW_CSUM) is not supported.
> - */
> - if (ice_is_feature_supported(pf, ICE_F_GCS) &&
> - ((features & NETIF_F_HW_CSUM) && (features &
> NETIF_F_ALL_TSO))) {
> - if (netdev->features & NETIF_F_HW_CSUM)
> - dev_err(ice_pf_to_dev(pf), "To enable TSO, you
> must first disable HW checksum.\n");
> - else
> - dev_err(ice_pf_to_dev(pf), "To enable HW
> checksum, you must first disable TSO.\n");
> - return -EIO;
> - }
> -
> return ret;
> }
>
> --
> 2.49.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
2026-04-28 8:34 ` Loktionov, Aleksandr
@ 2026-04-30 15:29 ` Simon Horman
2026-04-30 16:21 ` Simon Horman
2026-05-04 23:53 ` [Intel-wired-lan] " Jacob Keller
2 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2026-04-30 15:29 UTC (permalink / raw)
To: michal.swiatkowski
Cc: 'Simon Horman', intel-wired-lan, netdev, jramaseu,
anthony.l.nguyen, przemyslaw.kitszel, aleksandr.loktionov
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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-30 15:29 ` Simon Horman
@ 2026-04-30 16:21 ` Simon Horman
2026-05-04 8:09 ` Michal Swiatkowski
0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2026-04-30 16:21 UTC (permalink / raw)
To: michal.swiatkowski
Cc: intel-wired-lan, netdev, jramaseu, anthony.l.nguyen,
przemyslaw.kitszel, aleksandr.loktionov
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-30 16:21 ` Simon Horman
@ 2026-05-04 8:09 ` Michal Swiatkowski
0 siblings, 0 replies; 10+ messages in thread
From: Michal Swiatkowski @ 2026-05-04 8:09 UTC (permalink / raw)
To: Simon Horman
Cc: michal.swiatkowski, intel-wired-lan, netdev, jramaseu,
anthony.l.nguyen, przemyslaw.kitszel, aleksandr.loktionov
On Thu, Apr 30, 2026 at 05:21:49PM +0100, Simon Horman wrote:
> 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).
>
Thanks for adding the comments.
> > ---
> > 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.
>
I think there is a lack of skb_checksum_help() before one error return
from ice_tx_csum(). I will add it.
> > 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.
>
Yeah, righ.
> >
> > 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.
idpf, igbe too. It should return error in such case and drop the packet.
What do you think, should I add the patch for that here or send it separately?
To be honest I prefer sending it separately as this is in already exsisting
code and touch more than one driver.
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
2026-04-28 8:34 ` Loktionov, Aleksandr
2026-04-30 15:29 ` Simon Horman
@ 2026-05-04 23:53 ` Jacob Keller
2026-05-05 4:35 ` Michal Swiatkowski
2 siblings, 1 reply; 10+ messages in thread
From: Jacob Keller @ 2026-05-04 23:53 UTC (permalink / raw)
To: Michal Swiatkowski, intel-wired-lan
Cc: netdev, jramaseu, anthony.l.nguyen, przemyslaw.kitszel,
aleksandr.loktionov
On 4/28/2026 12:06 AM, Michal Swiatkowski wrote:
> The hardware is capable of calculating checksum for IPV6 packets with
> extension header. To not drop such packets switch from IP/IPV6 checksum
> to HW_CSUM.
>
> HW_CSUM is also used in previous generation (i40e).
>
> Previously HW_CSUM was used to indicate that hardware supports general
> checksum. Drop it assuming that if the hardware supports it, it is used.
>
> Disabling offload for E830 in case of TSO isn't needed anymore as the
> check for TSO is done in Tx path just before preparation of the special
> GCS descriptor.
>
> The commit from Fixes didn't introduce a bug, it just shown that the
> driver is doing sth wrong with the checksum features.
>
> Suggested-by: Jakub Ramaseuski <jramaseu@redhat.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Fixes: 04c20a9356f2 ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension")
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
Am I correct in thinking that this supersedes (really, properly fixes)
the patch "ice: enable NETIF_F_HW_CSUM for GSO packets" at
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20260310150557.1138437-1-jramaseu@redhat.com/
?
Thanks,
Jake
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-05-04 23:53 ` [Intel-wired-lan] " Jacob Keller
@ 2026-05-05 4:35 ` Michal Swiatkowski
2026-05-05 5:16 ` Jacob Keller
0 siblings, 1 reply; 10+ messages in thread
From: Michal Swiatkowski @ 2026-05-05 4:35 UTC (permalink / raw)
To: Jacob Keller
Cc: Michal Swiatkowski, intel-wired-lan, netdev, jramaseu,
anthony.l.nguyen, przemyslaw.kitszel, aleksandr.loktionov
On Mon, May 04, 2026 at 04:53:12PM -0700, Jacob Keller wrote:
> On 4/28/2026 12:06 AM, Michal Swiatkowski wrote:
> > The hardware is capable of calculating checksum for IPV6 packets with
> > extension header. To not drop such packets switch from IP/IPV6 checksum
> > to HW_CSUM.
> >
> > HW_CSUM is also used in previous generation (i40e).
> >
> > Previously HW_CSUM was used to indicate that hardware supports general
> > checksum. Drop it assuming that if the hardware supports it, it is used.
> >
> > Disabling offload for E830 in case of TSO isn't needed anymore as the
> > check for TSO is done in Tx path just before preparation of the special
> > GCS descriptor.
> >
> > The commit from Fixes didn't introduce a bug, it just shown that the
> > driver is doing sth wrong with the checksum features.
> >
> > Suggested-by: Jakub Ramaseuski <jramaseu@redhat.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Fixes: 04c20a9356f2 ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension")
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> Am I correct in thinking that this supersedes (really, properly fixes)
> the patch "ice: enable NETIF_F_HW_CSUM for GSO packets" at
> https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20260310150557.1138437-1-jramaseu@redhat.com/
> ?
>
> Thanks,
> Jake
Yes, exactly. I think I linked it in cover letter, but maybe I should do
it also here.
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6
2026-05-05 4:35 ` Michal Swiatkowski
@ 2026-05-05 5:16 ` Jacob Keller
0 siblings, 0 replies; 10+ messages in thread
From: Jacob Keller @ 2026-05-05 5:16 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: intel-wired-lan, netdev, jramaseu, anthony.l.nguyen,
przemyslaw.kitszel, aleksandr.loktionov
On 5/4/2026 9:35 PM, Michal Swiatkowski wrote:
> On Mon, May 04, 2026 at 04:53:12PM -0700, Jacob Keller wrote:
>> On 4/28/2026 12:06 AM, Michal Swiatkowski wrote:
>>> The hardware is capable of calculating checksum for IPV6 packets with
>>> extension header. To not drop such packets switch from IP/IPV6 checksum
>>> to HW_CSUM.
>>>
>>> HW_CSUM is also used in previous generation (i40e).
>>>
>>> Previously HW_CSUM was used to indicate that hardware supports general
>>> checksum. Drop it assuming that if the hardware supports it, it is used.
>>>
>>> Disabling offload for E830 in case of TSO isn't needed anymore as the
>>> check for TSO is done in Tx path just before preparation of the special
>>> GCS descriptor.
>>>
>>> The commit from Fixes didn't introduce a bug, it just shown that the
>>> driver is doing sth wrong with the checksum features.
>>>
>>> Suggested-by: Jakub Ramaseuski <jramaseu@redhat.com>
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> Fixes: 04c20a9356f2 ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension")
>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
>>> ---
>> Am I correct in thinking that this supersedes (really, properly fixes)
>> the patch "ice: enable NETIF_F_HW_CSUM for GSO packets" at
>> https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20260310150557.1138437-1-jramaseu@redhat.com/
>> ?
>>
>> Thanks,
>> Jake
>
> Yes, exactly. I think I linked it in cover letter, but maybe I should do
> it also here.
>
> Thanks
I think its fine, I just wanted to make sure I was correct in marking
that patch as rejected in patchwork.
Thanks,
Jake
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-05-05 5:16 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 7:06 [PATCH iwl-net v1 0/2] Rework ice Tx checksum Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 1/2] ice: always do GCS if hardware supports it Michal Swiatkowski
2026-04-28 7:06 ` [PATCH iwl-net v1 2/2] ice: use NETIF_F_HW_CSUM instead of IP/IPV6 Michal Swiatkowski
2026-04-28 8:34 ` Loktionov, Aleksandr
2026-04-30 15:29 ` Simon Horman
2026-04-30 16:21 ` Simon Horman
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox