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