[Intel-wired-lan] [PATCH net-next 12/12] ice: Ethtool fdb_cnt stats
Drewek, Wojciech
wojciech.drewek at intel.com
Tue May 9 12:52:26 UTC 2023
> -----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).
>
> > }
> >
> > static void
>
> [...]
>
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > index 8407c7175cf6..d06b2a688323 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > @@ -64,6 +64,7 @@ static const struct ice_stats ice_gstrings_vsi_stats[] = {
> > ICE_VSI_STAT("tx_linearize", tx_linearize),
> > ICE_VSI_STAT("tx_busy", tx_busy),
> > ICE_VSI_STAT("tx_restart", tx_restart),
> > + ICE_VSI_STAT("fdb_cnt", fdb_cnt),
>
> It's confusing to me to see it in the Ethtool stats. They're usually
> counters, ice is no an exception. But this one is not, so it might give
> wrong impression.
> Have you considered alternatives? rtnl (iproute) or maybe even Devlink
> (but I believe the former fits better)? This might be a good candidate
> to become a generic stat, who knows.
I'll do some research on alternatives
>
> > };
> >
> > enum ice_ethtool_test_id {
>
> Thanks,
> Olek
More information about the Intel-wired-lan
mailing list