Hi, Thanks for your comments. Some details inline.
> From: Daniele Di Proietto [mailto:[email protected]] > >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 Ok. I had missed this part getting out of date. > >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? As you suggest, the purpose is to ensure the minimum size is big enough to avoid scattering with normal Ethernet MTU. We actually had a patch version similar to your suggestion at one time, but dropped it for the even simpler alternative because the jumbo frame support is not really there anyway. Anyway, I agree that this MTU sensitive code makes the intention clearer. I will send a V2 soon. -Timo _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
