[ovs-dev] [PATCH] datapath: Properly initialize ovs_skb_cb of packet from userspace.

Jesse Gross jesse at nicira.com
Thu Feb 24 04:08:43 UTC 2011


On Tue, Feb 22, 2011 at 2:40 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, Feb 22, 2011 at 02:33:13PM -0800, Jesse Gross wrote:
>> On Fri, Feb 18, 2011 at 4:10 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > diff --git a/datapath/datapath.c b/datapath/datapath.c
>> > index 940a581..dcff05f 100644
>> > --- a/datapath/datapath.c
>> > +++ b/datapath/datapath.c
>> > @@ -709,6 +709,15 @@ static int odp_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
>> >        if (err)
>> >                goto exit;
>> >
>> > +       /* Initialize OVS_CB (it came from Netlink so might not be zeroed). */
>> > +       OVS_CB(packet)->vport = NULL;
>> > +       OVS_CB(packet)->flow = NULL;
>> > +       /* execute_actions() will reset tun_id to 0 anyhow. */
>> > +#ifdef NEED_CSUM_NORMALIZE
>> > +       OVS_CB(packet)->ip_summed = OVS_CSUM_NONE;
>> > +#endif
>> > +       vlan_copy_skb_tci(packet);
>>
>> Is there a reason to not just do:
>> memset(packet->cb, 0, sizeof(struct ovs_skb_cb));
>>
>> It's essentially what we were getting before with the ioctls since
>> alloc_skb() zeros out these fields and it's a bit nicer to not have to
>> enumerate all of the members.
>
> I have no objection to doing it that way.  Doing it this way forced me
> to consider what each member should actually be initialized to, but in
> fact all-zeros is fine.

I like the memset version a little better so I sent out a patch to do that.




More information about the dev mailing list