Linux Virtualization - Jun 2019 - Stats for XDP actions

David Ahern <dsahern at gmail.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 at gmail.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

On 4/18/19 8:24 AM, Toke H?iland-J?rgensen wrote:
>>>
>>
>> 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? :)
> 
Not yet. Almost done with the nexthop changes. Once that is out of the
way I can come back to this.

David Ahern <dsahern at gmail.com> writes:
> On 4/18/19 8:24 AM, Toke H?iland-J?rgensen wrote:
>>>>
>>>
>>> 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? :)
>> 
>
> Not yet. Almost done with the nexthop changes. Once that is out of the
> way I can come back to this.
Ping? :)

-Toke