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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 8 Apr 2021 20:20:41 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     Song Liu <song@...nel.org>, 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>, brouer@...hat.com
Subject: Re: [PATCH bpf-next] cpumap: bulk skb using netif_receive_skb_list

On Thu, 8 Apr 2021 12:10:19 +0200
Lorenzo Bianconi <lorenzo.bianconi@...hat.com> wrote:

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

I'm actually fine with not counting the drops in the tracepoint.

As you say it is already accounted elsewere in MIB counters for the
network stack.  Which is actually better, as having this drop counter
in tracepoint have confused people before (when using xdp_redirect_cpu).
If they instead looked at the MIB counters, it should be easier to
understand why the drop happens.

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

Powered by blists - more mailing lists