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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YlQj11JOHOYB+f62@lunn.ch>
Date:   Mon, 11 Apr 2022 14:49:27 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org,
        lorenzo.bianconi@...hat.com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, thomas.petazzoni@...tlin.com,
        linux@...linux.org.uk, jbrouer@...hat.com, jdamato@...tly.com
Subject: Re: [PATCH v3 net-next 1/2] net: page_pool: introduce ethtool stats

On Mon, Apr 11, 2022 at 03:34:21PM +0300, Ilias Apalodimas wrote:
> On Mon, 11 Apr 2022 at 15:28, Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> >
> > > Hi Lorenzo,
> >
> > Hi Ilias,
> >
> > >
> > > [...]
> > >
> > > >
> > > >         for_each_possible_cpu(cpu) {
> > > >                 const struct page_pool_recycle_stats *pcpu =
> > > > @@ -66,6 +87,47 @@ bool page_pool_get_stats(struct page_pool *pool,
> > > >         return true;
> > > >  }
> > > >  EXPORT_SYMBOL(page_pool_get_stats);
> > > > +
> > > > +u8 *page_pool_ethtool_stats_get_strings(u8 *data)
> > > > +{
> > > > +       int i;
> > > > +
> > > > +       for (i = 0; i < ARRAY_SIZE(pp_stats); i++) {
> > > > +               memcpy(data, pp_stats[i], ETH_GSTRING_LEN);
> > > > +               data += ETH_GSTRING_LEN;
> > > > +       }
> > > > +
> > > > +       return data;
> > >
> > > Is there a point returning data here or can we make this a void?
> >
> > it is to add the capability to add more strings in the driver code after
> > running page_pool_ethtool_stats_get_strings.
> 
> But the current driver isn't using it.

It could be you need it for the mlx5 driver, which puts the TLS
counters after the page pool counters. Or you could just move them to
the end. I don't think the order of statistics are ABI, just the
strings themselves..

> I don't have too much
> experience with how drivers consume ethtool stats, but would it make
> more sense to return a length instead of a pointer? Maybe Andrew has
> an idea.

Either is acceptable. Even if you do make it a void, the driver can
use the stats_get_count() and do the maths. But a length or a pointer
is simpler.

   Andrew

Powered by blists - more mailing lists