From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 79E41A0555; Sat, 4 Jun 2022 01:50:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5FA9D4021E; Sat, 4 Jun 2022 01:50:26 +0200 (CEST) Received: from forward501p.mail.yandex.net (forward501p.mail.yandex.net [77.88.28.111]) by mails.dpdk.org (Postfix) with ESMTP id 08D3640041 for ; Sat, 4 Jun 2022 01:50:25 +0200 (CEST) Received: from sas2-548eec917f61.qloud-c.yandex.net (sas2-548eec917f61.qloud-c.yandex.net [IPv6:2a02:6b8:c08:b889:0:640:548e:ec91]) by forward501p.mail.yandex.net (Yandex) with ESMTP id 858926212BA7; Sat, 4 Jun 2022 02:50:24 +0300 (MSK) Received: from sas1-78334f65778a.qloud-c.yandex.net (sas1-78334f65778a.qloud-c.yandex.net [2a02:6b8:c08:b21f:0:640:7833:4f65]) by sas2-548eec917f61.qloud-c.yandex.net (mxback/Yandex) with ESMTP id JUQ7QxVMxJ-oOfKaNah; Sat, 04 Jun 2022 02:50:24 +0300 X-Yandex-Fwd: 2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1654300224; bh=94WTGPdXifLNfzmZg89COI6tE10LuVRE9itTa9EFMvw=; h=In-Reply-To:From:Subject:Cc:References:Date:Message-ID:To; b=lIEw9gCMmACApjP+w81kmSlViogroIMgplLbj7oEZuLi8zshuF0qfKHyUAaLJm6MT YW4N/lsrAC2FNUpbWOvfhy2cO0xnbhB26PrRIvzzSSumW8p9nON7r3adtKCixpSmMe w35Lz6BM5SJpFLsXekmT3QV38y3mQ6gIWNBsfDA8= Authentication-Results: sas2-548eec917f61.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Received: by sas1-78334f65778a.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id SQgLSziypo-oNNirx3f; Sat, 04 Jun 2022 02:50:23 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) Message-ID: Date: Sat, 4 Jun 2022 00:50:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] ip_frag: add IPv4 fast fragment switch and test data Content-Language: en-US To: Huichao Cai , dev@dpdk.org Cc: konstantin.ananyev@intel.com References: <1654161183-5391-1-git-send-email-chcchc88@163.com> From: Konstantin Ananyev In-Reply-To: <1654161183-5391-1-git-send-email-chcchc88@163.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > Some NIC drivers support DEV_TX_OFFLOAD_MBUF_FAST_FREE offload( > Device supports optimization for fast release of mbufs.When set > application must guarantee that per-queue all mbufs comes from the > same mempool and has refcnt = 1).In order to adapt to this offload > function,we need to modify the existing fragment logic(attach mbuf, > so it is fast,we can call it fast fragment mode) and add the fragment > logic in the non-attach mbuf mode(slow fragment mode).Add some test > data for this modification. That doesn't look like a good idea to me. Yes, drivers configured with MBUF_FAST_FREE would not work correctly with indirect mbufs. So it is application responsibility to choose what it wants: either indirect mbufs enabled or MBUF_FAST_FREE enabled. Inside the lib we shouldn't try to guess what was particular driver configuration. Plus it is the change (and regression) of existing behavior - right now it is perfectly fine to use the same mbuf for both direct and indirect mbufs. If you really have a use-case for doing fragmentation via full copying all packet data, then probably the easiest and safest way would be to introduce new function: rte_ipv4_fragment_clone_packet(...) or so. > > Signed-off-by: Huichao Cai > --- > app/test/test_ipfrag.c | 14 +++++++-- > lib/ip_frag/rte_ipv4_fragmentation.c | 56 +++++++++++++++++++++++++----------- > 2 files changed, 51 insertions(+), 19 deletions(-) > > diff --git a/app/test/test_ipfrag.c b/app/test/test_ipfrag.c > index 610a86b..f5fe4b8 100644 > --- a/app/test/test_ipfrag.c > +++ b/app/test/test_ipfrag.c > @@ -407,12 +407,20 @@ static void ut_teardown(void) > pktid); > } > > - if (tests[i].ipv == 4) > - len = rte_ipv4_fragment_packet(b, pkts_out, BURST, > + if (tests[i].ipv == 4) { > + if (i % 2) > + len = rte_ipv4_fragment_packet(b, > + pkts_out, BURST, > tests[i].mtu_size, > direct_pool, > indirect_pool); > - else if (tests[i].ipv == 6) > + else > + len = rte_ipv4_fragment_packet(b, > + pkts_out, BURST, > + tests[i].mtu_size, > + direct_pool, > + direct_pool); > + } else if (tests[i].ipv == 6) > len = rte_ipv6_fragment_packet(b, pkts_out, BURST, > tests[i].mtu_size, > direct_pool, > diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c b/lib/ip_frag/rte_ipv4_fragmentation.c > index a562424..65bfad7 100644 > --- a/lib/ip_frag/rte_ipv4_fragmentation.c > +++ b/lib/ip_frag/rte_ipv4_fragmentation.c > @@ -102,6 +102,11 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph, > * MBUF pool used for allocating direct buffers for the output fragments. > * @param pool_indirect > * MBUF pool used for allocating indirect buffers for the output fragments. > + * If pool_indirect == pool_direct,this means that the fragment will adapt > + * to DEV_TX_OFFLOAD_MBUF_FAST_FREE offload. > + * DEV_TX_OFFLOAD_MBUF_FAST_FREE: Device supports optimization > + * for fast release of mbufs. When set application must guarantee that > + * per-queue all mbufs comes from the same mempool and has refcnt = 1. > * @return > * Upon successful completion - number of output fragments placed > * in the pkts_out array. > @@ -123,6 +128,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph, > uint16_t frag_bytes_remaining; > uint8_t ipopt_frag_hdr[IPV4_HDR_MAX_LEN]; > uint16_t ipopt_len; > + bool is_fast_frag_mode = true; > > /* > * Formal parameter checking. > @@ -133,6 +139,9 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph, > unlikely(mtu_size < RTE_ETHER_MIN_MTU)) > return -EINVAL; > > + if (pool_indirect == pool_direct) > + is_fast_frag_mode = false; > + > in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); > header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * > RTE_IPV4_IHL_MULTIPLIER; > @@ -190,30 +199,45 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph, > out_seg_prev = out_pkt; > more_out_segs = 1; > while (likely(more_out_segs && more_in_segs)) { > - struct rte_mbuf *out_seg = NULL; > uint32_t len; > > - /* Allocate indirect buffer */ > - out_seg = rte_pktmbuf_alloc(pool_indirect); > - if (unlikely(out_seg == NULL)) { > - rte_pktmbuf_free(out_pkt); > - __free_fragments(pkts_out, out_pkt_pos); > - return -ENOMEM; > - } > - out_seg_prev->next = out_seg; > - out_seg_prev = out_seg; > - > - /* Prepare indirect buffer */ > - rte_pktmbuf_attach(out_seg, in_seg); > len = frag_bytes_remaining; > if (len > (in_seg->data_len - in_seg_data_pos)) { > len = in_seg->data_len - in_seg_data_pos; > } > - out_seg->data_off = in_seg->data_off + in_seg_data_pos; > - out_seg->data_len = (uint16_t)len; > + > + if (is_fast_frag_mode) { > + struct rte_mbuf *out_seg = NULL; > + /* Allocate indirect buffer */ > + out_seg = rte_pktmbuf_alloc(pool_indirect); > + if (unlikely(out_seg == NULL)) { > + rte_pktmbuf_free(out_pkt); > + __free_fragments(pkts_out, out_pkt_pos); > + return -ENOMEM; > + } > + out_seg_prev->next = out_seg; > + out_seg_prev = out_seg; > + > + /* Prepare indirect buffer */ > + rte_pktmbuf_attach(out_seg, in_seg); > + > + out_seg->data_off = in_seg->data_off + > + in_seg_data_pos; > + out_seg->data_len = (uint16_t)len; > + out_pkt->nb_segs += 1; > + } else { > + rte_memcpy( > + rte_pktmbuf_mtod_offset(out_pkt, char *, > + out_pkt->pkt_len), > + rte_pktmbuf_mtod_offset(in_seg, char *, > + in_seg_data_pos), > + len); > + out_pkt->data_len = (uint16_t)(len + > + out_pkt->data_len); > + } > + > out_pkt->pkt_len = (uint16_t)(len + > out_pkt->pkt_len); > - out_pkt->nb_segs += 1; > in_seg_data_pos += len; > frag_bytes_remaining -= len; >