> -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] > Sent: Thursday, August 6, 2020 12:36 AM > To: wangyunjian <wangyunj...@huawei.com>; dev@dpdk.org > Cc: keith.wi...@intel.com; ophi...@mellanox.com; Lilijun (Jerry) > <jerry.lili...@huawei.com>; xudingke <xudin...@huawei.com>; > sta...@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] net/tap: free mempool when closing > > On 7/29/2020 12:35 PM, wangyunjian wrote: > > From: Yunjian Wang <wangyunj...@huawei.com> > > > > When setup tx queues, we will create a mempool for the 'gso_ctx'. > > The mempool is not freed when closing tap device. If free the tap > > device and create it with different name, it will create a new > > mempool. This maybe cause an OOM. > > > > Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)") > > Cc: sta...@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> > > <...> > > > @@ -1317,26 +1320,31 @@ tap_gso_ctx_setup(struct rte_gso_ctx *gso_ctx, > struct rte_eth_dev *dev) > > { > > uint32_t gso_types; > > char pool_name[64]; > > - > > - /* > > - * Create private mbuf pool with TAP_GSO_MBUF_SEG_SIZE bytes > > - * size per mbuf use this pool for both direct and indirect mbufs > > - */ > > - > > - struct rte_mempool *mp; /* Mempool for GSO packets */ > > + struct pmd_internals *pmd = dev->data->dev_private; > > + int ret; > > > > /* initialize GSO context */ > > gso_types = DEV_TX_OFFLOAD_TCP_TSO; > > - snprintf(pool_name, sizeof(pool_name), "mp_%s", dev->device->name); > > - mp = rte_mempool_lookup((const char *)pool_name); > > - if (!mp) { > > - mp = rte_pktmbuf_pool_create(pool_name, TAP_GSO_MBUFS_NUM, > > - TAP_GSO_MBUF_CACHE_SIZE, 0, > > + if (!pmd->gso_ctx_mp) { > > + /* > > + * Create private mbuf pool with TAP_GSO_MBUF_SEG_SIZE > > + * bytes size per mbuf use this pool for both direct and > > + * indirect mbufs > > + */ > > + ret = snprintf(pool_name, sizeof(pool_name), "mp_%s", > > + dev->device->name); > > + if (ret < 0 || ret >= (int)sizeof(pool_name)) { > > + TAP_LOG(ERR, > > + "%s: failed to create mbuf pool " > > + "name for device %s\n", > > + pmd->name, dev->device->name); > > Overall looks good. Only above error doesn't say why it failed, informing the > user that device name is too long may help her to overcome the error.
I found that the return value of functions snprintf was not checked when modifying the code, so fixed it. I think it maybe fail, because the max device name length is RTE_DEV_NAME_MAX_LEN(64). Do I need to split into two patches? Thanks, Yunjian
