[ovs-dev] [PATCH v2 3/3] netdev-dpdk: Fix mempool creation with large MTU.

Kavanagh, Mark B mark.b.kavanagh at intel.com
Wed Nov 15 10:48:02 UTC 2017


>From: Ilya Maximets [mailto:i.maximets at samsung.com]
>Sent: Friday, November 10, 2017 7:12 AM
>To: ovs-dev at openvswitch.org
>Cc: Heetae Ahn <heetae82.ahn at samsung.com>; Fischetti, Antonio
><antonio.fischetti at intel.com>; Loftus, Ciara <ciara.loftus at intel.com>;
>Kavanagh, Mark B <mark.b.kavanagh at intel.com>; Stokes, Ian
><ian.stokes at intel.com>; Wojciechowicz, RobertX
><robertx.wojciechowicz at intel.com>; Flavio Leitner <fbl at redhat.com>; Ilya
>Maximets <i.maximets at 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 at intel.com>
>CC: Robert Wojciechowicz <robertx.wojciechowicz at 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 at samsung.com>
>Acked-by: Antonio Fischetti <antonio.fischetti at 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 at intel.com>
Tested-by: Mark Kavanagh <mark.b.kavanagh at 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



More information about the dev mailing list