[ovs-dev] [PATCH] dpdk: add support for v2.1.0

Daniele Di Proietto diproiettod at vmware.com
Thu Aug 27 18:01:17 UTC 2015


The patch looks good, thanks.

Two comments below

On 26/08/2015 13:44, "Timo Puha" <timox.puha at intel.com> wrote:

>Update relevant artifacts to add support for DPDK v2.1.0
> - INSTALL.DPDK.md
> - acinclude.m4: Change DPDK library name
> - netdev-dpdk: Add 16 bytes extra padding to mbuf size to adapt to DPDK
>bug
>   fix that changes the treatment of the requested mbuf size
> - build.sh: Change DPDK version number
>
>Note that this breaks compatibility with DPDK v2.0.0 although only
>for the library name change.
>
>Note that throughput for vhost ports with mergeable buffers is reduced
>about 10% due to a necessary bug fix in DPDK vhost code.
>
>Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>Signed-off-by: Michal Weglicki <michalx.weglicki at intel.com>
>Signed-off-by: Timo Puha <timox.puha at intel.com>
>---
> .travis/build.sh  |  2 +-
> INSTALL.DPDK.md   | 12 ++++++------
> acinclude.m4      |  2 +-
> lib/netdev-dpdk.c |  3 ++-
> 4 files changed, 10 insertions(+), 9 deletions(-)
>
>diff --git a/.travis/build.sh b/.travis/build.sh
>index e90f4d0..3cadbf0 100755
>--- a/.travis/build.sh
>+++ b/.travis/build.sh
>@@ -71,7 +71,7 @@ fi
> 
> if [ "$DPDK" ]; then
>     if [ -z "$DPDK_VER" ]; then
>-        DPDK_VER="2.0.0"
>+        DPDK_VER="2.1.0"
>     fi
>     install_dpdk $DPDK_VER
>     if [ "$CC" = "clang" ]; then
>diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
>index 35dd9a0..39e1b5d 100644
>--- a/INSTALL.DPDK.md
>+++ b/INSTALL.DPDK.md
>@@ -16,7 +16,7 @@ OVS needs a system with 1GB hugepages support.
> Building and Installing:
> ------------------------
> 
>-Required: DPDK 2.0
>+Required: DPDK 2.1
> Optional (if building with vhost-cuse): `fuse`, `fuse-devel`
>(`libfuse-dev`
> on Debian/Ubuntu)
> 
>@@ -24,7 +24,7 @@ on Debian/Ubuntu)
>   1. Set `$DPDK_DIR`
> 
>      ```
>-     export DPDK_DIR=/usr/src/dpdk-2.0
>+     export DPDK_DIR=/usr/src/dpdk-2.1
>      cd $DPDK_DIR
>      ```

Should we also remove the requirement to change CONFIG_RTE_LIBRTE_VHOST?
It is set by default in DPDK 2.1

> 
>@@ -112,7 +112,7 @@ Using the DPDK with ovs-vswitchd:
>      3. Bind network device to vfio-pci:
>         `$DPDK_DIR/tools/dpdk_nic_bind.py --bind=vfio-pci eth1`
> 
>-3. Mount the hugetable filsystem
>+3. Mount the hugetable filesystem
> 
>    `mount -t hugetlbfs -o pagesize=1G none /dev/hugepages`
> 
>@@ -315,7 +315,7 @@ the vswitchd.
> DPDK vhost:
> -----------
> 
>-DPDK 2.0 supports two types of vhost:
>+DPDK 2.1 supports two types of vhost:
> 
> 1. vhost-user
> 2. vhost-cuse
>@@ -336,7 +336,7 @@ with OVS.
> DPDK vhost-user Prerequisites:
> -------------------------
> 
>-1. DPDK 2.0 with vhost support enabled as documented in the "Building and
>+1. DPDK 2.1 with vhost support enabled as documented in the "Building and
>    Installing section"
> 
> 2. QEMU version v2.1.0+
>@@ -418,7 +418,7 @@ with OVS.
> DPDK vhost-cuse Prerequisites:
> -------------------------
> 
>-1. DPDK 2.0 with vhost support enabled as documented in the "Building and
>+1. DPDK 2.1 with vhost support enabled as documented in the "Building and
>    Installing section"
>    As an additional step, you must enable vhost-cuse in DPDK by setting
>the
>    following additional flag in `config/common_linuxapp`:
>diff --git a/acinclude.m4 b/acinclude.m4
>index 45cfaf6..90bb708 100644
>--- a/acinclude.m4
>+++ b/acinclude.m4
>@@ -172,7 +172,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> 
>     DPDK_INCLUDE=$RTE_SDK/include
>     DPDK_LIB_DIR=$RTE_SDK/lib
>-    DPDK_LIB="-lintel_dpdk"
>+    DPDK_LIB="-ldpdk"
>     DPDK_EXTRA_LIB=""
> 
>     AC_COMPILE_IFELSE(
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 3444bb1..5798a93 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -68,7 +68,8 @@ static struct vlog_rate_limit rl =
>VLOG_RATE_LIMIT_INIT(5, 20);
>  */
> 
> #define MTU_TO_MAX_LEN(mtu)  ((mtu) + ETHER_HDR_LEN + ETHER_CRC_LEN)
>-#define MBUF_SIZE(mtu)       (MTU_TO_MAX_LEN(mtu) + (512) + \
>+#define MBUF_PADDING         16
>+#define MBUF_SIZE(mtu)       (MTU_TO_MAX_LEN(mtu) + (512) +
>(MBUF_PADDING) + \
>                              sizeof(struct rte_mbuf) +
>RTE_PKTMBUF_HEADROOM)

This change was not immediately clear to me. Then I tried to remove it and
I noticed a performance regression.

If the goal is just to make sure that the data room is at least
2048 + RTE_PKTMBUF_HEADROOM bytes, I would rewrite the macro like this:

#define MBUF_SIZE_MTU(mtu)   (MTU_TO_MAX_LEN(mtu) \
                              + sizeof(struct dp_packet) \
                              + RTE_PKTMBUF_HEADROOM)
#define MBUF_SIZE_DRIVER     (2048 \
                              + sizeof (struct rte_mbuf) \
                              + RTE_PKTMBUF_HEADROOM)
#define MBUF_SIZE(mtu)       MAX(MBUF_SIZE_MTU(mtu), MBUF_SIZE_DRIVER)


and add a comment to explain the logic in the code.

What do you think?

> 
> /* Max and min number of packets in the mempool.  OVS tries to allocate a
>-- 
>1.8.3.1
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm
>B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=V_hfzmHBLMGOqfzxOfYUVPRT7VFrrB
>z8dLoIfO8Vgy8&s=rdO3dIiqUmq0xrpIrUTBHmhK-e_LCswhVc2GEg3AyHE&e= 




More information about the dev mailing list