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
