[Intel-wired-lan] [PATCH net-next 12/12] ice: Ethtool fdb_cnt stats

[Alexander Lobakin]

From: Wojciech Drewek <wojciech.drewek at intel.com>
Date: Tue, 9 May 2023 14:52:26 +0200

> 
> 
>> -----Original Message-----
>> From: Lobakin, Aleksander <aleksander.lobakin at intel.com>
>> Sent: piątek, 21 kwietnia 2023 18:33
>> To: Drewek, Wojciech <wojciech.drewek at intel.com>
>> Cc: intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; Ertman, David M <david.m.ertman at intel.com>;
>> michal.swiatkowski at linux.intel.com; marcin.szycik at linux.intel.com; Chmielewski, Pawel <pawel.chmielewski at intel.com>;
>> Samudrala, Sridhar <sridhar.samudrala at intel.com>
>> Subject: Re: [PATCH net-next 12/12] ice: Ethtool fdb_cnt stats
>>
>> From: Wojciech Drewek <wojciech.drewek at intel.com>
>> Date: Mon, 17 Apr 2023 11:34:12 +0200
>>
>>> Introduce new ethtool statistic which is 'fdb_cnt'. It
>>> provides information about how many bridge fdbs are created on
>>> a given netdev.
>>
>> [...]
>>
>>> @@ -339,6 +340,7 @@ ice_eswitch_br_fdb_entry_delete(struct ice_esw_br *bridge,
>>>  	ice_eswitch_br_flow_delete(pf, fdb_entry->flow);
>>>
>>>  	kfree(fdb_entry);
>>> +	vsi->fdb_cnt--;
>>
>> Are FDB operations always serialized within one netdev? Because if it's
>> not, this probably needs to be atomic_t.
> 
> All the FDB operations are done either from notification context so they are protected by
> rtnl_lock or explicitly protected by us (see ice_eswitch_br_fdb_event_work, we use rtnl_lock there).

BTW, I would replace relying on RTNL lock with own locks bit-by-bit. I
would say, it was designed more for the kernel core internal usage, but
then got abused by tons of drivers.
Sure, it's outside of this series' scope, just FYI. This one is fine for
me as long as concurrent accesses from different SMP CPUs can't happen here.

[...]

Thanks,
Olek