-----Original Message----- From: Konstantin Ananyev <konstantin.anan...@huawei.com> To: Stephen Hemminger <step...@networkplumber.org>, vignesh.purushotham.srini...@ericsson.com <vignesh.purushotham.srini...@ericsson.com> Cc: konstantin.v.anan...@yandex.ru <konstantin.v.anan...@yandex.ru>, dev@dpdk.org <dev@dpdk.org> Subject: RE: [PATCH] ip_frag: support IPv6 reassembly with extensions Date: Tue, 17 Sep 2024 17:57:59 +0000
[You don't often get email from konstantin.anan...@huawei.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Mon, 26 Aug 2024 13:23:28 +0200 > <vignesh.purushotham.srini...@ericsson.com> wrote: > > > 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 field */ > > struct ip_frag frags[IP_MAX_FRAG_NUM]; /* fragments */ > > }; > > This creates a 32 bit hole in the structure. > Better to put next_proto after the start field. Another alternative - use offset within the mbuf instead of pointer. ACK > > > + > > + while (next_proto != IPPROTO_FRAGMENT && > > + num_exts < MAX_NUM_IPV6_EXTS && > > + (next_proto = rte_ipv6_get_next_ext( > > + *last_ext, next_proto, &ext_len)) >= 0) { > > I would break up this loop condition for clarity. + 1 ACK > Something like: > > while (next_proto != IPPROTO_FRAGMENT && num_exts < > MAX_NUM_IPV6_EXTS) { > next_proto = rte_ipv6_get_next_ext(*last_ext, > next_proto, &ext_len); > if (next_proto < 0) > break > > Also, need a new test cases for this. Agree, that would be good thing to add. ACK
