netdev - Re: [PATCH bpf-next] cpumap: bulk skb using netif_receive_skb

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: <YG7Wi/vFK+XFBUcQ@lore-desk>
Date:   Thu, 8 Apr 2021 12:10:19 +0200
From:   Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To:     Song Liu <song@...nel.org>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Subject: Re: [PATCH bpf-next] cpumap: bulk skb using netif_receive_skb_list

> On Thu, Apr 1, 2021 at 9:49 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> >
> > > On Thu, Apr 1, 2021 at 1:57 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> > > >
> >
> > [...]
> >
> > > > -                       /* Inject into network stack */
> > > > -                       ret = netif_receive_skb_core(skb);
> > > > -                       if (ret == NET_RX_DROP)
> > > > -                               drops++;
> > >
> > > I guess we stop tracking "drops" with this patch?
> > >
> > > Thanks,
> > > Song
> >
> > Hi Song,
> >
> > we do not report the packets dropped by the stack but we still count the drops
> > in the cpumap. If you think they are really important I guess we can change
> > return value of netif_receive_skb_list returning the dropped packets or
> > similar. What do you think?
> 
> I think we shouldn't silently change the behavior of the tracepoint below:
> 
> trace_xdp_cpumap_kthread(rcpu->map_id, n, drops, sched, &stats);
> 
> Returning dropped packets from netif_receive_skb_list() sounds good to me.

Hi Song,

I reviewed the netif_receive_skb_list() and I guess the code needed to count
number of dropped frames is a bit intrusive and we need to add some checks
in the hot path.
Moreover the dropped frames are already accounted in the networking stack
(e.g. mib counters for the ip traffic).
Since drop counter is just exported in a tracepoint in cpu_map_kthread_run,
I guess we can just not count dropped packets in the networking stack here
and rely on the mib counters. What do you think?

@Jesper: since you added the original code, what do you think about it?

Regards,
Lorenzo

> 
> Thanks,
> Song
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)