[ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on init.
Darrell Ball
dball at vmware.com
Wed Aug 9 17:58:03 UTC 2017
-----Original Message-----
From: Ben Pfaff <blp at ovn.org>
Date: Wednesday, August 9, 2017 at 10:40 AM
To: Darrell Ball <dball at vmware.com>
Cc: Darrell Ball <dlu998 at gmail.com>, "dev at openvswitch.org" <dev at openvswitch.org>
Subject: Re: [ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on init.
On Wed, Aug 09, 2017 at 05:32:02PM +0000, Darrell Ball wrote:
>
>
> -----Original Message-----
> From: <ovs-dev-bounces at openvswitch.org> on behalf of Ben Pfaff <blp at ovn.org>
> Date: Wednesday, August 9, 2017 at 10:15 AM
> To: Darrell Ball <dlu998 at gmail.com>
> Cc: "dev at openvswitch.org" <dev at openvswitch.org>
> Subject: Re: [ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on init.
>
> On Tue, Aug 08, 2017 at 06:54:46PM -0700, Darrell Ball wrote:
> > Reset the DPDK HWOL checksum flags in dp_packet_init_.
> > The new HWOL bad checksum flag is uninitialized for non-dpdk ports and
> > this is noticed as test failures using netdev-dummy ports, when built with
> > the --with-dpdk flag set. Hence, in this case, packets may be marked as
> > having a bad checksum.
>
> > diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> > index 67aa406..4926993 100644
> > --- a/lib/dp-packet.c
> > +++ b/lib/dp-packet.c
> > @@ -31,6 +31,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so
> > dp_packet_reset_offsets(b);
> > pkt_metadata_init(&b->md, 0);
> > dp_packet_rss_invalidate(b);
> > + reset_dp_packet_checksum_ol_flags(b);
>
> This function un-sets some bits in p->mbuf.ol_flags. The need for this
> implies that nothing initializes p->mbuf.ol_flags.
>
> Correct, I reused reset_dp_packet_checksum_ol_flags() to do the initialization as well
> I could also have created a separate function.
>
> In case a DPDK dev is used, those flags will be managed by DPDK.
>
> That sounds like a
> bug in itself--is there a missing call to initialize the mbuf somewhere?
>
> Are you suggesting to initialize the whole mbuf for each packet ?
The issue that I'm raising is that it's unusual to take an
uninitialized, indeterminate field, and then initialize it by clearing a
few bits. It's much more conventional to initialize it by setting it to
zero, like this:
p->mbuf.ol_flags = 0;
That is better; I will create a separate function then.
I will resend
Thanks
That made me wonder whether there's a larger problem of a failure to
initialize the mbuf more generally, although of course that does not
necessarily follow.
More information about the dev
mailing list