[ovs-dev] [PATCH 2/2] netdev-afxdp: Add need_wakeup supprt.
u9012063 at gmail.com
Wed Sep 4 18:23:39 UTC 2019
On Wed, Sep 4, 2019 at 7:27 AM Ilya Maximets <i.maximets at samsung.com> wrote:
> On 04.09.2019 17:04, William Tu wrote:
> > Hi Ilya,
> > Thanks for the feedback.
> > On Wed, Sep 4, 2019 at 5:19 AM Ilya Maximets <i.maximets at samsung.com> wrote:
> >> Hi William,
> >> Thanks for the patch!
> >> One general comment is that we, probably, should make this feature
> >> configurable. There are 2 reasons:
> >> 1. We might want to run OVS on older kernel while building with more
> >> recent libbpf. In this case socket creation will fail due to
> >> unsupported flag and we'll not be able to make it work.
> > I think it will also work, it falls back to old behavior.
> > From:
> > commit 77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10
> > Author: Magnus Karlsson <magnus.karlsson at intel.com>
> > Date: Wed Aug 14 09:27:17 2019 +0200
> > "
> > This flag needs some simple driver support. If the driver does not
> > support this, the Rx flag is always zero and the Tx flag is always
> > one. This makes any application relying on this feature default to the
> > old behaviour of not requiring any syscalls in the Rx path and always
> > having to call sendto() in the Tx path.
> > "
> This part is about relation between xdp subsystem and the device driver.
> If device driver doesn't support this feature, xdp subsystem will handle
> this by always exposing need_wakeup flag on rings.
> However, if bind_flags contains XDP_USE_NEED_WAKEUP, but xdp subsystem
> doesn't support it, userspace/libbpf will get failure on socket creation.
> To be honest, I didn't test that, but I think that it works this way.
> You may try building OVS with libbpf from bpf-next and run it on master
> kernel (without need_wakeup support).
Yes, you're right, it will fail at xsk_bind when checking
if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY) )
> >> 2. Performance impact is not always positive. It depends on the
> >> number of available CPU cores, port types (phisical or virtual),
> >> interrupts affinity. And this was proved by test results from
> >> Eelco. So, it might be good to have control over the enabling/
> >> disabling the feature.
> >> However, I think that it's better to keep it enabled by default.
> > OK, I will make it enabled by default, and add another option.
> > "options:need_wakeup=false" for user to disable it.
> This might be better to call it 'options:use_need_wakeup' because
> 'need_wakeup=false' sounds like we don't need to ever wake it up,
> but it's opposite in practice.
More information about the dev