> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ananyev, > Konstantin > Sent: Friday, September 4, 2015 6:51 PM > To: Azarewicz, PiotrX T; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/1] ip_frag: fix creating ipv6 fragment > extension header > > Hi Piotr, > > > -----Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Piotr > > Sent: Wednesday, September 02, 2015 3:13 PM > > To: dev at dpdk.org > > Subject: [dpdk-dev] [PATCH 1/1] ip_frag: fix creating ipv6 fragment > extension header > > > > From: Piotr Azarewicz <piotrx.t.azarewicz at intel.com> > > > > Previous implementation won't work on every environment. The order of > > allocation of bit-fields within a unit (high-order to low-order or > > low-order to high-order) is implementation-defined. > > Solution: used bytes instead of bit fields. > > Seems like right thing to do to me. > Though I think we also should replace: > union { > struct { > uint16_t frag_offset:13; /**< Offset from the start > of the packet > */ > uint16_t reserved2:2; /**< Reserved */ > uint16_t more_frags:1; > /**< 1 if more fragments left, 0 if last fragment */ > }; > uint16_t frag_data; > /**< union of all fragmentation data */ > }; > > With just: > uint16_t frag_data; > and probably provide macros to read/set fragment_offset and more_flags > values. > Otherwise people might keep using the wrong layout. > Konstantin >
I agree with your proposal, but wouldn't this be an ABI change? To avoid an ABI change, we should probably leave the union? > > > > Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz at intel.com> > > --- > > lib/librte_ip_frag/rte_ipv6_fragmentation.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c > b/lib/librte_ip_frag/rte_ipv6_fragmentation.c > > index 0e32aa8..7342421 100644 > > --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c > > +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c > > @@ -65,10 +65,8 @@ __fill_ipv6hdr_frag(struct ipv6_hdr *dst, > > > > fh = (struct ipv6_extension_fragment *) ++dst; > > fh->next_header = src->proto; > > - fh->reserved1 = 0; > > - fh->frag_offset = rte_cpu_to_be_16(fofs); > > - fh->reserved2 = 0; > > - fh->more_frags = rte_cpu_to_be_16(mf); > > + fh->reserved1 = 0; > > + fh->frag_data = rte_cpu_to_be_16((fofs & ~IPV6_HDR_FO_MASK) | > mf); > > fh->id = 0; > > } > > > > -- > > 1.7.9.5