From: Ben Pfaff <b...@ovn.org>
Date: Wednesday, August 9, 2017 at 10:40 AM
To: Darrell Ball <db...@vmware.com>
Cc: Darrell Ball <dlu...@gmail.com>, "d...@openvswitch.org"
Subject: Re: [ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on
On Wed, Aug 09, 2017 at 05:32:02PM +0000, Darrell Ball wrote:
> -----Original Message-----
> From: <ovs-dev-boun...@openvswitch.org> on behalf of Ben Pfaff
> Date: Wednesday, August 9, 2017 at 10:15 AM
> To: Darrell Ball <dlu...@gmail.com>
> Cc: "d...@openvswitch.org" <d...@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
> > this is noticed as test failures using netdev-dummy ports, when
> > the --with-dpdk flag set. Hence, in this case, packets may be
> > 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
> 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
> 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
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
dev mailing list