From: Yunjian Wang <wangyunjian@huawei.com> In hn_dev_tx_queue_setup() allocated memory for txq, we should free it when error happens, otherwise it will lead to memory leak. Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx descriptors") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- drivers/net/netvsc/hn_rxtx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index 65f1abae5..8c94e7ee3 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -273,6 +273,7 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, " (tx_free_thresh=%u port=%u queue=%u)\n", nb_desc - 3, tx_free_thresh, dev->data->port_id, queue_idx); + rte_free(txq); return -EINVAL; } -- 2.23.0
>Subject: [dpdk-dev] [PATCH] net/netvsc: fix txq leak in error path
>
>From: Yunjian Wang <wangyunjian@huawei.com>
>
>In hn_dev_tx_queue_setup() allocated memory for txq, we should free it
>when error happens, otherwise it will lead to memory leak.
>
>Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx descriptors")
>Cc: stable@dpdk.org
>
>Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
>---
> drivers/net/netvsc/hn_rxtx.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
>index 65f1abae5..8c94e7ee3 100644
>--- a/drivers/net/netvsc/hn_rxtx.c
>+++ b/drivers/net/netvsc/hn_rxtx.c
>@@ -273,6 +273,7 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev,
> " (tx_free_thresh=%u port=%u queue=%u)\n",
> nb_desc - 3,
> tx_free_thresh, dev->data->port_id, queue_idx);
>+ rte_free(txq);
> return -EINVAL;
> }
>
>--
>2.23.0
How about checking for tx_free_thresh at the beginning of the function, before calling txq = rte_zmalloc_socket()?
This way, you don't' need to call rte_free() on error.
Long
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Long Li > Sent: Thursday, October 1, 2020 6:27 AM > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org > Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan > <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; > jerry.lilijun@huawei.com; xudingke@huawei.com; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] net/netvsc: fix txq leak in error path > > >Subject: [dpdk-dev] [PATCH] net/netvsc: fix txq leak in error path > > > >From: Yunjian Wang <wangyunjian@huawei.com> > > > >In hn_dev_tx_queue_setup() allocated memory for txq, we should free it > >when error happens, otherwise it will lead to memory leak. > > > >Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx > >descriptors") > >Cc: stable@dpdk.org > > > >Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > >--- > > drivers/net/netvsc/hn_rxtx.c | 1 + > > 1 file changed, 1 insertion(+) > > > >diff --git a/drivers/net/netvsc/hn_rxtx.c > >b/drivers/net/netvsc/hn_rxtx.c index 65f1abae5..8c94e7ee3 100644 > >--- a/drivers/net/netvsc/hn_rxtx.c > >+++ b/drivers/net/netvsc/hn_rxtx.c > >@@ -273,6 +273,7 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, > > " (tx_free_thresh=%u port=%u queue=%u)\n", > > nb_desc - 3, > > tx_free_thresh, dev->data->port_id, queue_idx); > >+ rte_free(txq); > > return -EINVAL; > > } > > > >-- > >2.23.0 > > How about checking for tx_free_thresh at the beginning of the function, > before calling txq = rte_zmalloc_socket()? > > This way, you don't' need to call rte_free() on error. OK, I will fix it in next version. Thanks, Yunjian > > Long
From: Yunjian Wang <wangyunjian@huawei.com> In hn_dev_tx_queue_setup() allocated memory for txq, we don't free it when error happens and it will lead to memory leak. We can check for tx_free_thresh at the beginning of the function to fix it, before calling txq = rte_zmalloc_socket(). Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx descriptors") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- v2: fix code styles suggested by Long Li --- drivers/net/netvsc/hn_rxtx.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index 65f1abae5..5d59db513 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -252,16 +252,6 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, PMD_INIT_FUNC_TRACE(); - txq = rte_zmalloc_socket("HN_TXQ", sizeof(*txq), RTE_CACHE_LINE_SIZE, - socket_id); - if (!txq) - return -ENOMEM; - - txq->hv = hv; - txq->chan = hv->channels[queue_idx]; - txq->port_id = dev->data->port_id; - txq->queue_id = queue_idx; - tx_free_thresh = tx_conf->tx_free_thresh; if (tx_free_thresh == 0) tx_free_thresh = RTE_MIN(nb_desc / 4, @@ -276,6 +266,15 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, return -EINVAL; } + txq = rte_zmalloc_socket("HN_TXQ", sizeof(*txq), RTE_CACHE_LINE_SIZE, + socket_id); + if (!txq) + return -ENOMEM; + + txq->hv = hv; + txq->chan = hv->channels[queue_idx]; + txq->port_id = dev->data->port_id; + txq->queue_id = queue_idx; txq->free_thresh = tx_free_thresh; snprintf(name, sizeof(name), -- 2.23.0
>Subject: [dpdk-dev] [PATCH v2] net/netvsc: fix txq leak in error path > >From: Yunjian Wang <wangyunjian@huawei.com> > >In hn_dev_tx_queue_setup() allocated memory for txq, we don't free it >when error happens and it will lead to memory leak. > >We can check for tx_free_thresh at the beginning of the function to fix it, >before calling txq = rte_zmalloc_socket(). > >Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx descriptors") >Cc: stable@dpdk.org > >Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Acked-by: Long Li <longli@microsoft.com> >--- >v2: > fix code styles suggested by Long Li >--- > drivers/net/netvsc/hn_rxtx.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > >diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c >index 65f1abae5..5d59db513 100644 >--- a/drivers/net/netvsc/hn_rxtx.c >+++ b/drivers/net/netvsc/hn_rxtx.c >@@ -252,16 +252,6 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, > > PMD_INIT_FUNC_TRACE(); > >- txq = rte_zmalloc_socket("HN_TXQ", sizeof(*txq), >RTE_CACHE_LINE_SIZE, >- socket_id); >- if (!txq) >- return -ENOMEM; >- >- txq->hv = hv; >- txq->chan = hv->channels[queue_idx]; >- txq->port_id = dev->data->port_id; >- txq->queue_id = queue_idx; >- > tx_free_thresh = tx_conf->tx_free_thresh; > if (tx_free_thresh == 0) > tx_free_thresh = RTE_MIN(nb_desc / 4, @@ -276,6 +266,15 >@@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev, > return -EINVAL; > } > >+ txq = rte_zmalloc_socket("HN_TXQ", sizeof(*txq), >RTE_CACHE_LINE_SIZE, >+ socket_id); >+ if (!txq) >+ return -ENOMEM; >+ >+ txq->hv = hv; >+ txq->chan = hv->channels[queue_idx]; >+ txq->port_id = dev->data->port_id; >+ txq->queue_id = queue_idx; > txq->free_thresh = tx_free_thresh; > > snprintf(name, sizeof(name), >-- >2.23.0
On 10/9/2020 4:53 AM, Long Li wrote:
>> Subject: [dpdk-dev] [PATCH v2] net/netvsc: fix txq leak in error path
>>
>> From: Yunjian Wang <wangyunjian@huawei.com>
>>
>> In hn_dev_tx_queue_setup() allocated memory for txq, we don't free it
>> when error happens and it will lead to memory leak.
>>
>> We can check for tx_free_thresh at the beginning of the function to fix it,
>> before calling txq = rte_zmalloc_socket().
>>
>> Fixes: cc0251813277 ("net/netvsc: split send buffers from Tx descriptors")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
>
> Acked-by: Long Li <longli@microsoft.com>
>
Applied to dpdk-next-net/main, thanks.