Message-ID: <875znt3pxu.fsf@toke.dk>
Date:   Tue, 23 Jul 2019 14:17:49 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, nhorman@...driver.com,
        dsahern@...il.com, roopa@...ulusnetworks.com,
        nikolay@...ulusnetworks.com, jakub.kicinski@...ronome.com,
        andy@...yhouse.net, f.fainelli@...il.com, andrew@...n.ch,
        vivien.didelot@...il.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [RFC PATCH net-next 00/12] drop_monitor: Capture dropped packets and metadata

Ido Schimmel <idosch@...sch.org> writes:

> On Mon, Jul 22, 2019 at 09:43:15PM +0200, Toke Høiland-Jørgensen wrote:
>> Is there a mechanism for the user to filter the packets before they are
>> sent to userspace? A bpf filter would be the obvious choice I guess...
>
> Hi Toke,
>
> Yes, it's on my TODO list to write an eBPF program that only lets
> "unique" packets to be enqueued on the netlink socket. Where "unique" is
> defined as {5-tuple, PC}. The rest of the copies will be counted in an
> eBPF map, which is just a hash table keyed by {5-tuple, PC}.

Yeah, that's a good idea. Or even something simpler like tcpdump-style
filters for the packets returned by drop monitor (say if I'm just trying
to figure out what happens to my HTTP requests).

> I think it would be good to have the program as part of the bcc
> repository [1]. What do you think?

Sure. We could also add it to the XDP tutorial[2]; it could go into a
section on introspection and debugging (just added a TODO about that[3]).

>> For integrating with XDP the trick would be to find a way to do it that
>> doesn't incur any overhead when it's not enabled. Are you envisioning
>> that this would be enabled separately for the different "modes" (kernel,
>> hardware, XDP, etc)?
>
> Yes. Drop monitor have commands to enable and disable tracing, but they
> don't carry any attributes at the moment. My plan is to add an attribute
> (e.g., 'NET_DM_ATTR_DROP_TYPE') that will specify the type of drops
> you're interested in - SW/HW/XDP. If the attribute is not specified,
> then current behavior is maintained and all the drop types are traced.
> But if you're only interested in SW drops, then overhead for the rest
> should be zero.

Makes sense (although "should be" is the key here ;)).

I'm also worried about the drop monitor getting overwhelmed; if you turn
it on for XDP and you're running a filtering program there, you'll
suddenly get *a lot* of drops.

As I read your patch, the current code can basically queue up an
unbounded number of packets waiting to go out over netlink, can't it?

> For HW drops I'm going to have devlink call into drop monitor. The
> function call will just be a NOP in case user is not interested in HW
> drops. I'm not sure if for XDP you want to register a probe on a
> tracepoint or call into drop monitor. If you want to use the former,
> then you can just have drop monitor unregister its probe from the
> tracepoint, which is what drop monitor is currently doing with the
> kfree_skb() tracepoint.

I guess we would use whichever has the least overhead. We do already
have tracepoints in the XDP path, but only on the error path. We
certainly don't want the overhead of a function call in the XDP fast
path when this feature is turned off (even if that function does
nothing), but maybe the NOP-patching for a tracepoint is acceptable.
We'll have to benchmark it :)

-Toke

[2] https://github.com/xdp-project/xdp-tutorial

[3] https://github.com/xdp-project/xdp-project/commit/7637702087a6b9d94816092a054d77c479df808a

Powered by blists - more mailing lists