[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