On Wed, Sep 4, 2019 at 7:27 AM Ilya Maximets <i.maxim...@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.maxim...@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.karls...@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. OK Thanks William _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev