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 27CA2A0032; Mon, 11 Jul 2022 11:18:31 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B4CF940223; Mon, 11 Jul 2022 11:18:30 +0200 (CEST) Received: from forward500p.mail.yandex.net (forward500p.mail.yandex.net [77.88.28.110]) by mails.dpdk.org (Postfix) with ESMTP id BC1EC4021F for ; Mon, 11 Jul 2022 11:18:28 +0200 (CEST) Received: from vla5-94a0a219a8ea.qloud-c.yandex.net (vla5-94a0a219a8ea.qloud-c.yandex.net [IPv6:2a02:6b8:c18:3491:0:640:94a0:a219]) by forward500p.mail.yandex.net (Yandex) with ESMTP id C3DBCF02BB5; Mon, 11 Jul 2022 12:14:20 +0300 (MSK) Received: from vla3-23c3b031fed5.qloud-c.yandex.net (vla3-23c3b031fed5.qloud-c.yandex.net [2a02:6b8:c15:2582:0:640:23c3:b031]) by vla5-94a0a219a8ea.qloud-c.yandex.net (mxback/Yandex) with ESMTP id gJmLzIEQ9c-EKgSL0G3; Mon, 11 Jul 2022 12:14:20 +0300 X-Yandex-Fwd: 2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1657530860; bh=wFGkOziAddFR0SVKYmy5vfJdTIxiV2mxswr5Fn6WdXc=; h=In-Reply-To:From:Subject:Cc:References:Date:Message-ID:To; b=Q98p7GtEeVQdom5g0nLTyvlV6stAVpt7j5yRcWQotITtX/Vknxcxf41fkQoGyrvX6 YfDCPRctbJ2FnuM6DkA/Phfe5bXyM0MD3FxQZ7VzIKDZ77MWSJPa1GIunsuDJjxnk1 taN/vhN7yYkWMGa3dJ7nLaFTXV8KTjqUx3YldFA8= Authentication-Results: vla5-94a0a219a8ea.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Received: by vla3-23c3b031fed5.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id kwc7kNI6Wv-EJeWJkjd; Mon, 11 Jul 2022 12:14:20 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) Message-ID: Date: Mon, 11 Jul 2022 10:14:17 +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 v2] ip_frag: add IPv4 fragment copy packet API Content-Language: en-US From: Konstantin Ananyev To: chcchc88@163.com Cc: dev@dpdk.org References: <1654784398-11315-1-git-send-email-chcchc88@163.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 11/07/2022 00:35, Konstantin Ananyev пишет: > > Some NIC drivers support 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) > > offload. In order to adapt to this offload function, add this API. > > Add some test data for this API. > > > > Signed-off-by: Huichao Cai > > --- > > ... > > > --- a/lib/ip_frag/rte_ip_frag.h > > +++ b/lib/ip_frag/rte_ip_frag.h > > @@ -179,6 +179,32 @@ int32_t rte_ipv4_fragment_packet(struct rte_mbuf > > *pkt_in, > >               struct rte_mempool *pool_indirect); > > > >   /** > > + * IPv4 fragmentation by copy. > > + * > > + * This function implements the fragmentation of IPv4 packets by copy. > > + * > > + * @param pkt_in > > + *   The input packet. > > + * @param pkts_out > > + *   Array storing the output fragments. > > + * @param nb_pkts_out > > + *   The size of the array that stores the output fragments. > > + * @param mtu_size > > + *   Size in bytes of the Maximum Transfer Unit (MTU) for the outgoing > > IPv4 > > + *   datagrams. This value includes the size of the IPv4 header. > > + * @return > > + *   Upon successful completion - number of output fragments placed > > + *   in the pkts_out array. > > + *   Otherwise - (-1) * errno. > > + */ > > +__rte_experimental > > +int32_t > > +rte_ipv4_fragment_copy_packet(struct rte_mbuf *pkt_in, > > +    struct rte_mbuf **pkts_out, > > +    uint16_t nb_pkts_out, > > +    uint16_t mtu_size); > > + > > +/** > >    * This function implements reassembly of fragmented IPv4 packets. > >    * Incoming mbufs should have its l2_len/l3_len fields setup > correctly. > >    * > > diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c > > b/lib/ip_frag/rte_ipv4_fragmentation.c > > index a562424..9e050cc 100644 > > --- a/lib/ip_frag/rte_ipv4_fragmentation.c > > +++ b/lib/ip_frag/rte_ipv4_fragmentation.c > > @@ -262,3 +262,168 @@ static inline uint16_t > > __create_ipopt_frag_hdr(uint8_t *iph, > > > >       return out_pkt_pos; > >   } > > + > > +/** > > + * IPv4 fragmentation by copy. > > + * > > + * This function implements the fragmentation of IPv4 packets by copy. > > + * > > + * @param pkt_in > > + *   The input packet. > > + * @param pkts_out > > + *   Array storing the output fragments. > > + * @param nb_pkts_out > > + *   The size of the array that stores the output fragments. > > + * @param mtu_size > > + *   Size in bytes of the Maximum Transfer Unit (MTU) for the outgoing > > IPv4 > > + *   datagrams. This value includes the size of the IPv4 header. > > + * @return > > + *   Upon successful completion - number of output fragments placed > > + *   in the pkts_out array. > > + *   Otherwise - (-1) * errno. > > + */ > > +int32_t > > +rte_ipv4_fragment_copy_packet(struct rte_mbuf *pkt_in, > > +    struct rte_mbuf **pkts_out, > > +    uint16_t nb_pkts_out, > > +    uint16_t mtu_size) Forgot to mention, new API has to be experimental. > > +{ > > +    struct rte_mbuf *in_seg = NULL; > > +    struct rte_ipv4_hdr *in_hdr; > > +    uint32_t out_pkt_pos, in_seg_data_pos; > > +    uint32_t more_in_segs; > > +    uint16_t fragment_offset, flag_offset, frag_size, header_len; > > +    uint16_t frag_bytes_remaining; > > +    uint8_t ipopt_frag_hdr[IPV4_HDR_MAX_LEN]; > > +    uint16_t ipopt_len; > > + > > +    /* > > +     * Formal parameter checking. > > +     */ > > +    if (unlikely(pkt_in == NULL) || unlikely(pkts_out == NULL) || > > +        unlikely(nb_pkts_out == 0) || > > +        unlikely(pkt_in->pool == NULL) || > > +        unlikely(mtu_size < RTE_ETHER_MIN_MTU)) > > +        return -EINVAL; > > + > > +    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; > > + > > +    /* Check IP header length */ > > +    if (unlikely(pkt_in->data_len < header_len) || > > +        unlikely(mtu_size < header_len)) > > +        return -EINVAL; > > + > > +    /* > > +     * Ensure the IP payload length of all fragments is aligned to a > > +     * multiple of 8 bytes as per RFC791 section 2.3. > > +     */ > > +    frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), > > +                    IPV4_HDR_FO_ALIGN); > > + > > +    flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); > > + > > +    /* If Don't Fragment flag is set */ > > +    if (unlikely((flag_offset & IPV4_HDR_DF_MASK) != 0)) > > +        return -ENOTSUP; > > + > > +    /* Check that pkts_out is big enough to hold all fragments */ > > +    if (unlikely(frag_size * nb_pkts_out < > > +        (uint16_t)(pkt_in->pkt_len - header_len))) > > +        return -EINVAL; > > + > > +    in_seg = pkt_in; > > +    in_seg_data_pos = header_len; > > +    out_pkt_pos = 0; > > +    fragment_offset = 0; > > + > > +    ipopt_len = header_len - sizeof(struct rte_ipv4_hdr); > > +    if (unlikely(ipopt_len > RTE_IPV4_HDR_OPT_MAX_LEN)) > > +        return -EINVAL; > > + > > +    more_in_segs = 1; > > +    while (likely(more_in_segs)) { > > +        struct rte_mbuf *out_pkt = NULL; > > +        uint32_t more_out_segs; > > +        struct rte_ipv4_hdr *out_hdr; > > + > > +        /* Allocate buffer from pkt_in->pool*/ > > +        out_pkt = rte_pktmbuf_alloc(pkt_in->pool); > > > Instead of implicitly assuming that output mbufs will be allocated > from pkt_in pool, it would be better to have output_pool as explicit > parameter for that function. > In a same way we have it for rte_ipv4_fragment_packet(). > > > +        if (unlikely(out_pkt == NULL)) { > > +            __free_fragments(pkts_out, out_pkt_pos); > > +            return -ENOMEM; > > +        } > > + > > +        /* Reserve space for the IP header that will be built later */ > > +        out_pkt->data_len = header_len; > > +        out_pkt->pkt_len = header_len; > > +        frag_bytes_remaining = frag_size; > > + > > +        more_out_segs = 1; > > +        while (likely(more_out_segs && more_in_segs)) { > > > If I understand correctly, here you assume that out_pkt will always > be big enough to hold entire fragment, right? > But that can not always be the case and probably we shouldn't assume > that for generic function. > I suppose safest way would be either use rte_pktmbuf_copy() here > directly or do something similar to what that function doing ourselves > here. > > > > +            uint32_t len; > > + > > +            len = frag_bytes_remaining; > > +            if (len > (in_seg->data_len - in_seg_data_pos)) > > +                len = in_seg->data_len - in_seg_data_pos; > > + > > +            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); > > +            in_seg_data_pos += len; > > +            frag_bytes_remaining -= len; > > + > > +            /* Current output packet (i.e. fragment) done ? */ > > +            if (unlikely(frag_bytes_remaining == 0)) > > +                more_out_segs = 0; > > + > > +            /* Current input segment done ? */ > > +            if (unlikely(in_seg_data_pos == in_seg->data_len)) { > > +                in_seg = in_seg->next; > > +                in_seg_data_pos = 0; > > + > > +                if (unlikely(in_seg == NULL)) > > +                    more_in_segs = 0; > > +            } > > +        } > > + > > +        /* Build the IP header */ > > + > > +        out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); > > + > > +        __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, > > +            (uint16_t)out_pkt->pkt_len, > > +            flag_offset, fragment_offset, more_in_segs); > > + > > +        if (unlikely((fragment_offset == 0) && (ipopt_len) && > > +                ((flag_offset & RTE_IPV4_HDR_OFFSET_MASK) == 0))) { > > +            ipopt_len = __create_ipopt_frag_hdr((uint8_t *)in_hdr, > > +                ipopt_len, ipopt_frag_hdr); > > +            fragment_offset = (uint16_t)(fragment_offset + > > +                out_pkt->pkt_len - header_len); > > +            out_pkt->l3_len = header_len; > > + > > +            header_len = sizeof(struct rte_ipv4_hdr) + ipopt_len; > > +            in_hdr = (struct rte_ipv4_hdr *)ipopt_frag_hdr; > > +        } else { > > +            fragment_offset = (uint16_t)(fragment_offset + > > +                out_pkt->pkt_len - header_len); > > +            out_pkt->l3_len = header_len; > > +        } > > + > > +        /* Write the fragment to the output list */ > > +        pkts_out[out_pkt_pos] = out_pkt; > > +        out_pkt_pos++; > > +    } > > + > > +    return out_pkt_pos; > > +} > > diff --git a/lib/ip_frag/version.map b/lib/ip_frag/version.map > > index e537224..4aa66bc 100644 > > --- a/lib/ip_frag/version.map > > +++ b/lib/ip_frag/version.map > > @@ -17,4 +17,5 @@ EXPERIMENTAL { > >       global: > > > >       rte_ip_frag_table_del_expired_entries; > > +    rte_ipv4_fragment_copy_packet; > >   };