[Intel-wired-lan] [RFC PATCH bpf-next 00/12] AF_XDP, zero-copy support

Björn Töpel bjorn.topel at gmail.com
Wed May 16 17:49:22 UTC 2018


2018-05-16 19:04 GMT+02:00 Alexei Starovoitov <alexei.starovoitov at gmail.com>:
> 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.
>

Good point. We did realize it was an UAPI break, and intended to add
the "disable rebind" as a follow up in this merge window (honestly!
;-)), but still -- this proves your point that the ZC patches should
be done back-to-back to the non-ZC ones.

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

Well it *could*, but that combined with the whole "bolted on Rx path"
isn't something I'd like to be upstream. It needs more work, and is
too messy and fragile IMO.

>> * 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.
>

Hmm...

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

Yes.

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

Again, fair point. We think the copy-path is generic enough (with the
follow-ups you and Daniel suggested and the rebind state removed) --
but hey, we're that one vendor. ;-) More seriously -- having at least
two ZC implementations at the introduction of AF_XDP would make us
happier as well.


Thanks,
Björn


More information about the Intel-wired-lan mailing list