Fix loss of data stored in udata64 mbuf field
Checks
Commit Message
DPDK v20.11 removed udata64 field, and changed type of the dynfield1 to
uint32_t.
Due to define:
lib/common/pg_compat.h:#define udata64 dynfield1[0]
the copy of udata64 was in fact copying only the first 32 bits.
Using udata64 as an address is ok, as it will point to the beginning of
the dynfield1 array.
Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
---
lib/common/mbuf.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Comments
On Thu, Feb 4, 2021 at 9:52 AM Michal Krawczyk <mk@semihalf.com> wrote:
>
> DPDK v20.11 removed udata64 field, and changed type of the dynfield1 to
> uint32_t.
>
> Due to define:
> lib/common/pg_compat.h:#define udata64 dynfield1[0]
> the copy of udata64 was in fact copying only the first 32 bits.
I did not look at the pktgen code, but directly accessing mbuf field
dynfieldX is likely a bug.
pktgen should register a dynfield for its own usage to avoid conflicts
with other parts of the dpdk.
Example of such a conversion to the new mechanism:
https://git.dpdk.org/dpdk/commit/?id=eb8258402b3f
czw., 4 lut 2021 o 10:01 David Marchand <david.marchand@redhat.com> napisał(a):
>
> On Thu, Feb 4, 2021 at 9:52 AM Michal Krawczyk <mk@semihalf.com> wrote:
> >
> > DPDK v20.11 removed udata64 field, and changed type of the dynfield1 to
> > uint32_t.
> >
> > Due to define:
> > lib/common/pg_compat.h:#define udata64 dynfield1[0]
> > the copy of udata64 was in fact copying only the first 32 bits.
>
> I did not look at the pktgen code, but directly accessing mbuf field
> dynfieldX is likely a bug.
>
> pktgen should register a dynfield for its own usage to avoid conflicts
> with other parts of the dpdk.
> Example of such a conversion to the new mechanism:
> https://git.dpdk.org/dpdk/commit/?id=eb8258402b3f
>
Hi David,
thanks for pointing this out. I wasn't aware of that, I'll rework my
fix to satisfy the dynfield requirements.
Best regards,
Michal
>
> --
> David Marchand
>
04/02/2021 10:17, Michał Krawczyk:
> czw., 4 lut 2021 o 10:01 David Marchand <david.marchand@redhat.com> napisał(a):
> >
> > On Thu, Feb 4, 2021 at 9:52 AM Michal Krawczyk <mk@semihalf.com> wrote:
> > >
> > > DPDK v20.11 removed udata64 field, and changed type of the dynfield1 to
> > > uint32_t.
> > >
> > > Due to define:
> > > lib/common/pg_compat.h:#define udata64 dynfield1[0]
> > > the copy of udata64 was in fact copying only the first 32 bits.
> >
> > I did not look at the pktgen code, but directly accessing mbuf field
> > dynfieldX is likely a bug.
> >
> > pktgen should register a dynfield for its own usage to avoid conflicts
> > with other parts of the dpdk.
> > Example of such a conversion to the new mechanism:
> > https://git.dpdk.org/dpdk/commit/?id=eb8258402b3f
> >
>
> Hi David,
>
> thanks for pointing this out. I wasn't aware of that, I'll rework my
> fix to satisfy the dynfield requirements.
Sorry for having to say that,
but you are supposed to read the release notes when upgrading.
@@ -30,7 +30,11 @@ pktmbuf_reset(struct rte_mbuf *m)
{
union pktgen_data d;
- d.udata = m->udata64; /* Save the original value */
+ /* Save the original value. As udata64 can be either single field or the
+ * first value of the 32-bit elements array, the whole memory chunk must
+ * be converted directly to 64-bit.
+ */
+ d.udata = *(uint64_t *)(&m->udata64);
rte_pktmbuf_reset(m);