The patch looks good, thanks.
Two comments below
On 26/08/2015 13:44, "Timo Puha" <timox.p...@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.kavan...@intel.com>
>Signed-off-by: Michal Weglicki <michalx.wegli...@intel.com>
>Signed-off-by: Timo Puha <timox.p...@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@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=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
