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

[William Tu]

Hi Eelco,

Thanks for the review.

On Tue, Sep 10, 2019 at 7:24 AM Eelco Chaudron <echaudro at redhat.com> wrote:
>
> Hi William,
>
> One comment below, but I’m also wondering if we should warn/log a
> message if OVS is compiled without this feature and we explicitly
> configure it? Same for the reverse what if the kernel does not support
> this option, will it fail?
>
yes, it's a little confusing now. Considering the following cases

1) kernel support && libbpf support && user enable
--> works

2) kernel support && libbpf NOT support && user enable
--> works, the need_wakeup is no ops, I will add warning

3) kernel NOT support && libbpf support && user enable
--> fails, the AF_XDP packet rx/tx sees no packets (libbpf is not
backward compatible?)

4) kernel NOT && libbpf NOT support && user enable
--> works, the need_wakeup is no ops, I will add warning

will address the above cases in next version.

>
> //Eelco
>
>
> On 5 Sep 2019, at 22:51, 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 5.3.0-rc1 and OVS
> > enables it by default. Running the feature before this version causes
> > xsk bind fails, please use options:use_need_wakeup=false to disable
> > it.
> >
> > 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.
> >
> > I tested on kernel 5.3.0-rc3 using its libbpf.  On Intel Xeon E5-2620
> > v3 2.4GHz system, performance of physical port to physical port
> > improves
> > from 6.1Mpps to 7.3Mpps. Testing on 5.2.0-rc6 using libbpf from
> > 5.3.0-rc3
> > does not work due to libbpf API change. Users have to use the older
> > libbpf for older kernel.
> >
> > Suggested-by: Ilya Maximets <i.maximets at samsung.com>
> > Signed-off-by: William Tu <u9012063 at gmail.com>
> > ---
> > v2:
> > - address feedbacks from Ilya and Eelco
> > - add options:use_need_wakeup, default to true
> > - remove poll timeout=1sec, make poll() return immediately
> > - naming change: rename to xsk_rx_wakeup_if_needing
> > - fix indents and return value for errno
> > ---
> >  Documentation/intro/install/afxdp.rst | 14 ++++--
> >  acinclude.m4                          |  5 ++
> >  lib/netdev-afxdp.c                    | 94
> > +++++++++++++++++++++++++++++++----
> >  lib/netdev-linux-private.h            |  2 +
> >  vswitchd/vswitch.xml                  | 13 +++++
> >  5 files changed, 114 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/intro/install/afxdp.rst
> > b/Documentation/intro/install/afxdp.rst
> > index 820e9d993d8f..b7a3d0b7f57b 100644
> > --- a/Documentation/intro/install/afxdp.rst
> > +++ b/Documentation/intro/install/afxdp.rst
> > @@ -176,9 +176,17 @@ in :doc:`general`::
> >    ovs-vswitchd ...
> >    ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
> >
> > -Make sure your device driver support AF_XDP, and to use 1 PMD (on
> > core 4)
> > -on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask,
> > -pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or
> > "skb"::
> > +Make sure your device driver support AF_XDP, netdev-afxdp supports
> > +the following additional options (see man ovs-vswitchd.conf.db for
> > +more details):
> > +
> > + * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
> > +
> > + * **use_need_wakeup**: enable by setting to "true", otherwise
> > "false"
>
> Bit confusing here, as the default is true?

Yes, thanks. I will fix it in next version.

Regards,
William

<snip>