Re: [Intel-wired-lan] [PATCH net v2] ice: Fix missing 1's complement negation in GCS raw checksum

Re: [Intel-wired-lan] [PATCH net v2] ice: Fix missing 1's complement negation in GCS raw checksum

[Jacob Keller]

On 5/1/2026 2:57 AM, Matt Fleming wrote:
> From: Matt Fleming <mfleming@cloudflare.com>
> 
> Commit 905d1a220e8d ("ice: Add E830 checksum offload support") added
> Generic Checksum (GCS) support for E830 NICs but omitted the 1's
> complement negation (~) when converting the hardware raw_csum to
> skb->csum for CHECKSUM_COMPLETE.
> 
> Without the negation, every CHECKSUM_COMPLETE packet fails the
> fast-path validation in nf_ip_checksum() and falls through to software
> checksumming via __skb_checksum_complete(), which triggers the
> rate-limited "hw csum failure" warning. Packets are still accepted
> (the software recheck passes) but hardware checksum offload is
> effectively disabled and the warning floods dmesg on systems running
> nf_conntrack on VLAN sub-interfaces.
> 
> Multiple other drivers (idpf, ehea, iwlwifi, cassini, sunhme, enetc)
> also apply ~ for CHECKSUM_COMPLETE. The ice driver was the only in-tree
> user of csum_unfold() for CHECKSUM_COMPLETE that omitted it.
> 

Hi,

Based on your patch description, I assume that you've tested this on
real hardware.

I dug a little through some of our internal changes history and sawe
that it looks like the hardware has a register setting in its
GL_RDPU_CNTRL register which determines whether the checksum value
reported is inverted or not. In E830 hardware, it is supposed to be off
(i.e. the checksum value reported already matches the expected setting.

Perhaps your device somehow got the GL_RDPU_CNTRL register set to the
wrong mode and that results in the swap being necessary. Hmm.

I'll ask the team to see if they can confirm this behavior.

Thanks,
Jake

> Fixes: 905d1a220e8d ("ice: Add E830 checksum offload support")
> Cc: stable@vger.kernel.org
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Matt Fleming <mfleming@cloudflare.com>
> ---
> v2:
>   - Add Cc: stable@vger.kernel.org (Aleksandr)
>   - Pick up Reviewed-by tags from Aleksandr and Simon
>   - No code changes
> v1: https://lore.kernel.org/netdev/20260408190214.1287708-1-matt@readmodwrite.com/
> 
>  drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> index e695a664e53d..c177579e0114 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> @@ -92,7 +92,7 @@ static void ice_rx_gcs(struct sk_buff *skb,
>  	desc = (struct ice_32b_rx_flex_desc_nic *)rx_desc;
>  	skb->ip_summed = CHECKSUM_COMPLETE;
>  	csum = (__force u16)desc->raw_csum;
> -	skb->csum = csum_unfold((__force __sum16)swab16(csum));
> +	skb->csum = csum_unfold((__force __sum16)~swab16(csum));
>  }
>  
>  /**

Re: [Intel-wired-lan] [PATCH net v2] ice: Fix missing 1's complement negation in GCS raw checksum

[Matt Fleming]

On Mon, May 04, 2026 at 05:10:23PM -0700, Jacob Keller wrote:
> 
> Hi,
> 
> Based on your patch description, I assume that you've tested this on
> real hardware.
> 
> I dug a little through some of our internal changes history and sawe
> that it looks like the hardware has a register setting in its
> GL_RDPU_CNTRL register which determines whether the checksum value
> reported is inverted or not. In E830 hardware, it is supposed to be off
> (i.e. the checksum value reported already matches the expected setting.
> 
> Perhaps your device somehow got the GL_RDPU_CNTRL register set to the
> wrong mode and that results in the swap being necessary. Hmm.
> 
> I'll ask the team to see if they can confirm this behavior.

Hi Jake,

Thanks for digging into this.

I read GL_RDPU_CNTRL on our affected E830 and the value is the same on
both ports of the NIC:

  0000:c1:00.0: GL_RDPU_CNTRL = 0x0020a275
  0000:c1:00.1: GL_RDPU_CNTRL = 0x0020a275

Decoding bit 22 (E830_GL_RDPU_CNTRL_CHECKSUM_COMPLETE_INV) gives 0,
i.e. the hardware is supposedly in "not inverted" mode, which matches
the default you described.

However, looking at the data on the wire I see:

  - netdev_rx_csum_fault fires ~65 000 times/sec on this host.
  - bpftrace at fexit:ice_process_skb_fields shows skb->csum =
    swab16(raw_csum) directly (no negation), e.g. raw_csum=0xfb4f
    -> skb->csum=0x4ffb.
  - At fentry:__skb_checksum_complete the upper 16 bits of skb->csum
    are 0xFFFF on every TCP/UDP packet -- the signature of nf_ip_checksum
    adding the pseudo-header to a value that was the un-negated raw_csum.
  - fold2(skb->csum_at_fentry + skb_checksum(skb,0,len,0)) ≈ 0xFFFF
    for every packet, which means the two values are ones-complement
    complements of each other, i.e. the driver stored S where the
    stack expects ~S.

Negating the checksum makes the failures go away.

Thanks,
Matt

Re: [Intel-wired-lan] [PATCH net v2] ice: Fix missing 1's complement negation in GCS raw checksum

[Jacob Keller]

On 5/7/2026 2:34 AM, Matt Fleming wrote:
> On Mon, May 04, 2026 at 05:10:23PM -0700, Jacob Keller wrote:
>>
>> Hi,
>>
>> Based on your patch description, I assume that you've tested this on
>> real hardware.
>>
>> I dug a little through some of our internal changes history and sawe
>> that it looks like the hardware has a register setting in its
>> GL_RDPU_CNTRL register which determines whether the checksum value
>> reported is inverted or not. In E830 hardware, it is supposed to be off
>> (i.e. the checksum value reported already matches the expected setting.
>>
>> Perhaps your device somehow got the GL_RDPU_CNTRL register set to the
>> wrong mode and that results in the swap being necessary. Hmm.
>>
>> I'll ask the team to see if they can confirm this behavior.
> 
> Hi Jake,
> 
> Thanks for digging into this.
> 

I'm still trying to see if I can get someone on our team with hardware
setup to confirm this behavior.

> I read GL_RDPU_CNTRL on our affected E830 and the value is the same on
> both ports of the NIC:
> 
>   0000:c1:00.0: GL_RDPU_CNTRL = 0x0020a275
>   0000:c1:00.1: GL_RDPU_CNTRL = 0x0020a275
> 

Ok. Makes sense.

> Decoding bit 22 (E830_GL_RDPU_CNTRL_CHECKSUM_COMPLETE_INV) gives 0,
> i.e. the hardware is supposedly in "not inverted" mode, which matches
> the default you described.
> 
I wonder if it would actually work if we set the bit to 1. It could be
that there was miscommunication between us and the hardware folks so
what "inverted" means got.. inverted. (pun intended).

> However, looking at the data on the wire I see:
> 
>   - netdev_rx_csum_fault fires ~65 000 times/sec on this host.
>   - bpftrace at fexit:ice_process_skb_fields shows skb->csum =
>     swab16(raw_csum) directly (no negation), e.g. raw_csum=0xfb4f
>     -> skb->csum=0x4ffb.
>   - At fentry:__skb_checksum_complete the upper 16 bits of skb->csum
>     are 0xFFFF on every TCP/UDP packet -- the signature of nf_ip_checksum
>     adding the pseudo-header to a value that was the un-negated raw_csum.
>   - fold2(skb->csum_at_fentry + skb_checksum(skb,0,len,0)) ≈ 0xFFFF
>     for every packet, which means the two values are ones-complement
>     complements of each other, i.e. the driver stored S where the
>     stack expects ~S.
> 
> Negating the checksum makes the failures go away.
> 

Yea. Clearly we're inverting the checksum relative to what the stack wants.

I'm also curious why our validation folks haven't noticed or
complained.. but I think it may be partially because we have to disable
the GSC support when operating TSO offload, so TCP traffic won't be able
to replicate this. Hmm... Oh....

I wonder if the way we distinguish between these modes isn't per-flow
but instead per-device.. so if you have NETIF_F_ALL_TSO enabled you
won't get NETIF_F_HW_CSUM, which means that we'd go through the old
legacy path... Ugh. That might explain how this escaped testing :(

I've queued your v2 to dev-queue, so at the very least we can get some
validation and confirm the behavior.

> Thanks,
> Matt

Re: [Intel-wired-lan] [PATCH net v2] ice: Fix missing 1's complement negation in GCS raw checksum

[Jacob Keller]

On 5/7/2026 2:34 AM, Matt Fleming wrote:
> On Mon, May 04, 2026 at 05:10:23PM -0700, Jacob Keller wrote:
>>
>> Hi,
>>
>> Based on your patch description, I assume that you've tested this on
>> real hardware.
>>
>> I dug a little through some of our internal changes history and sawe
>> that it looks like the hardware has a register setting in its
>> GL_RDPU_CNTRL register which determines whether the checksum value
>> reported is inverted or not. In E830 hardware, it is supposed to be off
>> (i.e. the checksum value reported already matches the expected setting.
>>
>> Perhaps your device somehow got the GL_RDPU_CNTRL register set to the
>> wrong mode and that results in the swap being necessary. Hmm.
>>
>> I'll ask the team to see if they can confirm this behavior.
> 
> Hi Jake,
> 
> Thanks for digging into this.
> 
> I read GL_RDPU_CNTRL on our affected E830 and the value is the same on
> both ports of the NIC:
> 
>   0000:c1:00.0: GL_RDPU_CNTRL = 0x0020a275
>   0000:c1:00.1: GL_RDPU_CNTRL = 0x0020a275
> 
> Decoding bit 22 (E830_GL_RDPU_CNTRL_CHECKSUM_COMPLETE_INV) gives 0,
> i.e. the hardware is supposedly in "not inverted" mode, which matches
> the default you described.
> 
> However, looking at the data on the wire I see:
> 
>   - netdev_rx_csum_fault fires ~65 000 times/sec on this host.
>   - bpftrace at fexit:ice_process_skb_fields shows skb->csum =
>     swab16(raw_csum) directly (no negation), e.g. raw_csum=0xfb4f
>     -> skb->csum=0x4ffb.
>   - At fentry:__skb_checksum_complete the upper 16 bits of skb->csum
>     are 0xFFFF on every TCP/UDP packet -- the signature of nf_ip_checksum
>     adding the pseudo-header to a value that was the un-negated raw_csum.
>   - fold2(skb->csum_at_fentry + skb_checksum(skb,0,len,0)) ≈ 0xFFFF
>     for every packet, which means the two values are ones-complement
>     complements of each other, i.e. the driver stored S where the
>     stack expects ~S.
> 
> Negating the checksum makes the failures go away.
> 
> Thanks,
> Matt

Ok. This is getting strange. I checked a system I was able to borrow. In
our setup, we need the existing code (no negation), otherwise we see
checksum failures even for simple pings, including the initial
DO_ONCE_LITE stack dump, but I can see with ftrace that
netdev_rx_csum_fault is firing every packet.

I have the following on my system:

> $ lspci | grep Ethernet
> 17:00.0 Ethernet controller: Intel Corporation Ethernet Controller E830-CC for QSFP
> 17:00.1 Ethernet controller: Intel Corporation Ethernet Controller E830-CC for QSFP

> 
> $ devlink dev info pci/0000:17:00.0
> pci/0000:17:00.0:
>   driver ice
>   serial_number 00-01-00-ff-ff-00-00-00
>   versions:
>       fixed:
>         board.id N31483-000
>       running:
>         fw.mgmt 7.9.5
>         fw.mgmt.api 1.7.11
>         fw.mgmt.build 0x2bbde9a9
>         fw.undi 1.3910.0
>         fw.psid.api 1.20
>         fw.bundle_id 0x80017eed
>         fw.app.name ICE OS Default Package
>         fw.app 1.3.43.0
>         fw.app.bundle_id 0xc0000001
>         fw.netlist 0.0.1100-2.53.0
>         fw.netlist.build 0xf77b1d74
>       stored:
>         fw.undi 1.3910.0
>         fw.psid.api 1.20
>         fw.bundle_id 0x80017eed
>         fw.netlist 0.0.1100-2.53.0
>         fw.netlist.build 0xf77b1d74


I checked the GL_RDPU_CNTRL register, and indeed the checksum invert bit
is clear, matching your report. Even so, I see the opposite behavior you
do: we need to avoid the invert otherwise the checksums are reported
invalid.

Interestingly, I tried writing to GL_RDPU_CNTRL to set the invert bit,
which appears to have stuck but it doesn't seem to affect the behavior.

I wonder if there is an NVM difference here. It is possible that our
boards have a pre-production image still installed, and that might be
impacting the results. I've asked to try and have the devices updated to
confirm the behavior...


I'm suspicious that somehow there is an NVM setting that toggles this
behavior and which doesn't seem to respect the actual register setting.
I haven't been able to track down more information in our internal
documentation yet :(

In the mean time, could you share the device ID and firmware data from
devlink info? Feel free to reach out to my Intel address directly if you
don't want to share something on the public list. It would be very
helpful to know the device and NVM data from your setup so that we can
try to replicate it here.

Thanks,
Jake