| Message ID | 1510297926-18710-4-git-send-email-i.maximets@samsung.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | netdev-dpdk: Mempool creation failure + Appctl | expand |
>From: Ilya Maximets [mailto:i.maximets@samsung.com] >Sent: Friday, November 10, 2017 7:12 AM >To: ovs-dev@openvswitch.org >Cc: Heetae Ahn <heetae82.ahn@samsung.com>; Fischetti, Antonio ><antonio.fischetti@intel.com>; Loftus, Ciara <ciara.loftus@intel.com>; >Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Stokes, Ian ><ian.stokes@intel.com>; Wojciechowicz, RobertX ><robertx.wojciechowicz@intel.com>; Flavio Leitner <fbl@redhat.com>; Ilya >Maximets <i.maximets@samsung.com> >Subject: [PATCH v2 3/3] netdev-dpdk: Fix mempool creation with large MTU. > >Currently mempool name size limited to 25 characters by >RTE_MEMPOOL_NAMESIZE. netdev-dpdk tries to create mempool with the >following name pattern: "ovs_%{hash}_%{socket}_%{mtu}_%{n_mbuf}". > >We have 3 chars for "ovs" + 4 chars for delimiters + 8 chars for >hash (because it's the 32 bit integer printed in hex) + 1 char for >socket_id (mostly 1, but it could be 2 on some systems; larger?) = 16. > >Only 25 - 16 = 9 characters remains for mtu + n_mbufs. >Minimum usual value for mtu is 1500 --> 2030 (4 chars) after >dpdk_buf_size conversion and the minimum value for n_mbufs is 16384 >(5 chars). So, all the 9 characters are used. > >If we'll try to create port with mtu = 9500, mempool creation will >fail, because FRAME_LEN_TO_MTU(dpdk_buf_size(9500)) = 10222 (5 chars) >and this value will overflow the RTE_MEMPOOL_NAMESIZE limit. > >Same issue will happen if we'll try to create port with big enough >number of queues or will try to create big enough number of PMD >threads (number of tx queues will enlarge the mempool requirements). > >Fix that by removing the delimiters. To keep the readability (at least >partial) of the mempool names exact field sizes with zero padding >are used. > >Following limits should be suitable for now: > - Hash length: 8 chars (uint32_t in hex) > - Socket ID : 2 chars (For systems with up to 10 sockets) > - MTU : 5 chars (MTU (10^5 - 1) should be enough for now) > - n_mbufs : 7 chars (Up to 10^7 of mbufs) > > Total : 22 + 3 (for "ovs") = 25 > >CC: Antonio Fischetti <antonio.fischetti@intel.com> >CC: Robert Wojciechowicz <robertx.wojciechowicz@intel.com> >Fixes: f06546a51dd8 ("Fix mempool names to reflect socket id.") >Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each >port.") >Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> Thanks for this fix Ilya. I've tested this, and can confirm that it resolves a SEGV that previously occurred in the case of 'large' MTU values (in the case of my test, 9710). Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Best, Mark >--- > lib/netdev-dpdk.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index 75856bc..dc210cc 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -510,7 +510,8 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) > do { > /* Full DPDK memory pool name must be unique and cannot be > * longer than RTE_MEMPOOL_NAMESIZE. */ >- int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u", >+ int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, >+ "ovs%08x%02d%05d%07u", > hash, socket_id, mtu, n_mbufs); > if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) { > VLOG_DBG("snprintf returned %d. " >-- >2.7.4
> Thanks for this fix Ilya. > > I've tested this, and can confirm that it resolves a SEGV that previously > occurred in the case of 'large' MTU values (in the case of my test, 9710). > > Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> > Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com> > Thanks all for the work on this, I'll look to push this to the dpdk merge branch today. Ian > Best, > Mark > > >--- > > lib/netdev-dpdk.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >75856bc..dc210cc 100644 > >--- a/lib/netdev-dpdk.c > >+++ b/lib/netdev-dpdk.c > >@@ -510,7 +510,8 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) > > do { > > /* Full DPDK memory pool name must be unique and cannot be > > * longer than RTE_MEMPOOL_NAMESIZE. */ > >- int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, > "ovs_%x_%d_%d_%u", > >+ int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, > >+ "ovs%08x%02d%05d%07u", > > hash, socket_id, mtu, n_mbufs); > > if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) { > > VLOG_DBG("snprintf returned %d. " > >-- > >2.7.4
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 75856bc..dc210cc 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -510,7 +510,8 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) do { /* Full DPDK memory pool name must be unique and cannot be * longer than RTE_MEMPOOL_NAMESIZE. */ - int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u", + int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, + "ovs%08x%02d%05d%07u", hash, socket_id, mtu, n_mbufs); if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) { VLOG_DBG("snprintf returned %d. "
Currently mempool name size limited to 25 characters by RTE_MEMPOOL_NAMESIZE. netdev-dpdk tries to create mempool with the following name pattern: "ovs_%{hash}_%{socket}_%{mtu}_%{n_mbuf}". We have 3 chars for "ovs" + 4 chars for delimiters + 8 chars for hash (because it's the 32 bit integer printed in hex) + 1 char for socket_id (mostly 1, but it could be 2 on some systems; larger?) = 16. Only 25 - 16 = 9 characters remains for mtu + n_mbufs. Minimum usual value for mtu is 1500 --> 2030 (4 chars) after dpdk_buf_size conversion and the minimum value for n_mbufs is 16384 (5 chars). So, all the 9 characters are used. If we'll try to create port with mtu = 9500, mempool creation will fail, because FRAME_LEN_TO_MTU(dpdk_buf_size(9500)) = 10222 (5 chars) and this value will overflow the RTE_MEMPOOL_NAMESIZE limit. Same issue will happen if we'll try to create port with big enough number of queues or will try to create big enough number of PMD threads (number of tx queues will enlarge the mempool requirements). Fix that by removing the delimiters. To keep the readability (at least partial) of the mempool names exact field sizes with zero padding are used. Following limits should be suitable for now: - Hash length: 8 chars (uint32_t in hex) - Socket ID : 2 chars (For systems with up to 10 sockets) - MTU : 5 chars (MTU (10^5 - 1) should be enough for now) - n_mbufs : 7 chars (Up to 10^7 of mbufs) Total : 22 + 3 (for "ovs") = 25 CC: Antonio Fischetti <antonio.fischetti@intel.com> CC: Robert Wojciechowicz <robertx.wojciechowicz@intel.com> Fixes: f06546a51dd8 ("Fix mempool names to reflect socket id.") Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> --- lib/netdev-dpdk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)