netdev - Re: [PATCH net-next 12/12] ice: Ethtool fdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 9 May 2023 17:14:28 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: "Drewek, Wojciech" <wojciech.drewek@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Ertman, David M"
	<david.m.ertman@...el.com>, "michal.swiatkowski@...ux.intel.com"
	<michal.swiatkowski@...ux.intel.com>, "marcin.szycik@...ux.intel.com"
	<marcin.szycik@...ux.intel.com>, "Chmielewski, Pawel"
	<pawel.chmielewski@...el.com>, "Samudrala, Sridhar"
	<sridhar.samudrala@...el.com>
Subject: Re: [PATCH net-next 12/12] ice: Ethtool fdb_cnt stats

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

> 
> 
>> -----Original Message-----
>> From: Lobakin, Aleksander <aleksander.lobakin@...el.com>
>> Sent: piątek, 21 kwietnia 2023 18:33
>> To: Drewek, Wojciech <wojciech.drewek@...el.com>
>> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Ertman, David M <david.m.ertman@...el.com>;
>> michal.swiatkowski@...ux.intel.com; marcin.szycik@...ux.intel.com; Chmielewski, Pawel <pawel.chmielewski@...el.com>;
>> Samudrala, Sridhar <sridhar.samudrala@...el.com>
>> Subject: Re: [PATCH net-next 12/12] ice: Ethtool fdb_cnt stats
>>
>> From: Wojciech Drewek <wojciech.drewek@...el.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