DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2] gso: fix free issue of mbuf gso segments attach to
@ 2020-10-22  6:51 yang_y_yi
  2020-10-22 13:16 ` Ananyev, Konstantin
  0 siblings, 1 reply; 12+ messages in thread
From: yang_y_yi @ 2020-10-22  6:51 UTC (permalink / raw)
  To: dev; +Cc: jiayu.hu, konstantin.ananyev, techboard, thomas, yangyi01, yang_y_yi

From: Yi Yang <yangyi01@inspur.com>

rte_gso_segment decreased refcnt of pkt by one, but
it is wrong if pkt is external mbuf, pkt won't be
freed because of incorrect refcnt, the result is
application can't allocate mbuf from mempool because
mbufs in mempool are run out of.

One correct way is application should call
rte_pktmbuf_free after calling rte_gso_segment to free
pkt explicitly. rte_gso_segment mustn't handle it, this
should be responsibility of application.

Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO")
Signed-off-by: Yi Yang <yangyi01@inspur.com>
---
Changelog:

v1->v2:
  - update description of rte_gso_segment().
  - change code which calls rte_gso_segment() to
    fix free issue.

---
 app/test-pmd/csumonly.c                                    | 3 ++-
 doc/guides/prog_guide/generic_segmentation_offload_lib.rst | 7 +++++--
 lib/librte_gso/rte_gso.c                                   | 9 +--------
 lib/librte_gso/rte_gso.h                                   | 7 +++++--
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 3d7d244..829e07f 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -1080,11 +1080,12 @@ struct simple_gre_hdr {
 			ret = rte_gso_segment(pkts_burst[i], gso_ctx,
 					&gso_segments[nb_segments],
 					GSO_MAX_PKT_BURST - nb_segments);
+			/* pkts_burst[i] can be freed safely here. */
+			rte_pktmbuf_free(pkts_burst[i]);
 			if (ret >= 0)
 				nb_segments += ret;
 			else {
 				TESTPMD_LOG(DEBUG, "Unable to segment packet");
-				rte_pktmbuf_free(pkts_burst[i]);
 			}
 		}
 
diff --git a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
index 205cb8a..8577572 100644
--- a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
+++ b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
@@ -25,8 +25,9 @@ Bearing that in mind, the GSO library enables DPDK applications to segment
 packets in software. Note however, that GSO is implemented as a standalone
 library, and not via a 'fallback' mechanism (i.e. for when TSO is unsupported
 in the underlying hardware); that is, applications must explicitly invoke the
-GSO library to segment packets. The size of GSO segments ``(segsz)`` is
-configurable by the application.
+GSO library to segment packets, they also must call ``rte_pktmbuf_free()`` to
+free mbuf GSO segments attach to after calling ``rte_gso_segment()``. The size
+of GSO segments ``(segsz)`` is configurable by the application.
 
 Limitations
 -----------
@@ -233,6 +234,8 @@ To segment an outgoing packet, an application must:
 
 #. Invoke the GSO segmentation API, ``rte_gso_segment()``.
 
+#. Call ``rte_pktmbuf_free()`` to free mbuf ``rte_gso_segment()`` segments.
+
 #. If required, update the L3 and L4 checksums of the newly-created segments.
    For tunneled packets, the outer IPv4 headers' checksums should also be
    updated. Alternatively, the application may offload checksum calculation
diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c
index 751b5b6..0d6cae5 100644
--- a/lib/librte_gso/rte_gso.c
+++ b/lib/librte_gso/rte_gso.c
@@ -30,7 +30,6 @@
 		uint16_t nb_pkts_out)
 {
 	struct rte_mempool *direct_pool, *indirect_pool;
-	struct rte_mbuf *pkt_seg;
 	uint64_t ol_flags;
 	uint16_t gso_size;
 	uint8_t ipid_delta;
@@ -80,13 +79,7 @@
 		return 1;
 	}
 
-	if (ret > 1) {
-		pkt_seg = pkt;
-		while (pkt_seg) {
-			rte_mbuf_refcnt_update(pkt_seg, -1);
-			pkt_seg = pkt_seg->next;
-		}
-	} else if (ret < 0) {
+	if (ret < 0) {
 		/* Revert the ol_flags in the event of failure. */
 		pkt->ol_flags = ol_flags;
 	}
diff --git a/lib/librte_gso/rte_gso.h b/lib/librte_gso/rte_gso.h
index 3aab297..f6694f9 100644
--- a/lib/librte_gso/rte_gso.h
+++ b/lib/librte_gso/rte_gso.h
@@ -89,8 +89,11 @@ struct rte_gso_ctx {
  * the GSO segments are sent to should support transmission of multi-segment
  * packets.
  *
- * If the input packet is GSO'd, its mbuf refcnt reduces by 1. Therefore,
- * when all GSO segments are freed, the input packet is freed automatically.
+ * If the input packet is GSO'd, all the indirect segments are attached to the
+ * input packet.
+ *
+ * rte_gso_segment() will not free the input packet no matter whether it is
+ * GSO'd or not, the application should free it after call rte_gso_segment().
  *
  * If the memory space in pkts_out or MBUF pools is insufficient, this
  * function fails, and it returns (-1) * errno. Otherwise, GSO succeeds,
-- 
1.8.3.1


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

end of thread, other threads:[~2020-10-26  6:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22  6:51 [dpdk-dev] [PATCH v2] gso: fix free issue of mbuf gso segments attach to yang_y_yi
2020-10-22 13:16 ` Ananyev, Konstantin
2020-10-22 15:33   ` Ananyev, Konstantin
2020-10-23  0:57     ` Hu, Jiayu
2020-10-23 13:23       ` yang_y_yi
2020-10-23 13:21     ` yang_y_yi
2020-10-23 13:18   ` yang_y_yi
2020-10-23 14:46     ` Ananyev, Konstantin
2020-10-26  0:57       ` yang_y_yi
2020-10-26  2:06         ` Jiayu Hu
2020-10-26  2:12           ` yang_y_yi
2020-10-26  6:16             ` Jiayu Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).