netdev - Re: [PATCH net-next 4/5] net: mvneta: introduce xdp counters to ethtool

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: <20200217155139.6363aa52@carbon>
Date:   Mon, 17 Feb 2020 15:51:38 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org,
        ilias.apalodimas@...aro.org, davem@...emloft.net,
        David Ahern <dsahern@...nel.org>,
        BPF-dev-list <bpf@...r.kernel.org>, brouer@...hat.com
Subject: Re: [PATCH net-next 4/5] net: mvneta: introduce xdp counters to
 ethtool

On Mon, 17 Feb 2020 14:05:15 +0100
Andrew Lunn <andrew@...n.ch> wrote:

> On Mon, Feb 17, 2020 at 11:32:09AM +0100, Jesper Dangaard Brouer wrote:
> > On Mon, 17 Feb 2020 11:25:50 +0100
> > Lorenzo Bianconi <lorenzo.bianconi@...hat.com> wrote:
> >   
[...]
> > > 
> > > yes, I think it is definitely better. So to follow up:
> > > - rename current "xdp_tx" counter in "xdp_xmit" and increment it for
> > >   XDP_TX verdict and for ndo_xdp_xmit
> > > - introduce a new "xdp_tx" counter only for XDP_TX verdict.
> > > 
> > > If we agree I can post a follow-up patch.  
> > 
> > I agree, that sounds like an improvement to this patchset.
> > 
> > 
> > I suspect David Ahern have some opinions about more general stats for
> > XDP, but that it is a more general discussion, that it outside this
> > patchset, but we should also have that discussion.  
> 
> Hi Jesper
> 
> I've not been following XDP too much, but xdp_xmit seems pretty
> generic. It would be nice if all drivers used the same statistics
> names. Less user confusion that way. So why is this outside of the
> discussion?

I do want to have this discussion, please.

I had hoped this patchset sparked this that discussion... maybe we can
have it despite this patchset already got applied?

My only request is that, if we don't revert, we fixup the "xdp_tx"
counter name.  It would make it easier for us[1] if we can keep them
applied, as we are preparing (asciinema) demos for [1].

That said, I think it is rather important to standardize on same
statistics names across drivers... which is an assignment that Lorenzo
have already signed up for [2].


[1] https://netdevconf.info/0x14/session.html?tutorial-add-XDP-support-to-a-NIC-driver
[2] https://github.com/xdp-project/xdp-project/blob/master/planning.org#consistency-for-statistics-with-xdp
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer