netdev - Re: consistency for statistics with XDP mode

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: <20181201122252.54833841@redhat.com>
Date:   Sat, 1 Dec 2018 12:22:52 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "mst@...hat.com" <mst@...hat.com>, "toke@...e.dk" <toke@...e.dk>,
        "dsahern@...il.com" <dsahern@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "pstaszewski@...are.pl" <pstaszewski@...are.pl>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "jasowang@...hat.com" <jasowang@...hat.com>, brouer@...hat.com
Subject: Re: consistency for statistics with XDP mode

On Fri, 30 Nov 2018 23:54:10 +0000
Saeed Mahameed <saeedm@...lanox.com> wrote:

> On Fri, 2018-11-30 at 15:30 -0500, Michael S. Tsirkin wrote:
> > On Fri, Nov 30, 2018 at 08:10:58PM +0000, Saeed Mahameed wrote:  
> > > On Thu, 2018-11-22 at 18:00 +0100, Toke Høiland-Jørgensen wrote:  
> > > > David Ahern <dsahern@...il.com> writes:
> > > >   
> > > > > On 11/22/18 1:26 AM, Toke Høiland-Jørgensen wrote:  
> > > > > > Saeed Mahameed <saeedm@...lanox.com> writes:
> > > > > >   
> > > > > > > > > I'd say it sounds reasonable to include XDP in the
> > > > > > > > > normal
> > > > > > > > > traffic
> > > > > > > > > counters, but having the detailed XDP-specific counters
> > > > > > > > > is
> > > > > > > > > quite
> > > > > > > > > useful
> > > > > > > > > as well... So can't we do both (for all drivers)?
> > > > > > > > >   
> > > > > > > 
> > > > > > > What are you thinking ? 
> > > > > > > reporting XDP_DROP in interface dropped counter ?
> > > > > > > and XDP_TX/REDIRECT in the TX counter ?
> > > > > > > XDP_ABORTED in the  err/drop counter ?
> > > > > > > 
> > > > > > > how about having a special XDP command in the .ndo_bpf that
> > > > > > > would query
> > > > > > > the standardized XDP stats ?  
> > > > > > the XDP-specific stats are useful to have separately as well
> > > > > > :)
> > > > > >   
> > > > > 
> > > > > I would like to see basic packets, bytes, and dropped counters
> > > > > tracked
> > > > > for Rx and Tx via the standard netdev counters for all
> > > > > devices.   
> > > 
> > > The problem of reporting XDP_DROP in the netedev drop counter is
> > > that
> > > they don't fit this counter description : "no space in linux
> > > buffers"
> > > and it will be hard for the user to determine whether these drops
> > > are
> > > coming from XDP or because no buffer is available, which will make
> > > it
> > > impossible to estimate packet rate performance without looking at
> > > ethtool stats.
> > > And reporting XDP_DROP in the netdev rx packets counter is somehow
> > > misleading.. since those packets never made it out of this
> > > driver.. 
> > > 
> > > 
> > > And reporting XDP_DROP in the netdev rx packets counter is somehow
> > > misleading.. since those packets never made it out of this driver..  
> > 
> > I think I agree. XDP needs minimal overhead - if user wants to do
> > counters then user can via maps. And in a sense XDP dropping packet
> > is much like e.g. TCP dropping packet - it is not counted
> > against the driver since it's not driver's fault.  
> 
> So we should count all XDP RX packets as successful rx packets i.e
> netdev->stats.rx_packets++; regardless of the XDP program decision ? 

Yes.

> this implies that XDP_TX packets will be counted twice once in 
> netdev->stats.rx_packets and once in netdev->stats.tx_packets

Yes, because the packet was RX'ed on the interface, and then TX'ed on
the interface.  Users expect to see these packets (ac)counted.

> I think this is the only valid option if we are going to use standard
> netdev stats for XDP use cases.

IMHO XDP_DROP should not be accounted as netdev stats drops, this is a
user installed program like tc/iptables, that can also choose to drop
packets.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer