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 <b...@ovn.org>
> 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 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 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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to