netdev - Re: [PATCH 4/6 bfp-next] Simplify AF_XDP umem allocation path for Intel drivers.

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: <20190628134847.74bea2fb@cakuba.netronome.com>
Date:   Fri, 28 Jun 2019 13:48:47 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     "Jonathan Lemon" <jonathan.lemon@...il.com>
Cc:     netdev@...r.kernel.org, bjorn.topel@...el.com,
        magnus.karlsson@...el.com, saeedm@...lanox.com,
        maximmi@...lanox.com, brouer@...hat.com, kernel-team@...com
Subject: Re: [PATCH 4/6 bfp-next] Simplify AF_XDP umem allocation path for
 Intel drivers.

On Thu, 27 Jun 2019 19:36:42 -0700, Jonathan Lemon wrote:
> On 27 Jun 2019, at 15:42, Jakub Kicinski wrote:
> > Could you be more explicit on the motivation?  I'd call this patch set
> > "make all drivers use reuse queue" rather than "clean up".  
> 
> The motivation is to have packets which were received on a zero-copy
> AF_XDP socket, and which returned a TX verdict from the bpf program,
> queued directly on the TX ring (if they're in the same napi context).
> 
> When these TX packets are completed, they are placed back onto the
> reuse queue, as there isn't really any other place to handle them.
> 
> It also addresses Maxim's concern about having buffers end up sitting
> on the rq after a ring resize.
> 
> I was going to send the TX change out as part of this patch, but
> figured it would be better split unto its own series.

Makes sense, please put that info in the cover letter.  (in other nit
picks please prefix the patches which the name of the subsystem they
are touching - net: xsk: ?, and make sure all changes have a commit
message).

Looking at this yesterday I think I've seen you didn't change the RQ
sizing -  for the TX side to work the rq size must be RX ring size
+ XDP TX ring size (IIRC that was the worst case number of packets
which we may need to park during reconfiguration).  

Maybe you did do that and I missed it.

> > Also when you're changing code please make sure you CC the author.  
> 
> Who did I miss?

commit f5bd91388e26557f64ca999e0006038c7a919308
Author: Jakub Kicinski <jakub.kicinski@...ronome.com>
Date:   Fri Sep 7 10:18:46 2018 +0200

    net: xsk: add a simple buffer reuse queue

    XSK UMEM is strongly single producer single consumer so reuse of
    frames is challenging.  Add a simple "stash" of FILL packets to
    reuse for drivers to optionally make use of.  This is useful
    when driver has to free (ndo_stop) or resize a ring with an active
    AF_XDP ZC socket.
    
    Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
    Tested-by: Andrew Bowers <andrewx.bowers@...el.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>


Sorry to get upset, I missed a bpf patch recently and had to do 
a revert..