[PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Jesse Brandeburg]

From: Jesse Brandeburg <jbrandeburg@cloudflare.com>

Since the beginning, the Intel ice driver has counted receive checksum
offload mismatches into the rx_errors member of the rtnl_link_stats64
struct. In ethtool -S these show up as rx_csum_bad.nic.

I believe counting these in rx_errors is fundamentally wrong, as it's
pretty clear from the comments in if_link.h and from every other statistic
the driver is summing into rx_errors, that all of them would cause a
"hardware drop" except for the UDP checksum mismatch, as well as the fact
that all the other causes for rx_errors are L2 reasons, and this L4 UDP
"mismatch" is an outlier.

A last nail in the coffin is that rx_errors is monitored in production and
can indicate a bad NIC/cable/Switch port, but instead some random series of
UDP packets with bad checksums will now trigger this alert. This false
positive makes the alert useless and affects us as well as other companies.

This packet with presumably a bad UDP checksum is *already* passed to the
stack, just not marked as offloaded by the hardware/driver. If it is
dropped by the stack it will show up as UDP_MIB_CSUMERRORS.

And one more thing, none of the other Intel drivers, and at least bnxt_en
and mlx5 both don't appear to count UDP offload mismatches as rx_errors.

Here is a related customer complaint:
https://community.intel.com/t5/Ethernet-Products/ice-rx-errros-is-too-sensitive-to-IP-TCP-attack-packets-Intel/td-p/1662125

Fixes: 4f1fe43c920b ("ice: Add more Rx errors to netdev's rx_error counter")
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: Jake Keller <jacob.e.keller@intel.com>
Cc: IWL <intel-wired-lan@lists.osuosl.org>
Signed-off-by: Jesse Brandeburg <jbrandeburg@cloudflare.com>
--
I am sending this to net as I consider it a bug, and it will backport
cleanly.
---
 drivers/net/ethernet/intel/ice/ice_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 86f5859e88ef..d004acfa0f36 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi *vsi)
 		cur_ns->rx_errors = pf->stats.crc_errors +
 				    pf->stats.illegal_bytes +
 				    pf->stats.rx_undersize +
-				    pf->hw_csum_rx_error +
 				    pf->stats.rx_jabber +
 				    pf->stats.rx_fragments +
 				    pf->stats.rx_oversize;
-- 
2.47.3

Re: [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 2993 bytes --]



On 12/1/2025 3:38 PM, Jesse Brandeburg wrote:
> From: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> 
> Since the beginning, the Intel ice driver has counted receive checksum
> offload mismatches into the rx_errors member of the rtnl_link_stats64
> struct. In ethtool -S these show up as rx_csum_bad.nic.
> 
> I believe counting these in rx_errors is fundamentally wrong, as it's
> pretty clear from the comments in if_link.h and from every other statistic
> the driver is summing into rx_errors, that all of them would cause a
> "hardware drop" except for the UDP checksum mismatch, as well as the fact
> that all the other causes for rx_errors are L2 reasons, and this L4 UDP
> "mismatch" is an outlier.
> 
> A last nail in the coffin is that rx_errors is monitored in production and
> can indicate a bad NIC/cable/Switch port, but instead some random series of
> UDP packets with bad checksums will now trigger this alert. This false
> positive makes the alert useless and affects us as well as other companies.
> 
> This packet with presumably a bad UDP checksum is *already* passed to the
> stack, just not marked as offloaded by the hardware/driver. If it is
> dropped by the stack it will show up as UDP_MIB_CSUMERRORS.
> 
> And one more thing, none of the other Intel drivers, and at least bnxt_en
> and mlx5 both don't appear to count UDP offload mismatches as rx_errors.
> 
> Here is a related customer complaint:
> https://community.intel.com/t5/Ethernet-Products/ice-rx-errros-is-too-sensitive-to-IP-TCP-attack-packets-Intel/td-p/1662125
> 
> Fixes: 4f1fe43c920b ("ice: Add more Rx errors to netdev's rx_error counter")
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Jake Keller <jacob.e.keller@intel.com>
> Cc: IWL <intel-wired-lan@lists.osuosl.org>
> Signed-off-by: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> --
> I am sending this to net as I consider it a bug, and it will backport
> cleanly.
> ---

Its fine with me. I can't find anything explaining why we originally
chose to put these in rx_errors, and I think its better to align with
other drivers and vendors. I suspect its just as "this is an error, it
obviously goes in rx_errors" even though its of a completely different kind.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/intel/ice/ice_main.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 86f5859e88ef..d004acfa0f36 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi *vsi)
>  		cur_ns->rx_errors = pf->stats.crc_errors +
>  				    pf->stats.illegal_bytes +
>  				    pf->stats.rx_undersize +
> -				    pf->hw_csum_rx_error +
>  				    pf->stats.rx_jabber +
>  				    pf->stats.rx_fragments +
>  				    pf->stats.rx_oversize;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

RE: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jesse Brandeburg
> Sent: Tuesday, December 2, 2025 12:39 AM
> To: netdev@vger.kernel.org
> Cc: Brandeburg, Jesse <jbrandeburg@cloudflare.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; IWL <intel-wired-lan@lists.osuosl.org>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Brett Creeley <brett.creeley@intel.com>
> Subject: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum
> mismatch as rx_errors
> 
> From: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> 
> Since the beginning, the Intel ice driver has counted receive checksum
> offload mismatches into the rx_errors member of the rtnl_link_stats64
> struct. In ethtool -S these show up as rx_csum_bad.nic.
> 
> I believe counting these in rx_errors is fundamentally wrong, as it's
> pretty clear from the comments in if_link.h and from every other
> statistic
> the driver is summing into rx_errors, that all of them would cause a
> "hardware drop" except for the UDP checksum mismatch, as well as the
> fact
> that all the other causes for rx_errors are L2 reasons, and this L4
> UDP
> "mismatch" is an outlier.
> 
> A last nail in the coffin is that rx_errors is monitored in production
> and
> can indicate a bad NIC/cable/Switch port, but instead some random
> series of
> UDP packets with bad checksums will now trigger this alert. This false
> positive makes the alert useless and affects us as well as other
> companies.
> 
> This packet with presumably a bad UDP checksum is *already* passed to
> the
> stack, just not marked as offloaded by the hardware/driver. If it is
> dropped by the stack it will show up as UDP_MIB_CSUMERRORS.
> 
> And one more thing, none of the other Intel drivers, and at least
> bnxt_en
> and mlx5 both don't appear to count UDP offload mismatches as
> rx_errors.
> 
> Here is a related customer complaint:
> https://community.intel.com/t5/Ethernet-Products/ice-rx-errros-is-too-
> sensitive-to-IP-TCP-attack-packets-Intel/td-p/1662125
> 
> Fixes: 4f1fe43c920b ("ice: Add more Rx errors to netdev's rx_error
> counter")
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Jake Keller <jacob.e.keller@intel.com>
> Cc: IWL <intel-wired-lan@lists.osuosl.org>
> Signed-off-by: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> --
> I am sending this to net as I consider it a bug, and it will backport
> cleanly.
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 86f5859e88ef..d004acfa0f36 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi *vsi)
>  		cur_ns->rx_errors = pf->stats.crc_errors +
>  				    pf->stats.illegal_bytes +
>  				    pf->stats.rx_undersize +
> -				    pf->hw_csum_rx_error +

Good day , Jesse
It looks like you remove the single place where the ' hw_csum_rx_error' var is being really used.
What about removing it's declaration and calculation then?

>  				    pf->stats.rx_jabber +
>  				    pf->stats.rx_fragments +
>  				    pf->stats.rx_oversize;
> --
> 2.47.3

Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Jesse Brandeburg]

On 12/5/25 12:26 AM, Loktionov, Aleksandr wrote:
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index 86f5859e88ef..d004acfa0f36 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi *vsi)
>>   		cur_ns->rx_errors = pf->stats.crc_errors +
>>   				    pf->stats.illegal_bytes +
>>   				    pf->stats.rx_undersize +
>> -				    pf->hw_csum_rx_error +
> 
> Good day , Jesse
> It looks like you remove the single place where the ' hw_csum_rx_error' var is being really used.
> What about removing it's declaration and calculation then?

Hi Aleks! That's not true, however, as the stat is incremented in 
receive path and shown in ethtool -S. I think it is incredibly valuable 
to have in the ethtool stats that the hardware is "not offloading" a 
checksum. As well, all the other drivers in the high-speed Ethernet 
category have a similar counter.

I hope you'll agree it's still useful?

RE: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> Sent: Friday, December 5, 2025 8:05 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Jesse
> Brandeburg <jbrandeb@kernel.org>; netdev@vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; IWL <intel-wired-lan@lists.osuosl.org>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>
> Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP
> csum mismatch as rx_errors
> 
> On 12/5/25 12:26 AM, Loktionov, Aleksandr wrote:
> >> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> >> b/drivers/net/ethernet/intel/ice/ice_main.c
> >> index 86f5859e88ef..d004acfa0f36 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> >> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> >> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi
> *vsi)
> >>   		cur_ns->rx_errors = pf->stats.crc_errors +
> >>   				    pf->stats.illegal_bytes +
> >>   				    pf->stats.rx_undersize +
> >> -				    pf->hw_csum_rx_error +
> >
> > Good day , Jesse
> > It looks like you remove the single place where the '
> hw_csum_rx_error' var is being really used.
> > What about removing it's declaration and calculation then?
> 
> Hi Aleks! That's not true, however, as the stat is incremented in
> receive path and shown in ethtool -S. I think it is incredibly
> valuable to have in the ethtool stats that the hardware is "not
> offloading" a checksum. As well, all the other drivers in the high-
> speed Ethernet category have a similar counter.
> 
> I hope you'll agree it's still useful?

So, the hw_csum_rx_error still will be visible in rx_csum_bad.nic as 'private' ethtool statistics.
But I mean it will be not reflected in the standard "/sys/class/net/<if>/statistics".
What do you think about it? 

Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[jbrandeburg]

On 12/5/25 12:56 PM, "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com> wrote:
> 
> 
> > -----Original Message-----
> > From: Jesse Brandeburg <jbrandeburg@cloudflare.com>
> > Sent: Friday, December 5, 2025 8:05 PM
> > To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Jesse
> > Brandeburg <jbrandeb@kernel.org>; netdev@vger.kernel.org
> > Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E
> > <jacob.e.keller@intel.com>; IWL <intel-wired-lan@lists.osuosl.org>;
> > Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> > <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> > Paolo Abeni <pabeni@redhat.com>
> > Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP
> > csum mismatch as rx_errors
> >
> > On 12/5/25 12:26 AM, Loktionov, Aleksandr wrote:
> >>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> >>> b/drivers/net/ethernet/intel/ice/ice_main.c
> >>> index 86f5859e88ef..d004acfa0f36 100644
> >>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> >>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> >>> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi
> > *vsi)
> >>>    		cur_ns->rx_errors = pf->stats.crc_errors +
> >>>    				    pf->stats.illegal_bytes +
> >>>    				    pf->stats.rx_undersize +
> >>> -				    pf->hw_csum_rx_error +
> >>
> >> Good day , Jesse
> >> It looks like you remove the single place where the '
> > hw_csum_rx_error' var is being really used.
> >> What about removing it's declaration and calculation then?
> >
> > Hi Aleks! That's not true, however, as the stat is incremented in
> > receive path and shown in ethtool -S. I think it is incredibly
> > valuable to have in the ethtool stats that the hardware is "not
> > offloading" a checksum. As well, all the other drivers in the high-
> > speed Ethernet category have a similar counter.
> >
> > I hope you'll agree it's still useful?
> 
> So, the hw_csum_rx_error still will be visible in rx_csum_bad.nic as 'private' ethtool statistics.

Correct.

> But I mean it will be not reflected in the standard "/sys/class/net/<if>/statistics".
> What do you think about it?

As the commit message said, no other drivers reflect this stat in net/interface/statistics (also there is no where to put it). I think not showing this is the whole intent of the patch. If there *was* a bad checksum it will be reflected in the kernel's checksum MIB stats, because the driver will have passed the frame to the stack anyway.

Why should this driver be different than all the other kernel drivers I mentioned in the commit message?

BR,
 Jesse

Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP csum mismatch as rx_errors

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 3076 bytes --]



On 12/5/2025 2:44 PM, jbrandeburg@cloudflare.com wrote:
> On 12/5/25 12:56 PM, "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Jesse Brandeburg <jbrandeburg@cloudflare.com>
>>> Sent: Friday, December 5, 2025 8:05 PM
>>> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Jesse
>>> Brandeburg <jbrandeb@kernel.org>; netdev@vger.kernel.org
>>> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E
>>> <jacob.e.keller@intel.com>; IWL <intel-wired-lan@lists.osuosl.org>;
>>> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
>>> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
>>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>>> Paolo Abeni <pabeni@redhat.com>
>>> Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: stop counting UDP
>>> csum mismatch as rx_errors
>>>
>>> On 12/5/25 12:26 AM, Loktionov, Aleksandr wrote:
>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> index 86f5859e88ef..d004acfa0f36 100644
>>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> @@ -6995,7 +6995,6 @@ void ice_update_vsi_stats(struct ice_vsi
>>> *vsi)
>>>>>    		cur_ns->rx_errors = pf->stats.crc_errors +
>>>>>    				    pf->stats.illegal_bytes +
>>>>>    				    pf->stats.rx_undersize +
>>>>> -				    pf->hw_csum_rx_error +
>>>>
>>>> Good day , Jesse
>>>> It looks like you remove the single place where the '
>>> hw_csum_rx_error' var is being really used.
>>>> What about removing it's declaration and calculation then?
>>>
>>> Hi Aleks! That's not true, however, as the stat is incremented in
>>> receive path and shown in ethtool -S. I think it is incredibly
>>> valuable to have in the ethtool stats that the hardware is "not
>>> offloading" a checksum. As well, all the other drivers in the high-
>>> speed Ethernet category have a similar counter.
>>>
>>> I hope you'll agree it's still useful?
>>
>> So, the hw_csum_rx_error still will be visible in rx_csum_bad.nic as 'private' ethtool statistics.
> 
> Correct.
> 
>> But I mean it will be not reflected in the standard "/sys/class/net/<if>/statistics".
>> What do you think about it?
> 
> As the commit message said, no other drivers reflect this stat in net/interface/statistics (also there is no where to put it). I think not showing this is the whole intent of the patch. If there *was* a bad checksum it will be reflected in the kernel's checksum MIB stats, because the driver will have passed the frame to the stack anyway.
> 
> Why should this driver be different than all the other kernel drivers I mentioned in the commit message?
> 
> BR,
>  Jesse

Right. I agree with Jesse's proposed change. We still keep the stat for
the ethtool but we don't report it as a full error to the standard
statistics. This matches other drivers from our own products and other
vendors.

Thanks,
Jake


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]