[ovs-dev] [PATCHv6] netdev-afxdp: Add need_wakeup supprt.

Ilya Maximets i.maximets at ovn.org
Tue Oct 29 19:41:15 UTC 2019


On 29.10.2019 0:32, William Tu wrote:
> On Mon, Oct 28, 2019 at 12:11 PM Ilya Maximets <i.maximets at ovn.org> wrote:
>>
>> On 28.10.2019 11:46, Eelco Chaudron wrote:
>>>
>>>
>>> On 23 Oct 2019, at 23:06, William Tu wrote:
>>>
>>>> The patch adds support for using need_wakeup flag in AF_XDP rings.
>>>> A new option, use_need_wakeup, is added.  When this option is used,
>>>> it means that OVS has to explicitly wake up the kernel RX, using poll()
>>>> syscall and wake up TX, using sendto() syscall. This feature improves
>>>> the performance by avoiding unnecessary sendto syscalls for TX.
>>>> For RX, instead of kernel always busy-spinning on fille queue, OVS wakes
>>>> up the kernel RX processing when fill queue is replenished.
>>>>
>>>> The need_wakeup feature is merged into Linux kernel bpf-next tee with commit
>>>> 77cd0d7b3f25 ("xsk: add support for need_wakeup flag in AF_XDP rings") and
>>>> OVS enables it by default, if libbpf supports it.  If users enable it but
>>>> runs in an older version of libbpf, then the need_wakeup feature has no effect,
>>>> and a warning message is logged.
>>>>
>>>> For virtual interface, it's better set use_need_wakeup=false, since
>>>> the virtual device's AF_XDP xmit is synchronous: the sendto syscall
>>>> enters kernel and process the TX packet on tx queue directly.
>>>>
>>>> On Intel Xeon E5-2620 v3 2.4GHz system, performance of physical port
>>>> to physical port improves from 6.1Mpps to 7.3Mpps.
>>>>
>>>> Suggested-by: Ilya Maximets <i.maximets at ovn.org>
>>>> Signed-off-by: William Tu <u9012063 at gmail.com>
>>>
>>> Reviewed based on diff from previous version, also did quick compile test with and without need_wakeup supported kernel.
>>> No actual performance tests ran, as my setup is in a messed up state :(
>>>
>>> One small issue (guess can be fixed at apply time), the subject mentions “supprt”, it’s missing an o.
>>>
>>> Acked-by: Eelco Chaudron <echaudro at redhat.com>
>>>
>>
>> Thanks, I could fix the 'supprt' while applying the patch.
>>
>> But I have one more question.
>> William, Eelco, what do you think about renaming the option itself from
>> "options:use_need_wakeup" to "options:use-need-wakeup"?
>> Most of the netdev options for netdev-dpdk and netdev-linux except few of them
>> (e.g. n_rxq, n_rxq_desc) uses dashes in their names instead of underscores.
>> I agree that right now there is a mess in OVS and there is no specified convention
>> for options naming, but it might be good idea to name all the new options in a
>> similar way, to keep the API more or less consistent.  What do you think?
>>
>> I could squash in below diff while applying the patch (I also added a NEWS entry):
>>
> Hi Ilya,
> 
> Yes, I'm ok with using dashes.
> diff below looks good to me. Thanks
> William

Thanks, William and Eelco!
I fixed a typo in the subject line, squashed the diff and applied to master.

Best regards, Ilya Maximets.


More information about the dev mailing list