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

2019-09-17 Thread William Tu
On Tue, Sep 17, 2019 at 01:41:17PM +0200, Eelco Chaudron wrote:
> Two comments below…
>   
> 
> On 11 Sep 2019, at 19:58, 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.
> >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.
> >
> >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 
> >Signed-off-by: William Tu 
> >---
> >v3:
> >- add warning when user enables it but libbpf not support it
> >- revise documentation
> >
> >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 |  15 -
> > acinclude.m4  |   8 +++
> > lib/netdev-afxdp.c| 101
> >++
> > lib/netdev-linux-private.h|   2 +
> > vswitchd/vswitch.xml  |  13 +
> > 5 files changed, 124 insertions(+), 15 deletions(-)
> >
> >diff --git a/Documentation/intro/install/afxdp.rst
> >b/Documentation/intro/install/afxdp.rst
> >index 820e9d993d8f..545516b2bbec 100644
> >--- a/Documentation/intro/install/afxdp.rst
> >+++ b/Documentation/intro/install/afxdp.rst
> >@@ -176,9 +176,18 @@ 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**: disable by setting to "false", otherwise default
> >+   is "true"
> >+
> >+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**.
> >+The **xdpmode** can be "drv" or "skb"::
> >
> >   ethtool -L enp2s0 combined 1
> >   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
> >diff --git a/acinclude.m4 b/acinclude.m4
> >index f0e38898b17a..df1082c455fc 100644
> >--- a/acinclude.m4
> >+++ b/acinclude.m4
> >@@ -276,6 +276,14 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
> >   [Define to 1 if AF_XDP support is available and enabled.])
> > LIBBPF_LDADD=" -lbpf -lelf"
> > AC_SUBST([LIBBPF_LDADD])
> >+
> >+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
> >+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
> >+[XDP need wakeup support detected in xsk.h.])
> >+], [
> >+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [0],
> >+[XDP need wakeup support not detected in xsk.h.])
> >+  ], [#include ])
> >   fi
> >   AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
> > ])
> >diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> >index e5b058d08a09..4d4c90f91806 100644
> >--- a/lib/netdev-afxdp.c
> >+++ b/lib/netdev-afxdp.c
> >@@ -26,6 +26,7 @@
> > #include 
> > #include 
> > #include 
> >+#include 
> > #include 
> > #include 
> > #include 
> >@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
> > #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char *)base))
> >
> > static struct xsk_socket_info *xsk_configure(int ifindex, int
> >xdp_queue_id,
> >- int mode);
> >+ int mode, bool
> >use_need_wakeup);
> > 

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

2019-09-17 Thread Eelco Chaudron

Two comments below…


On 11 Sep 2019, at 19:58, 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.

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.

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 
Signed-off-by: William Tu 
---
v3:
- add warning when user enables it but libbpf not support it
- revise documentation

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 |  15 -
 acinclude.m4  |   8 +++
 lib/netdev-afxdp.c| 101 
++

 lib/netdev-linux-private.h|   2 +
 vswitchd/vswitch.xml  |  13 +
 5 files changed, 124 insertions(+), 15 deletions(-)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst

index 820e9d993d8f..545516b2bbec 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -176,9 +176,18 @@ 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**: disable by setting to "false", otherwise 
default

+   is "true"
+
+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**.

+The **xdpmode** can be "drv" or "skb"::

   ethtool -L enp2s0 combined 1
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
diff --git a/acinclude.m4 b/acinclude.m4
index f0e38898b17a..df1082c455fc 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -276,6 +276,14 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
   [Define to 1 if AF_XDP support is available and 
enabled.])

 LIBBPF_LDADD=" -lbpf -lelf"
 AC_SUBST([LIBBPF_LDADD])
+
+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
+[XDP need wakeup support detected in xsk.h.])
+], [
+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [0],
+[XDP need wakeup support not detected in xsk.h.])
+  ], [#include ])
   fi
   AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
 ])
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index e5b058d08a09..4d4c90f91806 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
 #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char 
*)base))


 static struct xsk_socket_info *xsk_configure(int ifindex, int 
xdp_queue_id,

- int mode);
+ int mode, bool 
use_need_wakeup);

 static void xsk_remove_xdp_program(uint32_t ifindex, int xdpmode);
 static void xsk_destroy(struct xsk_socket_info *xsk);
 static int xsk_configure_all(struct netdev *netdev);
@@ -117,6 +118,49 @@ struct xsk_socket_info {
 atomic_uint64_t tx_dropped;
 };

+#ifdef HAVE_XDP_NEED_WAKEUP
+static inline void
+xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
+

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

2019-09-11 Thread William Tu
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.
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.

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 
Signed-off-by: William Tu 
---
v3:
- add warning when user enables it but libbpf not support it
- revise documentation

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 |  15 -
 acinclude.m4  |   8 +++
 lib/netdev-afxdp.c| 101 ++
 lib/netdev-linux-private.h|   2 +
 vswitchd/vswitch.xml  |  13 +
 5 files changed, 124 insertions(+), 15 deletions(-)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst
index 820e9d993d8f..545516b2bbec 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -176,9 +176,18 @@ 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**: disable by setting to "false", otherwise default
+   is "true"
+
+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**.
+The **xdpmode** can be "drv" or "skb"::
 
   ethtool -L enp2s0 combined 1
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
diff --git a/acinclude.m4 b/acinclude.m4
index f0e38898b17a..df1082c455fc 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -276,6 +276,14 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
   [Define to 1 if AF_XDP support is available and enabled.])
 LIBBPF_LDADD=" -lbpf -lelf"
 AC_SUBST([LIBBPF_LDADD])
+
+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
+[XDP need wakeup support detected in xsk.h.])
+], [
+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [0],
+[XDP need wakeup support not detected in xsk.h.])
+  ], [#include ])
   fi
   AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
 ])
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index e5b058d08a09..4d4c90f91806 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
 #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char *)base))
 
 static struct xsk_socket_info *xsk_configure(int ifindex, int xdp_queue_id,
- int mode);
+ int mode, bool use_need_wakeup);
 static void xsk_remove_xdp_program(uint32_t ifindex, int xdpmode);
 static void xsk_destroy(struct xsk_socket_info *xsk);
 static int xsk_configure_all(struct netdev *netdev);
@@ -117,6 +118,49 @@ struct xsk_socket_info {
 atomic_uint64_t tx_dropped;
 };
 
+#ifdef HAVE_XDP_NEED_WAKEUP
+static inline void
+xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
+struct netdev *netdev, int fd)
+{
+struct pollfd pfd;
+int ret;
+
+if