On Mon, Oct 28, 2019 at 12:11 PM Ilya Maximets <i.maxim...@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.maxim...@ovn.org> > >> Signed-off-by: William Tu <u9012...@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 <echau...@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 > diff --git a/Documentation/intro/install/afxdp.rst > b/Documentation/intro/install/afxdp.rst > index 202b6cdf7..fa898912b 100644 > --- a/Documentation/intro/install/afxdp.rst > +++ b/Documentation/intro/install/afxdp.rst > @@ -182,7 +182,7 @@ more details): > > * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode. > > - * **use_need_wakeup**: default "true" if libbpf supports it, otherwise > false. > + * **use-need-wakeup**: default "true" if libbpf supports it, otherwise > false. > > For example, 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**. > diff --git a/NEWS b/NEWS > index 330ab3832..4d8092d62 100644 > --- a/NEWS > +++ b/NEWS > @@ -5,6 +5,9 @@ Post-v2.12.0 > separate project. You can find it at > https://github.com/ovn-org/ovn.git > - Userspace datapath: > + * New option 'use-need-wakeup' for netdev-afxdp to control enabling > + of corresponding 'need_wakup' flag in AF_XDP rings. Enabled by > default > + if supported by libbpf. > * Add option to enable, disable and query TCP sequence checking in > conntrack. > > diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c > index b6b03e39b..270a5ab0c 100644 > --- a/lib/netdev-afxdp.c > +++ b/lib/netdev-afxdp.c > @@ -330,7 +330,7 @@ xsk_configure_socket(struct xsk_umem_info *umem, uint32_t > ifindex, > &xsk->rx, &xsk->tx, &cfg); > if (ret) { > VLOG_ERR("xsk_socket__create failed (%s) mode: %s " > - "use_need_wakeup: %s qid: %d", > + "use-need-wakeup: %s qid: %d", > ovs_strerror(errno), > xdpmode == XDP_COPY ? "SKB": "DRV", > use_need_wakeup ? "true" : "false", > @@ -431,7 +431,7 @@ xsk_configure_all(struct netdev *netdev) > > /* Configure each queue. */ > for (i = 0; i < n_rxq; i++) { > - VLOG_DBG("%s: configure queue %d mode %s use_need_wakeup %s.", > + VLOG_DBG("%s: configure queue %d mode %s use-need-wakeup %s.", > netdev_get_name(netdev), i, > dev->xdpmode == XDP_COPY ? "SKB" : "DRV", > dev->use_need_wakeup ? "true" : "false"); > @@ -551,7 +551,7 @@ netdev_afxdp_set_config(struct netdev *netdev, const > struct smap *args, > return EINVAL; > } > > - need_wakeup = smap_get_bool(args, "use_need_wakeup", > NEED_WAKEUP_DEFAULT); > + need_wakeup = smap_get_bool(args, "use-need-wakeup", > NEED_WAKEUP_DEFAULT); > #ifndef HAVE_XDP_NEED_WAKEUP > if (need_wakeup) { > VLOG_WARN("XDP need_wakeup is not supported in libbpf."); > @@ -580,7 +580,7 @@ netdev_afxdp_get_config(const struct netdev *netdev, > struct smap *args) > smap_add_format(args, "n_rxq", "%d", netdev->n_rxq); > smap_add_format(args, "xdpmode", "%s", > dev->xdpmode == XDP_ZEROCOPY ? "drv" : "skb"); > - smap_add_format(args, "use_need_wakeup", "%s", > + smap_add_format(args, "use-need-wakeup", "%s", > dev->use_need_wakeup ? "true" : "false"); > ovs_mutex_unlock(&dev->mutex); > return 0; > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index decccfe84..00c6bd2d4 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -3122,7 +3122,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > type=patch options:peer=p1 \ > </p> > </column> > > - <column name="options" key="use_need_wakeup" > + <column name="options" key="use-need-wakeup" > type='{"type": "boolean"}'> > <p> > Specifies whether to use need_wakeup feature in afxdp netdev. > --- > > Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev