DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx
@ 2019-06-18  7:41 Tiwei Bie
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx Tiwei Bie
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Tiwei Bie @ 2019-06-18  7:41 UTC (permalink / raw)
  To: maxime.coquelin, zhihong.wang, dev

Tiwei Bie (4):
  net/virtio: fix memory leak in in-order Rx
  net/virtio: fix memory leak in mergeable Rx
  net/virtio: fix memory leak in mergeable packed Rx
  net/virtio: fix packets check in mergeable packed Rx

 drivers/net/virtio/virtio_rxtx.c | 56 +++++++++++++++-----------------
 1 file changed, 26 insertions(+), 30 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx
  2019-06-18  7:41 [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx Tiwei Bie
@ 2019-06-18  7:41 ` Tiwei Bie
  2019-06-19 12:34   ` Maxime Coquelin
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx Tiwei Bie
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Tiwei Bie @ 2019-06-18  7:41 UTC (permalink / raw)
  To: maxime.coquelin, zhihong.wang, dev; +Cc: stable

When there is no enough segments for a packet in in-order
mergeable Rx path, we should free the whole mbuf chain instead
of just the last segment.

Fixes: e5f456a98d3c ("net/virtio: support in-order Rx and Tx")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 1f1178467..bdb3a2f18 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1555,7 +1555,7 @@ virtio_recv_pkts_inorder(void *rx_queue,
 		} else {
 			PMD_RX_LOG(ERR,
 					"No enough segments for packet.");
-			virtio_discard_rxbuf_inorder(vq, prev);
+			rte_pktmbuf_free(rx_pkts[nb_rx]);
 			rxvq->stats.errors++;
 			break;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx
  2019-06-18  7:41 [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx Tiwei Bie
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx Tiwei Bie
@ 2019-06-18  7:41 ` Tiwei Bie
  2019-06-19 12:37   ` Maxime Coquelin
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx Tiwei Bie
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check " Tiwei Bie
  3 siblings, 1 reply; 10+ messages in thread
From: Tiwei Bie @ 2019-06-18  7:41 UTC (permalink / raw)
  To: maxime.coquelin, zhihong.wang, dev; +Cc: stable

When there is no enough segments for a packet in mergeable
Rx path, we should free the whole mbuf chain instead of just
the last segment.

Fixes: bcac5aa207f8 ("net/virtio: improve batching in mergeable path")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index bdb3a2f18..9cf422ffe 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1737,7 +1737,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 		} else {
 			PMD_RX_LOG(ERR,
 					"No enough segments for packet.");
-			virtio_discard_rxbuf(vq, prev);
+			rte_pktmbuf_free(rx_pkts[nb_rx]);
 			rxvq->stats.errors++;
 			break;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx
  2019-06-18  7:41 [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx Tiwei Bie
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx Tiwei Bie
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx Tiwei Bie
@ 2019-06-18  7:41 ` Tiwei Bie
  2019-06-19 12:37   ` Maxime Coquelin
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check " Tiwei Bie
  3 siblings, 1 reply; 10+ messages in thread
From: Tiwei Bie @ 2019-06-18  7:41 UTC (permalink / raw)
  To: maxime.coquelin, zhihong.wang, dev; +Cc: stable

When there is no enough segments for a packet in mergeable
packed Rx path, we should free the whole mbuf chain instead
of just the last segment.

Fixes: a76290c8f1cf ("net/virtio: implement Rx path for packed queues")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 9cf422ffe..8b75291f5 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1910,7 +1910,7 @@ virtio_recv_mergeable_pkts_packed(void *rx_queue,
 		} else {
 			PMD_RX_LOG(ERR,
 					"No enough segments for packet.");
-			virtio_discard_rxbuf(vq, prev);
+			rte_pktmbuf_free(rx_pkts[nb_rx]);
 			rxvq->stats.errors++;
 			break;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check in mergeable packed Rx
  2019-06-18  7:41 [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx Tiwei Bie
                   ` (2 preceding siblings ...)
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx Tiwei Bie
@ 2019-06-18  7:41 ` " Tiwei Bie
  2019-06-19 13:08   ` Maxime Coquelin
  3 siblings, 1 reply; 10+ messages in thread
From: Tiwei Bie @ 2019-06-18  7:41 UTC (permalink / raw)
  To: maxime.coquelin, zhihong.wang, dev; +Cc: stable

We should check the descriptor state instead of vq's internal
free count (i.e. the number of descriptors that we haven't made
available) for the remaining mergeable packets.

Fixes: a76290c8f1cf ("net/virtio: implement Rx path for packed queues")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_rxtx.c | 50 +++++++++++++++-----------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 8b75291f5..b5062ff84 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1882,38 +1882,34 @@ virtio_recv_mergeable_pkts_packed(void *rx_queue,
 	while (seg_res != 0) {
 		uint16_t rcv_cnt = RTE_MIN((uint16_t)seg_res,
 					VIRTIO_MBUF_BURST_SZ);
-		if (likely(vq->vq_free_cnt >= rcv_cnt)) {
-			num = virtqueue_dequeue_burst_rx_packed(vq, rcv_pkts,
-					len, rcv_cnt);
-			uint16_t extra_idx = 0;
+		uint16_t extra_idx = 0;
 
-			rcv_cnt = num;
-
-			while (extra_idx < rcv_cnt) {
-				rxm = rcv_pkts[extra_idx];
-
-				rxm->data_off =
-					RTE_PKTMBUF_HEADROOM - hdr_size;
-				rxm->pkt_len = (uint32_t)(len[extra_idx]);
-				rxm->data_len = (uint16_t)(len[extra_idx]);
-
-				prev->next = rxm;
-				prev = rxm;
-				rx_pkts[nb_rx]->pkt_len += len[extra_idx];
-				extra_idx += 1;
-			}
-			seg_res -= rcv_cnt;
-			if (!seg_res) {
-				virtio_rx_stats_updated(rxvq, rx_pkts[nb_rx]);
-				nb_rx++;
-			}
-		} else {
-			PMD_RX_LOG(ERR,
-					"No enough segments for packet.");
+		rcv_cnt = virtqueue_dequeue_burst_rx_packed(vq, rcv_pkts,
+				len, rcv_cnt);
+		if (unlikely(rcv_cnt == 0)) {
+			PMD_RX_LOG(ERR, "No enough segments for packet.");
 			rte_pktmbuf_free(rx_pkts[nb_rx]);
 			rxvq->stats.errors++;
 			break;
 		}
+
+		while (extra_idx < rcv_cnt) {
+			rxm = rcv_pkts[extra_idx];
+
+			rxm->data_off = RTE_PKTMBUF_HEADROOM - hdr_size;
+			rxm->pkt_len = (uint32_t)(len[extra_idx]);
+			rxm->data_len = (uint16_t)(len[extra_idx]);
+
+			prev->next = rxm;
+			prev = rxm;
+			rx_pkts[nb_rx]->pkt_len += len[extra_idx];
+			extra_idx += 1;
+		}
+		seg_res -= rcv_cnt;
+		if (!seg_res) {
+			virtio_rx_stats_updated(rxvq, rx_pkts[nb_rx]);
+			nb_rx++;
+		}
 	}
 
 	rxvq->stats.packets += nb_rx;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx Tiwei Bie
@ 2019-06-19 12:34   ` Maxime Coquelin
  2019-06-20  1:56     ` Tiwei Bie
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Coquelin @ 2019-06-19 12:34 UTC (permalink / raw)
  To: Tiwei Bie, zhihong.wang, dev; +Cc: stable



On 6/18/19 9:41 AM, Tiwei Bie wrote:
> When there is no enough segments for a packet in in-order
> mergeable Rx path, we should free the whole mbuf chain instead
> of just the last segment.

I would write instead:

"
we should free the whole mbuf chain instead of just recycling the last 
segment.
"

Because what was done before the patch it to refill the VQ with the last
segment. With your patch, the full chain is freed, then the refill is
done afterwards.

Do you agree?

Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks!
Maxime

> 
> Fixes: e5f456a98d3c ("net/virtio: support in-order Rx and Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 1f1178467..bdb3a2f18 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -1555,7 +1555,7 @@ virtio_recv_pkts_inorder(void *rx_queue,
>   		} else {
>   			PMD_RX_LOG(ERR,
>   					"No enough segments for packet.");
> -			virtio_discard_rxbuf_inorder(vq, prev);
> +			rte_pktmbuf_free(rx_pkts[nb_rx]);
>   			rxvq->stats.errors++;
>   			break;
>   		}
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx Tiwei Bie
@ 2019-06-19 12:37   ` Maxime Coquelin
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2019-06-19 12:37 UTC (permalink / raw)
  To: Tiwei Bie, zhihong.wang, dev; +Cc: stable



On 6/18/19 9:41 AM, Tiwei Bie wrote:
> When there is no enough segments for a packet in mergeable
> Rx path, we should free the whole mbuf chain instead of just
> the last segment.
> 
> Fixes: bcac5aa207f8 ("net/virtio: improve batching in mergeable path")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie<tiwei.bie@intel.com>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Same comment as patch 1.

Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx Tiwei Bie
@ 2019-06-19 12:37   ` Maxime Coquelin
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2019-06-19 12:37 UTC (permalink / raw)
  To: Tiwei Bie, zhihong.wang, dev; +Cc: stable



On 6/18/19 9:41 AM, Tiwei Bie wrote:
> When there is no enough segments for a packet in mergeable
> packed Rx path, we should free the whole mbuf chain instead
> of just the last segment.
> 
> Fixes: a76290c8f1cf ("net/virtio: implement Rx path for packed queues")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie<tiwei.bie@intel.com>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Same comment as patch 1.

Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check in mergeable packed Rx
  2019-06-18  7:41 ` [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check " Tiwei Bie
@ 2019-06-19 13:08   ` Maxime Coquelin
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2019-06-19 13:08 UTC (permalink / raw)
  To: Tiwei Bie, zhihong.wang, dev; +Cc: stable



On 6/18/19 9:41 AM, Tiwei Bie wrote:
> We should check the descriptor state instead of vq's internal
> free count (i.e. the number of descriptors that we haven't made
> available) for the remaining mergeable packets.
> 
> Fixes: a76290c8f1cf ("net/virtio: implement Rx path for packed queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 50 +++++++++++++++-----------------
>   1 file changed, 23 insertions(+), 27 deletions(-)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx
  2019-06-19 12:34   ` Maxime Coquelin
@ 2019-06-20  1:56     ` Tiwei Bie
  0 siblings, 0 replies; 10+ messages in thread
From: Tiwei Bie @ 2019-06-20  1:56 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: zhihong.wang, dev, stable

On Wed, Jun 19, 2019 at 02:34:40PM +0200, Maxime Coquelin wrote:
> On 6/18/19 9:41 AM, Tiwei Bie wrote:
> > When there is no enough segments for a packet in in-order
> > mergeable Rx path, we should free the whole mbuf chain instead
> > of just the last segment.
> 
> I would write instead:
> 
> "
> we should free the whole mbuf chain instead of just recycling the last
> segment.
> "
> 
> Because what was done before the patch it to refill the VQ with the last
> segment. With your patch, the full chain is freed, then the refill is
> done afterwards.
> 
> Do you agree?

Yeah, I totally agree :)

Thanks!
Tiwei

> 
> Other than that:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks!
> Maxime
> 
> > 
> > Fixes: e5f456a98d3c ("net/virtio: support in-order Rx and Tx")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> >   drivers/net/virtio/virtio_rxtx.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> > index 1f1178467..bdb3a2f18 100644
> > --- a/drivers/net/virtio/virtio_rxtx.c
> > +++ b/drivers/net/virtio/virtio_rxtx.c
> > @@ -1555,7 +1555,7 @@ virtio_recv_pkts_inorder(void *rx_queue,
> >   		} else {
> >   			PMD_RX_LOG(ERR,
> >   					"No enough segments for packet.");
> > -			virtio_discard_rxbuf_inorder(vq, prev);
> > +			rte_pktmbuf_free(rx_pkts[nb_rx]);
> >   			rxvq->stats.errors++;
> >   			break;
> >   		}
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18  7:41 [dpdk-dev] [PATCH 0/4] Some fixes for mergeable Rx Tiwei Bie
2019-06-18  7:41 ` [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx Tiwei Bie
2019-06-19 12:34   ` Maxime Coquelin
2019-06-20  1:56     ` Tiwei Bie
2019-06-18  7:41 ` [dpdk-dev] [PATCH 2/4] net/virtio: fix memory leak in mergeable Rx Tiwei Bie
2019-06-19 12:37   ` Maxime Coquelin
2019-06-18  7:41 ` [dpdk-dev] [PATCH 3/4] net/virtio: fix memory leak in mergeable packed Rx Tiwei Bie
2019-06-19 12:37   ` Maxime Coquelin
2019-06-18  7:41 ` [dpdk-dev] [PATCH 4/4] net/virtio: fix packets check " Tiwei Bie
2019-06-19 13:08   ` Maxime Coquelin

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox