From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7B560A04DD; Thu, 22 Oct 2020 08:52:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D4C834CA6; Thu, 22 Oct 2020 08:52:02 +0200 (CEST) Received: from mail-m975.mail.163.com (mail-m975.mail.163.com [123.126.97.5]) by dpdk.org (Postfix) with ESMTP id 548232B82; Thu, 22 Oct 2020 08:51:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=ta7az gcHLdBVwfr2k/7QyeKLAMP/owzI3bWpV/VFE7k=; b=DQHjb+9euf7vI8jM4Gvp7 joMo6C7DHmV2bnSKmuMlxZGw4H/vp6ddpkb+v2Xs67LcL2ttNklGAdpkfsEndcG/ s7BBUVVHCB6G+e6raRriY35HDAd0+3fOjKCFnXaws0RFZtocj47aIxbVCW8ekTyf vhxIIKtUP01vzJm/6gcZeA= Received: from yangyi0100.home.langchao.com (unknown [111.207.123.58]) by smtp5 (Coremail) with SMTP id HdxpCgCnMbkHLJFfb7wYAA--.56S2; Thu, 22 Oct 2020 14:51:55 +0800 (CST) From: yang_y_yi@163.com To: dev@dpdk.org Cc: jiayu.hu@intel.com, konstantin.ananyev@intel.com, techboard@dpdk.org, thomas@monjalon.net, yangyi01@inspur.com, yang_y_yi@163.com Date: Thu, 22 Oct 2020 14:51:51 +0800 Message-Id: <20201022065151.10108-1-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: HdxpCgCnMbkHLJFfb7wYAA--.56S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxuF1Dtr4DCr17JFy7Xr1fWFg_yoWrurWrpr ZxKa1xtrn3Jr1xCFWkt3Wjq3s3GFZ5Gr13GrWDW34rZr1rG3yIqFWDta45ZFyfXrW5Aa1x XrsFgayUAa1UC3JanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07UfEfrUUUUU= X-Originating-IP: [111.207.123.58] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/xtbBEAvFi1UMUMkwtAAAsR Subject: [dpdk-dev] [PATCH v2] gso: fix free issue of mbuf gso segments attach to X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Yi Yang 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 --- 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