On 12/5/2025 2:44 PM, [email protected] wrote: > On 12/5/25 12:56 PM, "Loktionov, Aleksandr" <[email protected]> > wrote: >> >> >>> -----Original Message----- >>> From: Jesse Brandeburg <[email protected]> >>> Sent: Friday, December 5, 2025 8:05 PM >>> To: Loktionov, Aleksandr <[email protected]>; Jesse >>> Brandeburg <[email protected]>; [email protected] >>> Cc: Nguyen, Anthony L <[email protected]>; Keller, Jacob E >>> <[email protected]>; IWL <[email protected]>; >>> Kitszel, Przemyslaw <[email protected]>; Andrew Lunn >>> <[email protected]>; David S. Miller <[email protected]>; Eric >>> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; >>> Paolo Abeni <[email protected]> >>> 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
OpenPGP_signature.asc
Description: OpenPGP digital signature
