[ovs-dev] [runt-flows 5/8] datapath: Report memory allocation errors in flow_extract().

Jesse Gross jesse at nicira.com
Fri Aug 27 19:36:59 UTC 2010


On Fri, Aug 27, 2010 at 12:31 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Fri, Aug 27, 2010 at 11:36:11AM -0700, Jesse Gross wrote:
>> On Fri, Aug 27, 2010 at 9:47 AM, Ben Pfaff <blp at nicira.com> wrote:
>> > On Thu, Aug 26, 2010 at 02:56:04PM -0700, Jesse Gross wrote:
>> >> On Fri, Aug 13, 2010 at 10:55 AM, Ben Pfaff <blp at nicira.com> wrote:
>> >> > Until now flow_extract() has simply returned a bogus flow when memory
>> >> > allocation errors occurred.  This fixes the problem by propagating the
>> >> > error to the caller.
>> >> >
>> >> > Signed-off-by: Ben Pfaff <blp at nicira.com>
>> >>
>> >> Hmm, that's pretty nasty behavior that we had before.
>> >>
>> >> One general comment about this is that the behavior seems a little
>> >> asymmetrical for the different protocol types: why switch to this new
>> >> style for IP but not for ARP?  Why are the L4 protocols different from
>> >> L3?  Why can't we just do all of this at the beginning?  The answer to
>> >> all of these questions is (I assume) "IP options".  However, it took a
>> >> little more thought to convince myself that it is OK than seems
>> >> necessary.
>> >
>> > We could do it all at the beginning, if we are willing to pull 98 bytes
>> > of headers:
>> >
>> >        14      Ethernet
>> >        4       VLAN
>> >        60      max IP header
>> >        20      TCP/UDP/ICMP header
>> >        --
>> >        98
>> >
>> > The 64 bytes we currently pull covers any of the protocols we support as
>> > long as there are no IP options.  I was reluctant to increase the amount
>> > we pull.  But maybe it's not a big deal.  What do you think?
>>
>> I don't think it is a good idea to increase the amount that we pull.
>> Xen puts the first 64 or 72 bytes (depending on the version) of the
>> packet in the linear data area and the rest is paged.  If we increase
>> the pull amount we'll end up reallocating and copying for every packet
>> larger than the value that Xen chooses.
>>
>> I think that this just needs to be commented better.
>
> I added this comment:
>
>        /*
>         * We would really like to pull as many bytes as we could possibly
>         * want to parse into the linear data area.  Currently that is:
>         *
>         *    14     Ethernet header
>         *     4     VLAN header
>         *    60     max IP header with options
>         *    20     max TCP/UDP/ICMP header (don't care about options)
>         *    --
>         *    98
>         *
>         * But Xen only allocates 64 or 72 bytes for the linear data area in
>         * netback, which means that we would reallocate and copy the skb's
>         * linear data on every packet if we did that.  So instead just pull 64
>         * bytes, which is always sufficient without IP options, and then check
>         * whether we need to pull more later when we look at the IP header.
>         */
>
> Maybe too verbose...

Thanks.




More information about the dev mailing list