Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: David Ahern <dsahern@gmail.com>, Ido Schimmel <idosch@idosch.org>,
	netdev@vger.kernel.org, davem@davemloft.net, jiri@nvidia.com,
	amcohen@nvidia.com, danieller@nvidia.com, mlxsw@nvidia.com,
	roopa@nvidia.com, andrew@lunn.ch, vivien.didelot@gmail.com,
	tariqt@nvidia.com, ayal@nvidia.com, mkubecek@suse.cz,
	Ido Schimmel <idosch@nvidia.com>
Subject: Re: [RFC PATCH net-next 0/6] devlink: Add device metric support
Date: Wed, 19 Aug 2020 11:07:25 -0700	[thread overview]
Message-ID: <20200819110725.6e8744ce@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <e4fd9b1c-5f7c-d560-9da0-362ddf93165c@gmail.com>

On Wed, 19 Aug 2020 10:20:08 -0700 Florian Fainelli wrote:
> > I'm trying to find a solution which will not require a policeman to
> > constantly monitor the compliance. Please see my effort to ensure
> > drivers document and use the same ethtool -S stats in the TLS offload
> > implementations. I've been trying to improve this situation for a long
> > time, and it's getting old.  
> 
> Which is why I am asking genuinely what do you think should be done
> besides doing more code reviews? It does not seem to me that there is an
> easy way to catch new stats being added with tools/scripts/whatever and
> then determine what they are about, right?

I don't have a great way forward in mind, sadly. All I can think of is
that we should try to create more well defined interfaces and steer
away from free-form ones.

Example, here if the stats are vxlan decap/encap/error - we should
expose that from the vxlan module. That way vxlan module defines one
set of stats for everyone.

In general unless we attach stats to the object they relate to, we will
end up building parallel structures for exposing statistics from the
drivers. I posted a set once which was implementing hierarchical stats,
but I've abandoned it for this reason.

> > Please focus on the stats this set adds, instead of fantasizing of what
> > could be. These are absolutely not implementation specific!  
> 
> Not sure if fantasizing is quite what I would use. I am just pointing
> out that given the inability to standardize on statistics maybe we
> should have namespaces and try our best to have everything fit into the
> standard namespace along with a standard set of names, and push back
> whenever we see vendor stats being added (or more pragmatically, ask
> what they are). But maybe this very idea is moot.

IDK. I just don't feel like this is going to fly, see how many names
people invented for the CRC error statistic in ethtool -S, even tho
there is a standard stat for that! And users are actually parsing the
output of ethtool -S to get CRC stats because (a) it became the go-to
place for NIC stats and (b) some drivers forget to report in the
standard place.

The cover letter says this set replaces the bad debugfs with a good,
standard API. It may look good and standard for _vendors_ because they
will know where to dump their counters, but it makes very little
difference for _users_. If I have to parse names for every vendor I use,
I can as well add a per-vendor debugfs path to my script.

The bar for implementation-specific driver stats has to be high.

> >>> If I have to download vendor documentation and tooling, or adapt my own
> >>> scripts for every new vendor, I could have as well downloaded an SDK.    
> >>
> >> Are not you being a bit over dramatic here with your example?   
> > 
> > I hope not. It's very hard/impossible today to run a fleet of Linux
> > machines without resorting to vendor tooling.  
> 
> Your argument was putting on the same level resorting to vendor tooling
> to extract meaningful statistics/counters versus using a SDK to operate
> the hardware (this is how I understood it), and I do not believe this is
> fair.

Okay, fair. I just think that in datacenter deployments we are way
closer to the SDK model than people may want to admit.

  reply	other threads:[~2020-08-19 18:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17 12:50 [RFC PATCH net-next 0/6] devlink: Add device metric support Ido Schimmel
2020-08-17 12:50 ` [RFC PATCH net-next 1/6] devlink: Add device metric infrastructure Ido Schimmel
2020-08-17 14:12   ` Andrew Lunn
2020-08-17 12:50 ` [RFC PATCH net-next 2/6] netdevsim: Add devlink metric support Ido Schimmel
2020-08-17 12:50 ` [RFC PATCH net-next 3/6] selftests: netdevsim: Add devlink metric tests Ido Schimmel
2020-08-17 12:50 ` [RFC PATCH net-next 4/6] mlxsw: reg: Add Tunneling NVE Counters Register Ido Schimmel
2020-08-17 12:50 ` [RFC PATCH net-next 5/6] mlxsw: reg: Add Tunneling NVE Counters Register Version 2 Ido Schimmel
2020-08-17 12:50 ` [RFC PATCH net-next 6/6] mlxsw: spectrum_nve: Expose VXLAN counters via devlink-metric Ido Schimmel
2020-08-17 14:29   ` Andrew Lunn
2020-08-18  6:59     ` Ido Schimmel
2020-08-19  0:24 ` [RFC PATCH net-next 0/6] devlink: Add device metric support Jakub Kicinski
2020-08-19  2:43   ` David Ahern
2020-08-19  3:35     ` Jakub Kicinski
2020-08-19  4:30       ` Florian Fainelli
2020-08-19 16:18         ` Jakub Kicinski
2020-08-19 17:20           ` Florian Fainelli
2020-08-19 18:07             ` Jakub Kicinski [this message]
2020-08-20 14:35               ` David Ahern
2020-08-20 16:09                 ` Jakub Kicinski
2020-08-21 10:30                   ` Ido Schimmel
2020-08-21 16:53                     ` Jakub Kicinski
2020-08-21 19:12                       ` David Ahern
2020-08-21 23:50                         ` Jakub Kicinski
2020-08-21 23:59                           ` David Ahern
2020-08-22  0:37                             ` Jakub Kicinski
2020-08-22  1:18                               ` David Ahern
2020-08-22 16:27                                 ` Jakub Kicinski
2020-08-23  7:04                                   ` Ido Schimmel
2020-08-24 19:11                                     ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200819110725.6e8744ce@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=amcohen@nvidia.com \
    --cc=andrew@lunn.ch \
    --cc=ayal@nvidia.com \
    --cc=danieller@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@idosch.org \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=mkubecek@suse.cz \
    --cc=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@nvidia.com \
    --cc=tariqt@nvidia.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).