netdev - Stats for XDP actions (was: Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp

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 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]

Date:   Thu, 18 Apr 2019 15:24:45 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     David Ahern <dsahern@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     David Miller <davem@...emloft.net>, mst@...hat.com,
        makita.toshiaki@....ntt.co.jp, jasowang@...hat.com,
        netdev@...r.kernel.org, virtualization@...ts.linux-foundation.org,
        hawk@...nel.org, Jakub Kicinski <jakub.kicinski@...ronome.com>,
        John Fastabend <john.fastabend@...il.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Stats for XDP actions (was: Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames)

David Ahern <dsahern@...il.com> writes:

> On 2/4/19 3:53 AM, Jesper Dangaard Brouer wrote:
>> On Sat, 2 Feb 2019 14:27:26 -0700
>> David Ahern <dsahern@...il.com> wrote:
>> 
>>> On 1/31/19 1:15 PM, Jesper Dangaard Brouer wrote:
>>>>>
>>>>> David, Jesper, care to chime in where we ended up in that last thread
>>>>> discussion this?  
>>>>
>>>> IHMO packets RX and TX on a device need to be accounted, in standard
>>>> counters, regardless of XDP.  For XDP RX the packet is counted as RX,
>>>> regardless if XDP choose to XDP_DROP.  On XDP TX which is via
>>>> XDP_REDIRECT or XDP_TX, the driver that transmit the packet need to
>>>> account the packet in a TX counter (this if often delayed to DMA TX
>>>> completion handling).  We cannot break the expectation that RX and TX
>>>> counter are visible to userspace stats tools. XDP should not make these
>>>> packets invisible.  
>>>
>>> Agreed. What I was pushing on that last thread was Rx, Tx and dropped
>>> are all accounted by the driver in standard stats. Basically if the
>>> driver touched it, the driver's counters should indicate that.
>> 
>> Sound like we all agree (except with the dropped counter, see below).
>> 
>> Do notice that mlx5 driver doesn't do this.  It is actually rather
>> confusing to use XDP on mlx5, as when XDP "consume" which include
>> XDP_DROP, XDP_REDIRECT or XDP_TX, then the driver standard stats are
>> not incremented... the packet is invisible to "ifconfig" stat based
>> tools.
>
> mlx5 needs some work. As I recall it still has the bug/panic removing
> xdp programs - at least I don't recall seeing a patch for it.
>
>> 
>> 
>>> The push back was on dropped packets and whether that counter should be
>>> bumped on XDP_DROP.
>> 
>> My opinion is the XDP_DROP action should NOT increment the drivers drop
>> counter.  First of all the "dropped" counter is also use for other
>> stuff, which will confuse that this counter express.  Second, choosing
>> XDP_DROP is a policy choice, it still means it was RX-ed at the driver
>> level.
>> 
>
> Understood. Hopefully in March I will get some time to come back to this
> and propose an idea on what I would like to see - namely, the admin has
> a config option at load time to enable driver counters versus custom map
> counters. (meaning the operator of the node chooses standard stats over
> strict performance.) But of course that means the drivers have the code
> to collect those stats.

Hi David

I don't recall seeing any follow-up on this. Did you have a chance to
formulate your ideas? :)

-Toke