-----Original Message-----
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" 
<d...@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-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 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.
    

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to