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 14D7F459B6; Tue, 17 Sep 2024 20:07:51 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D7BC540280; Tue, 17 Sep 2024 20:07:50 +0200 (CEST) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 07FEC40261 for ; Tue, 17 Sep 2024 20:07:50 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4X7V5Q6G4Fz6LCDC; Wed, 18 Sep 2024 02:03:38 +0800 (CST) Received: from frapeml500005.china.huawei.com (unknown [7.182.85.13]) by mail.maildlp.com (Postfix) with ESMTPS id 6324D140159; Wed, 18 Sep 2024 02:07:25 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml500005.china.huawei.com (7.182.85.13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 17 Sep 2024 20:07:25 +0200 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2507.039; Tue, 17 Sep 2024 20:07:25 +0200 From: Konstantin Ananyev To: "vignesh.purushotham.srinivas@ericsson.com" , "konstantin.v.ananyev@yandex.ru" CC: "dev@dpdk.org" Subject: RE: [PATCH] ip_frag: support IPv6 reassembly with extensions Thread-Topic: [PATCH] ip_frag: support IPv6 reassembly with extensions Thread-Index: AQHa96p7DJtUreh0vEKAb1S1N5t0lLJcZvLg Date: Tue, 17 Sep 2024 18:07:25 +0000 Message-ID: <1265a3bd25f3463a898227adbabcb4d6@huawei.com> References: <20240826112328.3028488-1-vignesh.purushotham.srinivas@ericsson.com> In-Reply-To: <20240826112328.3028488-1-vignesh.purushotham.srinivas@ericsson.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.206.138.42] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 > From: Vignesh PS >=20 > Add support to ip_frag library to perform IPv6 reassembly > when extension headers are present before the fragment > extension in the packet. >=20 > Signed-off-by: Vignesh PS > --- > .mailmap | 1 + > lib/ip_frag/ip_frag_common.h | 2 + > lib/ip_frag/ip_reassembly.h | 2 + > lib/ip_frag/rte_ipv6_reassembly.c | 68 +++++++++++++++++++++++++++---- > 4 files changed, 64 insertions(+), 9 deletions(-) >=20 > diff --git a/.mailmap b/.mailmap > index 4a508bafad..69b229a5b7 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -1548,6 +1548,7 @@ Viacheslav Ovsiienko > Victor Kaplansky > Victor Raj > Vidya Sagar Velumuri > +Vignesh PS > Vignesh Sridhar > Vijayakumar Muthuvel Manickam > Vijaya Mohan Guvva > diff --git a/lib/ip_frag/ip_frag_common.h b/lib/ip_frag/ip_frag_common.h > index 51fc9d47fb..db2665e846 100644 > --- a/lib/ip_frag/ip_frag_common.h > +++ b/lib/ip_frag/ip_frag_common.h > @@ -169,6 +169,8 @@ ip_frag_reset(struct ip_frag_pkt *fp, uint64_t tms) > fp->total_size =3D UINT32_MAX; > fp->frag_size =3D 0; > fp->last_idx =3D IP_MIN_FRAG_NUM; > + fp->exts_len =3D 0; > + fp->next_proto =3D NULL; > fp->frags[IP_LAST_FRAG_IDX] =3D zero_frag; > fp->frags[IP_FIRST_FRAG_IDX] =3D zero_frag; > } > diff --git a/lib/ip_frag/ip_reassembly.h b/lib/ip_frag/ip_reassembly.h > index 54afed5417..429e74f1b3 100644 > --- a/lib/ip_frag/ip_reassembly.h > +++ b/lib/ip_frag/ip_reassembly.h > @@ -54,6 +54,8 @@ struct __rte_cache_aligned ip_frag_pkt { > uint32_t total_size; /* expected reassembled size */ > uint32_t frag_size; /* size of fragments received */ > uint32_t last_idx; /* index of next entry to fill *= / > + uint32_t exts_len; /* length of extension hdrs for = first fragment */ > + uint8_t *next_proto; /* pointer of the next_proto fie= ld */ > struct ip_frag frags[IP_MAX_FRAG_NUM]; /* fragments */ > }; >=20 > diff --git a/lib/ip_frag/rte_ipv6_reassembly.c b/lib/ip_frag/rte_ipv6_rea= ssembly.c > index 88863a98d1..8decf592a6 100644 > --- a/lib/ip_frag/rte_ipv6_reassembly.c > +++ b/lib/ip_frag/rte_ipv6_reassembly.c > @@ -91,19 +91,19 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp) > /* update ipv6 header for the reassembled datagram */ > ip_hdr =3D rte_pktmbuf_mtod_offset(m, struct rte_ipv6_hdr *, m->l2_len)= ; >=20 > + payload_len +=3D fp->exts_len; > ip_hdr->payload_len =3D rte_cpu_to_be_16(payload_len); >=20 > /* > * remove fragmentation header. note that per RFC2460, we need to updat= e > * the last non-fragmentable header with the "next header" field to con= tain > - * type of the first fragmentable header, but we currently don't suppor= t > - * other headers, so we assume there are no other headers and thus upda= te > - * the main IPv6 header instead. > + * type of the first fragmentable header. > */ > - move_len =3D m->l2_len + m->l3_len - sizeof(*frag_hdr); > - frag_hdr =3D (struct rte_ipv6_fragment_ext *) (ip_hdr + 1); > - ip_hdr->proto =3D frag_hdr->next_header; > + frag_hdr =3D (struct rte_ipv6_fragment_ext *) > + ((uint8_t *) (ip_hdr + 1) + fp->exts_len); > + *fp->next_proto =3D frag_hdr->next_header; >=20 > + move_len =3D m->l2_len + m->l3_len - sizeof(*frag_hdr); > ip_frag_memmove(rte_pktmbuf_mtod_offset(m, char *, sizeof(*frag_hdr)), > rte_pktmbuf_mtod(m, char*), move_len); >=20 > @@ -112,6 +112,39 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp) > return m; > } >=20 > +/* > + * Function to crawl through the extension header stack. > + * This function breaks as soon a the fragment header is > + * found and returns the total length the traversed exts > + * and the last extension before the fragment header > + */ > +static inline uint32_t > +ip_frag_get_last_exthdr(struct rte_ipv6_hdr *ip_hdr, uint8_t **last_ext) > +{ > + uint32_t total_len =3D 0; > + uint8_t num_exts =3D 0; > + size_t ext_len =3D 0; > + *last_ext =3D (uint8_t *)(ip_hdr + 1); > + int next_proto =3D ip_hdr->proto; > +#define MAX_NUM_IPV6_EXTS 8 As a nit - let's keep coding style consistent: Pls move #define outside the function definition.=20 > + > + while (next_proto !=3D IPPROTO_FRAGMENT && > + num_exts < MAX_NUM_IPV6_EXTS && > + (next_proto =3D rte_ipv6_get_next_ext( > + *last_ext, next_proto, &ext_len)) >=3D 0) { > + > + total_len +=3D ext_len; > + > + if (next_proto =3D=3D IPPROTO_FRAGMENT) > + return total_len; > + > + *last_ext +=3D ext_len; > + num_exts++; > + } So if IPPROTO_FRAGMENT was not found, we just use extension #8 instead? Shouldn't we return an error in that case, and probably drop the fragment? > + return total_len; > +} > + > /* > * Process new mbuf with fragment of IPV6 datagram. > * Incoming mbuf should have its l2_len/l3_len fields setup correctly. > @@ -139,6 +172,8 @@ rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tb= l *tbl, > { > struct ip_frag_pkt *fp; > struct ip_frag_key key; > + uint8_t *last_ipv6_ext; > + uint32_t exts_len; > uint16_t ip_ofs; > int32_t ip_len; > int32_t trim; > @@ -154,10 +189,10 @@ rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_= tbl *tbl, > /* > * as per RFC2460, payload length contains all extension headers > * as well. > - * since we don't support anything but frag headers, > - * this is what we remove from the payload len. > + * so we remove the extension len from the payload len. > */ > - ip_len =3D rte_be_to_cpu_16(ip_hdr->payload_len) - sizeof(*frag_hdr); > + exts_len =3D ip_frag_get_last_exthdr(ip_hdr, &last_ipv6_ext); > + ip_len =3D rte_be_to_cpu_16(ip_hdr->payload_len) - exts_len - sizeof(*f= rag_hdr); Hmm..., as I remember ip_len is what we want to preserve in the packet... Why we want to remove all previous ext headers here? > trim =3D mb->pkt_len - (ip_len + mb->l3_len + mb->l2_len); >=20 > IP_FRAG_LOG(DEBUG, "%s:%d:\n" > @@ -201,6 +236,21 @@ rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_t= bl *tbl, > /* process the fragmented packet. */ > mb =3D ip_frag_process(fp, dr, mb, ip_ofs, ip_len, > MORE_FRAGS(frag_hdr->frag_data)); Can you explain why we setting these new fp fields after 'ip_frag_process()= '? Ip_frag_process() itself can call reassembly() - if all fragments are alrea= dy in place. > + > + /* store extension stack info, only for first fragment */ > + if (ip_ofs =3D=3D 0) { If we want it for first fragment only, why not invoke ip_frag_get_last_exth= dr() only when ip_ofs =3D=3D 0? =20 > + /* > + * fp->next_proto points to either the IP's next header > + * or th next header of the extension before the fragment > + * extension > + */ > + fp->next_proto =3D (uint8_t *)&ip_hdr->proto; > + if (exts_len > 0) { > + fp->exts_len =3D exts_len; > + fp->next_proto =3D last_ipv6_ext; > + } > + } > + > ip_frag_inuse(tbl, fp); >=20 > IP_FRAG_LOG(DEBUG, "%s:%d:\n" > -- > 2.34.1