> -----Original Message-----
> From: yang_y...@163.com <yang_y...@163.com>
> Sent: Saturday, October 10, 2020 11:10 AM
> To: dev@dpdk.org
> Cc: Hu, Jiayu <jiayu...@intel.com>; mark.b.kavan...@intel.com; Ananyev,
> Konstantin <konstantin.anan...@intel.com>; olivier.m...@6wind.com;
> tho...@monjalon.net; yangy...@inspur.com; yang_y...@163.com
> Subject: [PATCH] gso: fix free issue of mbuf gso segments attach to
>
> From: Yi Yang <yangy...@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.
GSO doesn't support the input pktmbuf has external buffer.
Indeed, requiring users to free the input pktmbuf can avoid
memory leak, but I'm afraid that it also changes the semantic
of rte_gso_segment() which is defined in rte_gso.h.
@Konstantin, any suggestions?
Thanks,
Jiayu
>
> Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO")
> Signed-off-by: Yi Yang <yangy...@inspur.com>
> ---
> doc/guides/prog_guide/generic_segmentation_offload_lib.rst | 7 +++++--
> lib/librte_gso/rte_gso.c | 9 +--------
> 2 files changed, 6 insertions(+), 10 deletions(-)
>
> 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;
> }
> --
> 1.8.3.1
