netdev - Re: [RFC PATCH bpf-next 00/12] AF

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]

Date:   Wed, 16 May 2018 10:04:29 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     magnus.karlsson@...il.com, magnus.karlsson@...el.com,
        alexander.h.duyck@...el.com, alexander.duyck@...il.com,
        john.fastabend@...il.com, ast@...com, brouer@...hat.com,
        willemdebruijn.kernel@...il.com, daniel@...earbox.net,
        mst@...hat.com, netdev@...r.kernel.org,
        Björn Töpel <bjorn.topel@...el.com>,
        michael.lundkvist@...csson.com, jesse.brandeburg@...el.com,
        anjali.singhai@...el.com, qi.z.zhang@...el.com,
        intel-wired-lan@...ts.osuosl.org
Subject: Re: [RFC PATCH bpf-next 00/12] AF_XDP, zero-copy support

On Tue, May 15, 2018 at 09:06:03PM +0200, Björn Töpel wrote:
> 
> Alexei had two concerns in conjunction with adding ZC support to
> AF_XDP: show that the user interface holds and can deliver good
> performance for ZC and that the driver interfaces for ZC are good. We
> think that this patch set shows that we have addressed the first
> issue: performance is good and there is no change to the uapi. But
> please take a look at the code and see if you like the ZC interfaces
> that was the second concern.

Looks like we're not on the same page with definition of 'uapi'.
Here you're saying that patches demonstrate performance without
a change to uapi, whereas patch 1 does remove rebind support
which _is_ a change to uapi.
That was exactly my concern with the previous set.

The other restrictions that are introduced in this patch set
are actually ok:
- like in patch 12: 'no redirect to an AF_XDP enabled XDP Tx ring'
  this is fine, since this restriction can be lifted later without
  breaking uapi
- patch 11: 'No passing to the stack via XDP_PASS'
  also fine, since can be addressed later.

> To do for this RFC to become a patch set:
> 
> * Implement dynamic creation and deletion of queues in the i40e driver

can be deferred, no?

> * Properly splitting up the i40e changes

Imo patch 11 and 12 are reasonable in terms of size
and reviewable as-is. I don't think they have to be split.
Would be nice though.

> * Have the Intel NIC team review the i40e changes from at least an
>   architecture point of view

As Alexander pointed out in patch 11, if you split it into
separate file the changes to i40e core become pretty small and
I think Intel folks (Jeff, Alexander, ...) will be ok if we merge
this set via bpf-next tree asap and clean up, refactor, share
more code with i40e core later.

> * Implement a more fair scheduling policy for multiple XSKs that share
>   an umem for TX. This can be combined with a batching API for
>   xsk_umem_consume_tx.

can be deferred too?

I think the first 10 patches in this set is a hard dependency on i40e
patches, so the whole thing have to reviewed and landed together.
May be the first 5 patches can be applied already.

Anyway at this point I still think that removing AF_XDP and bpf xskmap
from uapi is necessary before the merge window, unless this patch set
(including i40e changes can land right now).
Also I'd like to see another NIC vendor demonstrating RFC for ZC as well.
The allocator api looks good and I don't anticipate issues, but still
I think it's necessary to make sure that we're not adding i40e-only feature.