* [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 0/4] " Olivier Matz
@ 2018-01-19 15:55 ` Olivier Matz
2018-01-22 2:56 ` Tiwei Bie
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 2/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Olivier Matz @ 2018-01-19 15:55 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie; +Cc: stable
When using vector Rx mode (use_simple_rx = 1), vq->vq_descx[] is not
kept up to date. To properly detach the mbufs in this case, browse
sw_ring[] instead, as it's done in virtqueue_rxvq_flush().
Since we need virtio_get_queue_type(), also move this function in
virtqueue.h as a static inline.
Fixes: fc3d66212fed ("virtio: add vector Rx")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
Tiwei,
While it passes my test plan, please carefully check that what I did in
virtqueue_detatch_unused() is correct. I tried to reproduce what is done
in virtqueue_rxvq_flush(), but I may be mistaking due to the different
ring layout assumption and mbuf management between standard and vector.
Thanks
Olivier
drivers/net/virtio/virtio_ethdev.c | 11 -----------
drivers/net/virtio/virtqueue.c | 17 +++++++++++++++--
drivers/net/virtio/virtqueue.h | 11 +++++++++++
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index ec012c2ac..2c7f902e7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -268,17 +268,6 @@ virtio_dev_queue_release(void *queue __rte_unused)
/* do nothing */
}
-static int
-virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
-{
- if (vtpci_queue_idx == hw->max_queue_pairs * 2)
- return VTNET_CQ;
- else if (vtpci_queue_idx % 2 == 0)
- return VTNET_RQ;
- else
- return VTNET_TQ;
-}
-
static uint16_t
virtio_get_nr_vq(struct virtio_hw *hw)
{
diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 1ada4fe08..43cb51d1b 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -18,17 +18,30 @@
struct rte_mbuf *
virtqueue_detatch_unused(struct virtqueue *vq)
{
+ struct virtio_hw *hw = vq->hw;
struct rte_mbuf *cookie;
int idx;
+ int type = virtio_get_queue_type(hw, vq->vq_queue_index);
+
+ if (vq == NULL)
+ return NULL;
- if (vq != NULL)
- for (idx = 0; idx < vq->vq_nentries; idx++) {
+ for (idx = 0; idx < vq->vq_nentries; idx++) {
+ if (hw->use_simple_rx && type == VTNET_RQ) {
+ cookie = vq->sw_ring[idx];
+ if (cookie != NULL) {
+ vq->sw_ring[idx] = NULL;
+ return cookie;
+ }
+ } else {
cookie = vq->vq_descx[idx].cookie;
if (cookie != NULL) {
vq->vq_descx[idx].cookie = NULL;
return cookie;
}
}
+ }
+
return NULL;
}
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index fedbaa39c..cae1b318a 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -281,6 +281,17 @@ virtqueue_full(const struct virtqueue *vq)
return vq->vq_free_cnt == 0;
}
+static inline int
+virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
+{
+ if (vtpci_queue_idx == hw->max_queue_pairs * 2)
+ return VTNET_CQ;
+ else if (vtpci_queue_idx % 2 == 0)
+ return VTNET_RQ;
+ else
+ return VTNET_TQ;
+}
+
#define VIRTQUEUE_NUSED(vq) ((uint16_t)((vq)->vq_ring.used->idx - (vq)->vq_used_cons_idx))
void vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
@ 2018-01-22 2:56 ` Tiwei Bie
2018-01-22 10:38 ` Olivier Matz
0 siblings, 1 reply; 28+ messages in thread
From: Tiwei Bie @ 2018-01-22 2:56 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Yuanhan Liu, Maxime Coquelin, stable
On Fri, Jan 19, 2018 at 04:55:53PM +0100, Olivier Matz wrote:
> When using vector Rx mode (use_simple_rx = 1), vq->vq_descx[] is not
> kept up to date. To properly detach the mbufs in this case, browse
> sw_ring[] instead, as it's done in virtqueue_rxvq_flush().
>
> Since we need virtio_get_queue_type(), also move this function in
> virtqueue.h as a static inline.
>
> Fixes: fc3d66212fed ("virtio: add vector Rx")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>
> Tiwei,
>
> While it passes my test plan, please carefully check that what I did in
> virtqueue_detatch_unused() is correct. I tried to reproduce what is done
> in virtqueue_rxvq_flush(), but I may be mistaking due to the different
> ring layout assumption and mbuf management between standard and vector.
>
> Thanks
> Olivier
>
>
> drivers/net/virtio/virtio_ethdev.c | 11 -----------
> drivers/net/virtio/virtqueue.c | 17 +++++++++++++++--
> drivers/net/virtio/virtqueue.h | 11 +++++++++++
> 3 files changed, 26 insertions(+), 13 deletions(-)
>
[...]
> struct rte_mbuf *
> virtqueue_detatch_unused(struct virtqueue *vq)
> {
> + struct virtio_hw *hw = vq->hw;
> struct rte_mbuf *cookie;
> int idx;
> + int type = virtio_get_queue_type(hw, vq->vq_queue_index);
> +
> + if (vq == NULL)
> + return NULL;
>
> - if (vq != NULL)
> - for (idx = 0; idx < vq->vq_nentries; idx++) {
> + for (idx = 0; idx < vq->vq_nentries; idx++) {
> + if (hw->use_simple_rx && type == VTNET_RQ) {
> + cookie = vq->sw_ring[idx];
> + if (cookie != NULL) {
> + vq->sw_ring[idx] = NULL;
Thanks for working on this!
The vq->sw_ring[idx] isn't zeroed during Rx. So besides the
check of (cookie != NULL), some other check is also needed
to avoid freeing the mbufs already delivered to application.
The mbufs in below interval belong to application:
start: sw_ring[vq->vq_avail_idx & (vq->vq_nentries - 1)] (included)
end: sw_ring[(vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1)] (excluded)
PS. (vq->vq_avail_idx & (vq->vq_nentries - 1)) can be greater than
(vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1).
Thanks,
Tiwei
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled
2018-01-22 2:56 ` Tiwei Bie
@ 2018-01-22 10:38 ` Olivier Matz
2018-01-23 2:05 ` Tiwei Bie
0 siblings, 1 reply; 28+ messages in thread
From: Olivier Matz @ 2018-01-22 10:38 UTC (permalink / raw)
To: Tiwei Bie; +Cc: dev, Yuanhan Liu, Maxime Coquelin, stable
On Mon, Jan 22, 2018 at 10:56:41AM +0800, Tiwei Bie wrote:
> On Fri, Jan 19, 2018 at 04:55:53PM +0100, Olivier Matz wrote:
> > When using vector Rx mode (use_simple_rx = 1), vq->vq_descx[] is not
> > kept up to date. To properly detach the mbufs in this case, browse
> > sw_ring[] instead, as it's done in virtqueue_rxvq_flush().
> >
> > Since we need virtio_get_queue_type(), also move this function in
> > virtqueue.h as a static inline.
> >
> > Fixes: fc3d66212fed ("virtio: add vector Rx")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >
> > Tiwei,
> >
> > While it passes my test plan, please carefully check that what I did in
> > virtqueue_detatch_unused() is correct. I tried to reproduce what is done
> > in virtqueue_rxvq_flush(), but I may be mistaking due to the different
> > ring layout assumption and mbuf management between standard and vector.
> >
> > Thanks
> > Olivier
> >
> >
> > drivers/net/virtio/virtio_ethdev.c | 11 -----------
> > drivers/net/virtio/virtqueue.c | 17 +++++++++++++++--
> > drivers/net/virtio/virtqueue.h | 11 +++++++++++
> > 3 files changed, 26 insertions(+), 13 deletions(-)
> >
> [...]
> > struct rte_mbuf *
> > virtqueue_detatch_unused(struct virtqueue *vq)
> > {
> > + struct virtio_hw *hw = vq->hw;
> > struct rte_mbuf *cookie;
> > int idx;
> > + int type = virtio_get_queue_type(hw, vq->vq_queue_index);
> > +
> > + if (vq == NULL)
> > + return NULL;
> >
> > - if (vq != NULL)
> > - for (idx = 0; idx < vq->vq_nentries; idx++) {
> > + for (idx = 0; idx < vq->vq_nentries; idx++) {
> > + if (hw->use_simple_rx && type == VTNET_RQ) {
> > + cookie = vq->sw_ring[idx];
> > + if (cookie != NULL) {
> > + vq->sw_ring[idx] = NULL;
>
> Thanks for working on this!
>
> The vq->sw_ring[idx] isn't zeroed during Rx. So besides the
> check of (cookie != NULL), some other check is also needed
> to avoid freeing the mbufs already delivered to application.
>
> The mbufs in below interval belong to application:
>
> start: sw_ring[vq->vq_avail_idx & (vq->vq_nentries - 1)] (included)
> end: sw_ring[(vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1)] (excluded)
>
> PS. (vq->vq_avail_idx & (vq->vq_nentries - 1)) can be greater than
> (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1).
Thank you for the review. What about something like this?
struct rte_mbuf *
virtqueue_detach_unused(struct virtqueue *vq)
{
struct rte_mbuf *cookie;
struct virtio_hw *hw;
uint16_t start, end;
int type, idx;
if (vq == NULL)
return NULL;
hw = vq->hw;
type = virtio_get_queue_type(hw, vq->vq_queue_index);
start = vq->vq_avail_idx & (vq->vq_nentries - 1);
end = (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1);
end = (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1);
for (idx = 0; idx < vq->vq_nentries; idx++) {
if (hw->use_simple_rx && type == VTNET_RQ) {
if (start <= end && idx >= start && idx < end)
continue;
if (start > end && (idx >= start || idx < end))
continue;
cookie = vq->sw_ring[idx];
if (cookie == NULL)
continue;
vq->sw_ring[idx] = NULL;
return cookie;
} else {
cookie = vq->vq_descx[idx].cookie;
if (cookie != NULL) {
vq->vq_descx[idx].cookie = NULL;
return cookie;
}
}
}
return NULL;
}
Regards,
Olivier
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled
2018-01-22 10:38 ` Olivier Matz
@ 2018-01-23 2:05 ` Tiwei Bie
0 siblings, 0 replies; 28+ messages in thread
From: Tiwei Bie @ 2018-01-23 2:05 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Yuanhan Liu, Maxime Coquelin, stable
On Mon, Jan 22, 2018 at 11:38:58AM +0100, Olivier Matz wrote:
> On Mon, Jan 22, 2018 at 10:56:41AM +0800, Tiwei Bie wrote:
> > On Fri, Jan 19, 2018 at 04:55:53PM +0100, Olivier Matz wrote:
> > > When using vector Rx mode (use_simple_rx = 1), vq->vq_descx[] is not
> > > kept up to date. To properly detach the mbufs in this case, browse
> > > sw_ring[] instead, as it's done in virtqueue_rxvq_flush().
> > >
> > > Since we need virtio_get_queue_type(), also move this function in
> > > virtqueue.h as a static inline.
> > >
> > > Fixes: fc3d66212fed ("virtio: add vector Rx")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > >
> > > Tiwei,
> > >
> > > While it passes my test plan, please carefully check that what I did in
> > > virtqueue_detatch_unused() is correct. I tried to reproduce what is done
> > > in virtqueue_rxvq_flush(), but I may be mistaking due to the different
> > > ring layout assumption and mbuf management between standard and vector.
> > >
> > > Thanks
> > > Olivier
> > >
> > >
> > > drivers/net/virtio/virtio_ethdev.c | 11 -----------
> > > drivers/net/virtio/virtqueue.c | 17 +++++++++++++++--
> > > drivers/net/virtio/virtqueue.h | 11 +++++++++++
> > > 3 files changed, 26 insertions(+), 13 deletions(-)
> > >
> > [...]
> > > struct rte_mbuf *
> > > virtqueue_detatch_unused(struct virtqueue *vq)
> > > {
> > > + struct virtio_hw *hw = vq->hw;
> > > struct rte_mbuf *cookie;
> > > int idx;
> > > + int type = virtio_get_queue_type(hw, vq->vq_queue_index);
> > > +
> > > + if (vq == NULL)
> > > + return NULL;
> > >
> > > - if (vq != NULL)
> > > - for (idx = 0; idx < vq->vq_nentries; idx++) {
> > > + for (idx = 0; idx < vq->vq_nentries; idx++) {
> > > + if (hw->use_simple_rx && type == VTNET_RQ) {
> > > + cookie = vq->sw_ring[idx];
> > > + if (cookie != NULL) {
> > > + vq->sw_ring[idx] = NULL;
> >
> > Thanks for working on this!
> >
> > The vq->sw_ring[idx] isn't zeroed during Rx. So besides the
> > check of (cookie != NULL), some other check is also needed
> > to avoid freeing the mbufs already delivered to application.
> >
> > The mbufs in below interval belong to application:
> >
> > start: sw_ring[vq->vq_avail_idx & (vq->vq_nentries - 1)] (included)
> > end: sw_ring[(vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1)] (excluded)
> >
> > PS. (vq->vq_avail_idx & (vq->vq_nentries - 1)) can be greater than
> > (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1).
>
> Thank you for the review. What about something like this?
>
> struct rte_mbuf *
> virtqueue_detach_unused(struct virtqueue *vq)
> {
> struct rte_mbuf *cookie;
> struct virtio_hw *hw;
> uint16_t start, end;
> int type, idx;
>
> if (vq == NULL)
> return NULL;
>
> hw = vq->hw;
> type = virtio_get_queue_type(hw, vq->vq_queue_index);
> start = vq->vq_avail_idx & (vq->vq_nentries - 1);
> end = (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1);
> end = (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1);
>
> for (idx = 0; idx < vq->vq_nentries; idx++) {
> if (hw->use_simple_rx && type == VTNET_RQ) {
> if (start <= end && idx >= start && idx < end)
> continue;
> if (start > end && (idx >= start || idx < end))
> continue;
> cookie = vq->sw_ring[idx];
> if (cookie == NULL)
> continue;
> vq->sw_ring[idx] = NULL;
> return cookie;
> } else {
> cookie = vq->vq_descx[idx].cookie;
> if (cookie != NULL) {
> vq->vq_descx[idx].cookie = NULL;
> return cookie;
> }
> }
> }
>
> return NULL;
> }
Apart from the repeated line, it looks good to me. Thanks!
Thanks,
Tiwei
^ permalink raw reply [flat|nested] 28+ messages in thread
* [dpdk-dev] [PATCH v2 2/4] net/virtio: fix memory leak when reinitializing device
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 0/4] " Olivier Matz
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
@ 2018-01-19 15:55 ` Olivier Matz
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 3/4] net/virtio: rationalize queue flushing Olivier Matz
` (2 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-19 15:55 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie; +Cc: stable, Zijie Pan
Free the previous queues and the attached mbufs before initializing new
ones.
The function virtio_dev_free_mbufs() is now called when reconfiguring the
device, so we also need to add a check to ensure that it won't crash for
uninitialized queues.
Cc: stable@dpdk.org
Fixes: 60e6f4707ef2 ("net/virtio: reinitialize device when configuring")
Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 2c7f902e7..a8adf0037 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1368,6 +1368,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
/* Reset the device although not necessary at startup */
vtpci_reset(hw);
+ if (hw->vqs) {
+ virtio_dev_free_mbufs(eth_dev);
+ virtio_free_queues(hw);
+ }
+
/* Tell the host we've noticed this device. */
vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
@@ -1863,6 +1868,9 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_rx_queues; i++) {
struct virtnet_rx *rxvq = dev->data->rx_queues[i];
+ if (rxvq == NULL || rxvq->vq == NULL)
+ continue;
+
PMD_INIT_LOG(DEBUG,
"Before freeing rxq[%d] used and unused buf", i);
VIRTQUEUE_DUMP(rxvq->vq);
@@ -1882,6 +1890,9 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_tx_queues; i++) {
struct virtnet_tx *txvq = dev->data->tx_queues[i];
+ if (txvq == NULL || txvq->vq == NULL)
+ continue;
+
PMD_INIT_LOG(DEBUG,
"Before freeing txq[%d] used and unused bufs",
i);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v2 3/4] net/virtio: rationalize queue flushing
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 0/4] " Olivier Matz
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 2/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
@ 2018-01-19 15:55 ` Olivier Matz
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 4/4] net/virtio: fix typo in function name Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-19 15:55 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie
Use the same kind of loop than in virtio_free_queues() and factorize
common code.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 57 ++++++++++++++++----------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index a8adf0037..b8a5f90c7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1862,53 +1862,44 @@ virtio_dev_start(struct rte_eth_dev *dev)
static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
{
+ struct virtio_hw *hw = dev->data->dev_private;
+ uint16_t nr_vq = virtio_get_nr_vq(hw);
+ const char *type __rte_unused;
+ unsigned int i, mbuf_num = 0;
+ struct virtqueue *vq;
struct rte_mbuf *buf;
- int i, mbuf_num = 0;
-
- for (i = 0; i < dev->data->nb_rx_queues; i++) {
- struct virtnet_rx *rxvq = dev->data->rx_queues[i];
+ int queue_type;
- if (rxvq == NULL || rxvq->vq == NULL)
+ for (i = 0; i < nr_vq; i++) {
+ vq = hw->vqs[i];
+ if (!vq)
continue;
- PMD_INIT_LOG(DEBUG,
- "Before freeing rxq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(rxvq->vq);
-
- PMD_INIT_LOG(DEBUG, "rx_queues[%d]=%p", i, rxvq);
- while ((buf = virtqueue_detatch_unused(rxvq->vq)) != NULL) {
- rte_pktmbuf_free(buf);
- mbuf_num++;
- }
-
- PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
- PMD_INIT_LOG(DEBUG,
- "After freeing rxq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(rxvq->vq);
- }
-
- for (i = 0; i < dev->data->nb_tx_queues; i++) {
- struct virtnet_tx *txvq = dev->data->tx_queues[i];
-
- if (txvq == NULL || txvq->vq == NULL)
+ queue_type = virtio_get_queue_type(hw, i);
+ if (queue_type == VTNET_RQ)
+ type = "rxq";
+ else if (queue_type == VTNET_TQ)
+ type = "txq";
+ else
continue;
PMD_INIT_LOG(DEBUG,
- "Before freeing txq[%d] used and unused bufs",
- i);
- VIRTQUEUE_DUMP(txvq->vq);
+ "Before freeing %s[%d] used and unused buf",
+ type, i);
+ VIRTQUEUE_DUMP(vq);
- mbuf_num = 0;
- while ((buf = virtqueue_detatch_unused(txvq->vq)) != NULL) {
+ while ((buf = virtqueue_detatch_unused(vq)) != NULL) {
rte_pktmbuf_free(buf);
mbuf_num++;
}
- PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
PMD_INIT_LOG(DEBUG,
- "After freeing txq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(txvq->vq);
+ "After freeing %s[%d] used and unused buf",
+ type, i);
+ VIRTQUEUE_DUMP(vq);
}
+
+ PMD_INIT_LOG(DEBUG, "%d mbufs freed", mbuf_num);
}
/*
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v2 4/4] net/virtio: fix typo in function name
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 0/4] " Olivier Matz
` (2 preceding siblings ...)
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 3/4] net/virtio: rationalize queue flushing Olivier Matz
@ 2018-01-19 15:55 ` Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-19 15:55 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie
Fixes: c1f86306a026 ("virtio: add new driver")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtqueue.c | 2 +-
drivers/net/virtio/virtqueue.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b8a5f90c7..d196afad3 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1888,7 +1888,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
type, i);
VIRTQUEUE_DUMP(vq);
- while ((buf = virtqueue_detatch_unused(vq)) != NULL) {
+ while ((buf = virtqueue_detach_unused(vq)) != NULL) {
rte_pktmbuf_free(buf);
mbuf_num++;
}
diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 43cb51d1b..8682455b1 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -16,7 +16,7 @@
* 2) mbuf that hasn't been consued by backend.
*/
struct rte_mbuf *
-virtqueue_detatch_unused(struct virtqueue *vq)
+virtqueue_detach_unused(struct virtqueue *vq)
{
struct virtio_hw *hw = vq->hw;
struct rte_mbuf *cookie;
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index cae1b318a..35145e7b2 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -270,7 +270,7 @@ void virtqueue_dump(struct virtqueue *vq);
/**
* Get all mbufs to be freed.
*/
-struct rte_mbuf *virtqueue_detatch_unused(struct virtqueue *vq);
+struct rte_mbuf *virtqueue_detach_unused(struct virtqueue *vq);
/* Flush the elements in the used ring. */
void virtqueue_rxvq_flush(struct virtqueue *vq);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 0/4] " Olivier Matz
` (3 preceding siblings ...)
2018-01-19 15:55 ` [dpdk-dev] [PATCH v2 4/4] net/virtio: fix typo in function name Olivier Matz
@ 2018-01-23 15:54 ` Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
` (4 more replies)
4 siblings, 5 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-23 15:54 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie
When devops->configure() is called, the queues may be reallocated
if the features flag changed, but the previous are not freed. This
patchset fixes this issue.
To really point out the issue, I instrumented rte_malloc,
rte_memzone_reserve, rte_mbuf_alloc to track the allocations and frees.
For reference, the patch is available here:
https://www.droids-corp.org/~zer0/hidden/0001-debug-malloc.patch
Iit conflicts a bit with the patchset, but it is quite easy to solve.
To reproduce the issue:
cd dpdk
make config T=x86_64-native-linuxapp-gcc
sed -i 's,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=n,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=y,' build/.config
make -j4
mkdir -p /mnt/huge
mount -t hugetlbfs nodev /mnt/huge
echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
modprobe uio_pci_generic
python usertools/dpdk-devbind.py -b uio_pci_generic 0000:00:02.0
./build/app/testpmd -l 0,1 --log-level 8 -- --total-num-mbufs=16384 \
-i --port-topology=chained --disable-hw-vlan-filter \
--disable-hw-vlan-strip --enable-rx-cksum --enable-lro \
--txqflags=0 --no-lsc-interrupt
# testpmd commands
port stop 0
port config all rx-cksum off
port start 0
quit
Without the patchset, the debug patch displays a leak of mbufs allocated
from virtio_dev_rx_queue_setup_finish() and a leak of rte_mallocs allocated
from virtio_init_queue().
After the patchset, only one malloc is remaining after the device close,
but this is normal, because this area is freed in dev_uninit().
v2 -> v3:
- fix queue flushing with vector Rx enabled (Tiwei).
The patch is slightly changed compared to what was proposed by mail,
due to a checkpatch warning.
v1 -> v2:
- move the vq != NULL check from the refactor patch to the memory
leak fix patch (Yuanhan)
- new patch to fix queue flushing with vector Rx enabled (Tiwei)
- move the backportable fixes at the beginning of the patchset (Yuanhan)
- remove Cc stable on the typo fix (Yuanhan)
note: I finally kept it in the patchset
Olivier Matz (4):
net/virtio: fix queue flushing with vector Rx enabled
net/virtio: fix memory leak when reinitializing device
net/virtio: rationalize queue flushing
net/virtio: fix typo in function name
drivers/net/virtio/virtio_ethdev.c | 71 +++++++++++++++++---------------------
drivers/net/virtio/virtqueue.c | 30 +++++++++++++---
drivers/net/virtio/virtqueue.h | 13 ++++++-
3 files changed, 69 insertions(+), 45 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v3 1/4] net/virtio: fix queue flushing with vector Rx enabled
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
@ 2018-01-23 15:54 ` Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 2/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-23 15:54 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie; +Cc: stable
When using vector Rx mode (use_simple_rx = 1), vq->vq_descx[] is not
kept up to date. To properly detach the mbufs in this case, browse
sw_ring[] instead, as it's done in virtqueue_rxvq_flush().
Since we need virtio_get_queue_type(), also move this function in
virtqueue.h as a static inline.
Fixes: fc3d66212fed ("virtio: add vector Rx")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 11 -----------
drivers/net/virtio/virtqueue.c | 28 +++++++++++++++++++++++++---
drivers/net/virtio/virtqueue.h | 11 +++++++++++
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 17ac04931..24ffbeeb2 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -279,17 +279,6 @@ virtio_dev_queue_release(void *queue __rte_unused)
/* do nothing */
}
-static int
-virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
-{
- if (vtpci_queue_idx == hw->max_queue_pairs * 2)
- return VTNET_CQ;
- else if (vtpci_queue_idx % 2 == 0)
- return VTNET_RQ;
- else
- return VTNET_TQ;
-}
-
static uint16_t
virtio_get_nr_vq(struct virtio_hw *hw)
{
diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 1ada4fe08..5a8b21204 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -19,16 +19,38 @@ struct rte_mbuf *
virtqueue_detatch_unused(struct virtqueue *vq)
{
struct rte_mbuf *cookie;
- int idx;
+ struct virtio_hw *hw;
+ uint16_t start, end;
+ int type, idx;
- if (vq != NULL)
- for (idx = 0; idx < vq->vq_nentries; idx++) {
+ if (vq == NULL)
+ return NULL;
+
+ hw = vq->hw;
+ type = virtio_get_queue_type(hw, vq->vq_queue_index);
+ start = vq->vq_avail_idx & (vq->vq_nentries - 1);
+ end = (vq->vq_avail_idx + vq->vq_free_cnt) & (vq->vq_nentries - 1);
+
+ for (idx = 0; idx < vq->vq_nentries; idx++) {
+ if (hw->use_simple_rx && type == VTNET_RQ) {
+ if (start <= end && idx >= start && idx < end)
+ continue;
+ if (start > end && (idx >= start || idx < end))
+ continue;
+ cookie = vq->sw_ring[idx];
+ if (cookie != NULL) {
+ vq->sw_ring[idx] = NULL;
+ return cookie;
+ }
+ } else {
cookie = vq->vq_descx[idx].cookie;
if (cookie != NULL) {
vq->vq_descx[idx].cookie = NULL;
return cookie;
}
}
+ }
+
return NULL;
}
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 2d9599218..e46de1583 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -292,6 +292,17 @@ virtqueue_full(const struct virtqueue *vq)
return vq->vq_free_cnt == 0;
}
+static inline int
+virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
+{
+ if (vtpci_queue_idx == hw->max_queue_pairs * 2)
+ return VTNET_CQ;
+ else if (vtpci_queue_idx % 2 == 0)
+ return VTNET_RQ;
+ else
+ return VTNET_TQ;
+}
+
#define VIRTQUEUE_NUSED(vq) ((uint16_t)((vq)->vq_ring.used->idx - (vq)->vq_used_cons_idx))
void vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v3 2/4] net/virtio: fix memory leak when reinitializing device
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
@ 2018-01-23 15:54 ` Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 3/4] net/virtio: rationalize queue flushing Olivier Matz
` (2 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-23 15:54 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie; +Cc: stable, Zijie Pan
Free the previous queues and the attached mbufs before initializing new
ones.
The function virtio_dev_free_mbufs() is now called when reconfiguring the
device, so we also need to add a check to ensure that it won't crash for
uninitialized queues.
Cc: stable@dpdk.org
Fixes: 60e6f4707ef2 ("net/virtio: reinitialize device when configuring")
Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 24ffbeeb2..500556a05 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1471,6 +1471,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
/* Reset the device although not necessary at startup */
vtpci_reset(hw);
+ if (hw->vqs) {
+ virtio_dev_free_mbufs(eth_dev);
+ virtio_free_queues(hw);
+ }
+
/* Tell the host we've noticed this device. */
vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
@@ -1968,6 +1973,9 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_rx_queues; i++) {
struct virtnet_rx *rxvq = dev->data->rx_queues[i];
+ if (rxvq == NULL || rxvq->vq == NULL)
+ continue;
+
PMD_INIT_LOG(DEBUG,
"Before freeing rxq[%d] used and unused buf", i);
VIRTQUEUE_DUMP(rxvq->vq);
@@ -1987,6 +1995,9 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_tx_queues; i++) {
struct virtnet_tx *txvq = dev->data->tx_queues[i];
+ if (txvq == NULL || txvq->vq == NULL)
+ continue;
+
PMD_INIT_LOG(DEBUG,
"Before freeing txq[%d] used and unused bufs",
i);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v3 3/4] net/virtio: rationalize queue flushing
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 1/4] net/virtio: fix queue flushing with vector Rx enabled Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 2/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
@ 2018-01-23 15:54 ` Olivier Matz
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 4/4] net/virtio: fix typo in function name Olivier Matz
2018-01-26 15:34 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Yuanhan Liu
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-23 15:54 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie
Use the same kind of loop than in virtio_free_queues() and factorize
common code.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 57 ++++++++++++++++----------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 500556a05..260179598 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1967,53 +1967,44 @@ virtio_dev_start(struct rte_eth_dev *dev)
static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
{
+ struct virtio_hw *hw = dev->data->dev_private;
+ uint16_t nr_vq = virtio_get_nr_vq(hw);
+ const char *type __rte_unused;
+ unsigned int i, mbuf_num = 0;
+ struct virtqueue *vq;
struct rte_mbuf *buf;
- int i, mbuf_num = 0;
-
- for (i = 0; i < dev->data->nb_rx_queues; i++) {
- struct virtnet_rx *rxvq = dev->data->rx_queues[i];
+ int queue_type;
- if (rxvq == NULL || rxvq->vq == NULL)
+ for (i = 0; i < nr_vq; i++) {
+ vq = hw->vqs[i];
+ if (!vq)
continue;
- PMD_INIT_LOG(DEBUG,
- "Before freeing rxq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(rxvq->vq);
-
- PMD_INIT_LOG(DEBUG, "rx_queues[%d]=%p", i, rxvq);
- while ((buf = virtqueue_detatch_unused(rxvq->vq)) != NULL) {
- rte_pktmbuf_free(buf);
- mbuf_num++;
- }
-
- PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
- PMD_INIT_LOG(DEBUG,
- "After freeing rxq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(rxvq->vq);
- }
-
- for (i = 0; i < dev->data->nb_tx_queues; i++) {
- struct virtnet_tx *txvq = dev->data->tx_queues[i];
-
- if (txvq == NULL || txvq->vq == NULL)
+ queue_type = virtio_get_queue_type(hw, i);
+ if (queue_type == VTNET_RQ)
+ type = "rxq";
+ else if (queue_type == VTNET_TQ)
+ type = "txq";
+ else
continue;
PMD_INIT_LOG(DEBUG,
- "Before freeing txq[%d] used and unused bufs",
- i);
- VIRTQUEUE_DUMP(txvq->vq);
+ "Before freeing %s[%d] used and unused buf",
+ type, i);
+ VIRTQUEUE_DUMP(vq);
- mbuf_num = 0;
- while ((buf = virtqueue_detatch_unused(txvq->vq)) != NULL) {
+ while ((buf = virtqueue_detatch_unused(vq)) != NULL) {
rte_pktmbuf_free(buf);
mbuf_num++;
}
- PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
PMD_INIT_LOG(DEBUG,
- "After freeing txq[%d] used and unused buf", i);
- VIRTQUEUE_DUMP(txvq->vq);
+ "After freeing %s[%d] used and unused buf",
+ type, i);
+ VIRTQUEUE_DUMP(vq);
}
+
+ PMD_INIT_LOG(DEBUG, "%d mbufs freed", mbuf_num);
}
/*
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* [dpdk-dev] [PATCH v3 4/4] net/virtio: fix typo in function name
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
` (2 preceding siblings ...)
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 3/4] net/virtio: rationalize queue flushing Olivier Matz
@ 2018-01-23 15:54 ` Olivier Matz
2018-01-26 15:34 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Yuanhan Liu
4 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2018-01-23 15:54 UTC (permalink / raw)
To: dev, Yuanhan Liu, Maxime Coquelin, Tiwei Bie
Fixes: c1f86306a026 ("virtio: add new driver")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtqueue.c | 2 +-
drivers/net/virtio/virtqueue.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 260179598..ce5663902 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1993,7 +1993,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
type, i);
VIRTQUEUE_DUMP(vq);
- while ((buf = virtqueue_detatch_unused(vq)) != NULL) {
+ while ((buf = virtqueue_detach_unused(vq)) != NULL) {
rte_pktmbuf_free(buf);
mbuf_num++;
}
diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 5a8b21204..a7d0a9cbe 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -16,7 +16,7 @@
* 2) mbuf that hasn't been consued by backend.
*/
struct rte_mbuf *
-virtqueue_detatch_unused(struct virtqueue *vq)
+virtqueue_detach_unused(struct virtqueue *vq)
{
struct rte_mbuf *cookie;
struct virtio_hw *hw;
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index e46de1583..14364f356 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -281,7 +281,7 @@ void virtqueue_dump(struct virtqueue *vq);
/**
* Get all mbufs to be freed.
*/
-struct rte_mbuf *virtqueue_detatch_unused(struct virtqueue *vq);
+struct rte_mbuf *virtqueue_detach_unused(struct virtqueue *vq);
/* Flush the elements in the used ring. */
void virtqueue_rxvq_flush(struct virtqueue *vq);
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 0/4] net/virtio: fix memory leak when reinitializing device Olivier Matz
` (3 preceding siblings ...)
2018-01-23 15:54 ` [dpdk-dev] [PATCH v3 4/4] net/virtio: fix typo in function name Olivier Matz
@ 2018-01-26 15:34 ` Yuanhan Liu
4 siblings, 0 replies; 28+ messages in thread
From: Yuanhan Liu @ 2018-01-26 15:34 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Maxime Coquelin, Tiwei Bie
On Tue, Jan 23, 2018 at 04:54:39PM +0100, Olivier Matz wrote:
> When devops->configure() is called, the queues may be reallocated
> if the features flag changed, but the previous are not freed. This
> patchset fixes this issue.
Seires applied to dpdk-next-virtio.
Thanks.
--yliu
>
> To really point out the issue, I instrumented rte_malloc,
> rte_memzone_reserve, rte_mbuf_alloc to track the allocations and frees.
> For reference, the patch is available here:
> https://www.droids-corp.org/~zer0/hidden/0001-debug-malloc.patch
> Iit conflicts a bit with the patchset, but it is quite easy to solve.
>
> To reproduce the issue:
>
> cd dpdk
> make config T=x86_64-native-linuxapp-gcc
> sed -i 's,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=n,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=y,' build/.config
> make -j4
> mkdir -p /mnt/huge
> mount -t hugetlbfs nodev /mnt/huge
> echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
> modprobe uio_pci_generic
> python usertools/dpdk-devbind.py -b uio_pci_generic 0000:00:02.0
> ./build/app/testpmd -l 0,1 --log-level 8 -- --total-num-mbufs=16384 \
> -i --port-topology=chained --disable-hw-vlan-filter \
> --disable-hw-vlan-strip --enable-rx-cksum --enable-lro \
> --txqflags=0 --no-lsc-interrupt
>
> # testpmd commands
> port stop 0
> port config all rx-cksum off
> port start 0
> quit
>
> Without the patchset, the debug patch displays a leak of mbufs allocated
> from virtio_dev_rx_queue_setup_finish() and a leak of rte_mallocs allocated
> from virtio_init_queue().
>
> After the patchset, only one malloc is remaining after the device close,
> but this is normal, because this area is freed in dev_uninit().
>
> v2 -> v3:
> - fix queue flushing with vector Rx enabled (Tiwei).
> The patch is slightly changed compared to what was proposed by mail,
> due to a checkpatch warning.
>
> v1 -> v2:
> - move the vq != NULL check from the refactor patch to the memory
> leak fix patch (Yuanhan)
> - new patch to fix queue flushing with vector Rx enabled (Tiwei)
> - move the backportable fixes at the beginning of the patchset (Yuanhan)
> - remove Cc stable on the typo fix (Yuanhan)
> note: I finally kept it in the patchset
>
>
> Olivier Matz (4):
> net/virtio: fix queue flushing with vector Rx enabled
> net/virtio: fix memory leak when reinitializing device
> net/virtio: rationalize queue flushing
> net/virtio: fix typo in function name
>
> drivers/net/virtio/virtio_ethdev.c | 71 +++++++++++++++++---------------------
> drivers/net/virtio/virtqueue.c | 30 +++++++++++++---
> drivers/net/virtio/virtqueue.h | 13 ++++++-
> 3 files changed, 69 insertions(+), 45 deletions(-)
>
> --
> 2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread