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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 8 Sep 2020 13:32:01 +0300
From:   Maxim Mikityanskiy <maximmi@...dia.com>
To:     Björn Töpel <bjorn.topel@...el.com>,
        Björn Töpel <bjorn.topel@...il.com>,
        <ast@...nel.org>, <daniel@...earbox.net>
CC:     <netdev@...r.kernel.org>, <bpf@...r.kernel.org>,
        Maxim Mikityanskiy <maximmi@...lanox.com>,
        <magnus.karlsson@...el.com>, <davem@...emloft.net>,
        <kuba@...nel.org>, <hawk@...nel.org>, <john.fastabend@...il.com>,
        <intel-wired-lan@...ts.osuosl.org>
Subject: Re: [PATCH bpf-next 0/6] xsk: exit NAPI loop when AF_XDP Rx ring is
 full

On 2020-09-04 16:59, Björn Töpel wrote:
> On 2020-09-04 15:53, Björn Töpel wrote:
>> This series addresses a problem that arises when AF_XDP zero-copy is 
>> enabled, and the kernel softirq Rx processing and userland process
>> is running on the same core.
>>
> [...]
>>
> 
> @Maxim I'm not well versed in Mellanox drivers. Would this be relevant 
> to mlx5 as well?

Thanks for letting me know about this series! So the basic idea is to 
stop processing hardware completions if the RX ring gets full, because 
the application didn't have chance to run? Yes, I think it's also 
relevant to mlx5, the issue is not driver-specific, and a similar fix is 
applicable. However, it may lead to completion queue overflows - some 
analysis is needed to understand what happens then and how to handle it.

Regarding the feature, I think it should be opt-in (disabled by 
default), because old applications may not wakeup RX after they process 
packets in the RX ring. Is it required to change xdpsock accordingly? 
Also, when need_wakeup is disabled, your driver implementation seems to 
quit NAPI anyway, but it shouldn't happen, because no one will send a 
wakeup.

Waiting until the RX ring fills up, then passing control to the 
application and waiting until the hardware completion queue fills up, 
and so on increases latency - the busy polling approach sounds more 
legit here.

The behavior may be different depending on the driver implementation:

1. If you arm the completion queue and leave interrupts enabled on early 
exit too, the application will soon be interrupted anyway and won't have 
much time to process many packets, leading to app <-> NAPI ping-pong one 
packet at a time, making NAPI inefficient.

2. If you don't arm the completion queue on early exit and wait for the 
explicit wakeup from the application, it will easily overflow the 
hardware completion queue, because we don't have a symmetric mechanism 
to stop the application on imminent hardware queue overflow. It doesn't 
feel correct and may be trickier to handle: if the application is too 
slow, such drops should happen on driver/kernel level, not in hardware.

Which behavior is used in your drivers? Or am I missing some more options?

BTW, it should be better to pass control to the application before the 
first dropped packet, not after it has been dropped.

Some workloads different from pure AF_XDP, for example, 50/50 AF_XDP and 
XDP_TX may suffer from such behavior, so it's another point to make a 
knob on the application layer to enable/disable it.

 From the driver API perspective, I would prefer to see a simpler API if 
possible. The current API exposes things that the driver shouldn't know 
(BPF map type), and requires XSK-specific handling. It would be better 
if some specific error code returned from xdp_do_redirect was reserved 
to mean "exit NAPI early if you support it". This way we wouldn't need 
two new helpers, two xdp_do_redirect functions, and this approach would 
be extensible to other non-XSK use cases without further changes in the 
driver, and also the logic to opt-in the feature could be put inside the 
kernel.

Thanks,
Max

> 
> Cheers,
> Björn

Powered by blists - more mailing lists